[PATCH] gitweb: Fix displaying unchopped argument in chop_and_escape_str

From: Jakub Narebski <jnareb@gmail.com>
Date: Feb 16, 2008, 22:07 UTC
Message-ID: <200802162307.47323.jnareb@gmail.com>
In-Reply-To: <7vve4o7jhz.fsf@gitster.siamese.dyndns.org>

Do not use esc_html to escape [title] _attribute_ of a HTML element, and quote unprintable characters. Replace unprintable characters by '?' and use CGI method to generate HTML element and do the escaping.

This caused bug noticed by Martin Koegler,
  Message-ID: <20080216130037.GA14571@auto.tuwien.ac.at>
that for bad commit encoding in author name, the title attribute (here
to show full, not shortened name) had embedded HTML code in it, result
of quoting unprintable characters the gitweb/HTML way. This of course
broke the HTML, causing page being not displayed in XML validating web
browsers.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

Junio C Hamano wrote:

Show 18 quoted lines

> Jakub Narebski <jnareb@gmail.com> writes:
>> Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:
>>
>>> http://repo.or.cz/w/alt-git.git?a=shortlog
>>> 
>>> fails to load in my Seamonkey browser (Debian stable):
>>> 
>>> XML Parsing Error: not well-formed
>>> Location: http://repo.or.cz/w/alt-git.git?a=shortlog
>>> Line Number 561, Column 33:<td><i><span title="Uwe Kleine-K<span class="cntrl">\e</span>,Av<span class="cntrl">\e</span>(Bnig">Uwe Kleine ...</span></i></td>
>>> --------------------------------^
>>
>> It looks like gitweb uses esc_html instead of esc_param (or leaving it
>> to CGI module) title attribute of span (?) element in a shortlog.
>>
>> I'd try to fix this bug.
> 
> Thanks.

And here it is. It fixes this bug; I hope there aren't any similar bugs, but I have not checked this.

Robert Schiele wrote:

Show 6 quoted lines

> On Sat, Feb 16, 2008 at 11:52:42AM -0800, Jakub Narebski wrote:
>> 
>> It looks like gitweb uses esc_html instead of esc_param (or leaving it
> 
> Huh?  Isn't that the wrong escaping?  esc_param is for URLs not for XML
> attributes in general, isn't it?

True, esc_param is for escaping values of CGI parameters, not for escaping (and quoting) attributes of HTML element.

P.S. I am sorely dissapointed by the fact that CGI version 3.10 doesn't do escaping / quoting of unprintable (control) characters in attributes (characters outside specified character set).

 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a89b478..acf155c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -866,8 +866,8 @@ sub chop_and_escape_str {
 	if ($chopped eq $str) {
 		return esc_html($chopped);
 	} else {
-		return qq{<span title="} . esc_html($str) . qq{">} .
-			esc_html($chopped) . qq{</span>};
+		$str =~ s/([[:cntrl:]])/?/g;
+		return $cgi->span({-title=>$str}, esc_html($chopped));
 	}
 }

-- 
1.5.4