[PATCH] gitweb: Harden parse_commit and parse_commits
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Feb 8, 2012, 15:04 UTC
- Message-ID
- <201202081604.17187.jnareb@gmail.com>
- In-Reply-To
- <201202071753.12436.jnareb@gmail.com>
On Tue, 7 Feb 2012, Jakub Narebski wrote:
> On Mon, 6 Feb 2012, rajesh boyapati wrote:
[...]
Show 26 quoted lines
> > Then, I restarted gerrit server to take changes. > > Now the error log of gerrit shows: > > > [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] > > > <<<<<<<<<<<<<<<< > > 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 <jnareb@gmail.com> 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 <boyapatisrajesh@gmail.com> Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- 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