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@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxx> --- Junio C Hamano wrote: > Jakub Narebski <jnareb@xxxxxxxxx> writes: >> Martin Koegler <mkoegler@xxxxxxxxxxxxxxxxx> 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: > 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 - 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
