Re: gitweb not friendly to firefox revived

[Jakub Narebski <jnareb@gmail.com>]

On Tue, Aug 03, 2010, Uwe Kleine-König wrote:
> On Sun, Aug 01, 2010 at 01:26:16PM -0700, Jakub Narebski wrote:
> > Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:
> > 
> > > Hello,
> > > 
> > > gitweb (at least) doesn't quote author names enough.
> > > 
> > > Firefox barfs for me at looking at
> > > 
> > > 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> > > 
> > > with an error:
> > > 
> > > 	XML Parsing Error: not well-formed Location:
> > > http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> > > Line Number 112, Column 81:
> > > <td class="author"><a title="Search for commits authored by YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B" class="list" href="/?p=ukl/linux-2.6.git;a=search;h=v2.6.16.10;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a></td><td><a class="list subject" title="[PATCH] IPV6: XFRM: Fix decoding session with preceding extension header(s)." href="/?p=ukl/linux-2.6.git;a=commit;h=fa39df2ff7f6102f1f37d3cf1f68243534d56253">[PATCH] IPV6: XFRM: Fix decoding session with preceding... </a></td>
> > > --------------------------------------------------------------------------------^
> > > 
> > > This is with git 1.7.1 and Iceweasel (aka. Firefox) 3.5.10.
> > > 
> > > Making
> > > 
> > > 	title=>"Search for commits $performed by $author"
> > > 
> > > in line 1694 of Debian's /usr/lib/cgi-bin/gitweb.cgi from the git 1.7.1
> > > package read
> > > 
> > > 	title=>esc_html("Search for commits $performed by $author")
> > > 
> > > this problem goes away.  (Still my browser barfs when clicking at the name.)
> > > 
> > > I'm not sure if this is the right way to fix this and I'm too tired now
> > > to do a complete patch, so I let this for someone else.
> > 
> > Actually gitweb leaves quoting of tag attributes to CGI module:
> > 
> >   return $cgi->a({-href => href(action=>"search", hash=>$hash,
> >                                 searchtext=>$author, searchtype=>$searchtype),
> >                   -class => "list",
> >                   -title => "Search for commits $performed by $author"},
> >                  $displaytext);
> > 
> > I am worrying (perhaps unnecessary) that using esc_html would result
> > in double escaping.  But it looks like the problem is with Unicode,
> > so perhaps using
> > 
> >   	title => to_utf8("Search for commits $performed by $author")
> > 
> > in place of
> > 
> >   	title=>esc_html("Search for commits $performed by $author")
> > 
> > would be a better fix?  Does this fix work for you?
>
> No, this doesn't help.  Firefox still barfs with to_utf8.
> 
> With esc_html the code generated is:
> 
> <a title="Search for commits authored by YOSHIFUJI Hideaki / <span class="cntrl">\e</span>$B5HF#1QL@<span class="cntrl">\e</span>(B" class="list" href="/?p=.git;a=search;h=f66ab685594d49e570b2176cfa20b03360e9a6e9;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a>

As you can see the HTML code generated with esc_html solution is way wrong
because of embedded '<span class="cntrl">\e</span>' as you see _without_
'"' being escaped, so HTML is wrong.

Nevertheless it shows what's the problem.  Somehow (perhaps wrong
encoding, perhaps screw up with quoted-printable and git-am, perhaps
copy'n' paste included ANSII color codes from terminal, perhaps something
different altogether) you got control characters (\e = ESC) in $author.
In strict XHTML mode (with 'application/xml

Please try the following patch

-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] gitweb: Harden format_search_author()

Protect format_search_author against control characters in $author.
While at it simplify it a bit, and use spaces for align.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   29 ++++++++++++++---------------
 1 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8b02767..ea9c09c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1856,23 +1856,22 @@ sub format_search_author {
 	my ($author, $searchtype, $displaytext) = @_;
 	my $have_search = gitweb_check_feature('search');
 
-	if ($have_search) {
-		my $performed = "";
-		if ($searchtype eq 'author') {
-			$performed = "authored";
-		} elsif ($searchtype eq 'committer') {
-			$performed = "committed";
-		}
-
-		return $cgi->a({-href => href(action=>"search", hash=>$hash,
-				searchtext=>$author,
-				searchtype=>$searchtype), class=>"list",
-				title=>"Search for commits $performed by $author"},
-				$displaytext);
+	return $displaytext unless ($have_search);
 
-	} else {
-		return $displaytext;
+	my $performed = "";
+	if ($searchtype eq 'author') {
+		$performed = "authored";
+	} elsif ($searchtype eq 'committer') {
+		$performed = "committed";
 	}
+
+	my $title = to_utf8("Search for commits $performed by $author");
+	$title =~ s/[[:cntrl:]]/?/g;
+
+	return $cgi->a({-href => href(action=>"search", hash=>$hash,
+	                              searchtext=>$author, searchtype=>$searchtype),
+	                -class=>"list", -title=>$title},
+	               $displaytext);
 }
 
 # format the author name of the given commit with the given tag
-- 
1.7.2.1