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@xxxxxxxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html