[PATCH] gitweb: Harden parse_commit and parse_commits

On Tue, 7 Feb 2012, Jakub Narebski wrote:

On Mon, 6 Feb 2012, rajesh boyapati wrote:

[...]

quoted

Then, I restarted gerrit server to take changes.
Now the error log of gerrit shows:

quoted

[2012-02-06 11:21:46,726] ERROR
com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
'HEAD'
[2012-02-06 11:21:49,167] ERROR
com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: [Mon Feb  6 11:21:49
2012] gitweb.cgi: Use of uninitialized value $commit_id in open at
/usr/lib/cgi-bin/gitweb.cgi line 2817.
[2012-02-06 11:21:49,169] ERROR
com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision ''

[the same errors repeated few times]

quoted

<<<<<<<<<<<<<<<<
Previously, there is a error showing at line 4720. Now, with this patch,
that error has gone.

As I said I was able to find a fix only for part of the issue.  
Unfortunately I was not able to reproduce this error in this form.
Note that the error location doesn't help much, because it is more
interesting for find which callers of parse_commits() pass undefined
$commit_id.

I can try to harden parse_commits() against bogus parameters; maybe
this would help.

Does the following patch help, and does it fix the issue?

(Nb. you can try to simply change filename, and apply it with fuzz
against index.cgi file).
-- >8 -- ----- ----- ----- ----- ----- -- >8 --
From: Jakub Narebski <redacted>
Subject: [PATCH] gitweb: Harden parse_commit and parse_commits

Gitweb has problems and gives errors when repository it shows is on
unborn branch (HEAD doesn't point to a valid commit), but there exist
other branches.

One of errors that shows in gitweb logs is undefined $commit_id in
parse_commits() subroutine.  Therefore we harden both parse_commit()
and parse_commits() against undefined $commit_id, and against no
output from git-rev-list because HEAD doesn't point to a commit.

Reported-by: rajesh boyapati <redacted>
Signed-off-by: Jakub Narebski <redacted>
---
 gitweb/gitweb.perl |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f9535eb..1181aeb 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -3334,6 +3334,8 @@ sub parse_commit {
 	my ($commit_id) = @_;
 	my %co;
 
+	return unless defined $commit_id;
+
 	local $/ = "\0";
 
 	open my $fd, "-|", git_cmd(), "rev-list",

@@ -3343,7 +3345,9 @@ sub parse_commit {
 		$commit_id,
 		"--",
 		or die_error(500, "Open git-rev-list failed");
-	%co = parse_commit_text(<$fd>, 1);
+	my $commit_text = <$fd>;
+	%co = parse_commit_text($commit_text, 1)
+		if defined $commit_text;
 	close $fd;
 
 	return %co;

@@ -3353,6 +3357,7 @@ sub parse_commits {
 	my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
 	my @cos;
 
+	return unless defined $commit_id;
 	$maxcount ||= 1;
 	$skip ||= 0;
 

-- 
1.7.9