Re: [PATCH] gitweb: filter escapes from longer commit titles that break firefox

From: Jakub Narebski <jnareb@gmail.com>
Date: Apr 25, 2009, 09:04 UTC
Message-ID: <200904251104.45236.jnareb@gmail.com>
In-Reply-To: <200904250010.46299.jnareb@gmail.com>

On Sat, 25 April 2009, Jakub Narebski wrote:

> So it is not that simple...

That said, here is simple patch which should fix the bug you found. It always creates sensible short and long values, contrary to your patch (take a look at gitweb output after your patch, including tooltips on mouseover).

But it is NOT TESTED if it works correctly, and if it covers all occurrences. And it might be not necessary in all its complication: we could simply replace control characters by '?' like in chop_and_escape_str subroutine (which would also make gitweb more consistent). It also lacks commit message.

Nevertheless it might be good bandaid for your problem:

-- >8 --

diff --git c/gitweb/gitweb.perl w/gitweb/gitweb.perl
index 3f99361..8575d5f 100755
--- c/gitweb/gitweb.perl
+++ w/gitweb/gitweb.perl
@@ -1035,6 +1035,24 @@ sub esc_url {
 	return $str;
 }
 
+# quote and escape tag attribute values; autoEscape has to be turned off
+sub esc_attr {
+	my $str = shift;
+	return $str unless defined $str;
+
+	my %ent = ( # named HTML entities
+		'"' => '&quot;',
+		'&' => '&amp;',
+		'<' => '&lt;',
+		'>' => '&gt;',
+	);
+	$str = to_utf8($str);
+	$str =~ s|([\"&<>])|$ent{$1}|eg;
+	$str =~ s|([[:cntrl:]])|(($1 ne "\t") ? quot_upr($1) : $1)|eg;
+
+	return $str;
+}
+
 # replace invalid utf8 character with SUBSTITUTION sequence
 sub esc_html ($;%) {
 	my $str = shift;
@@ -1457,14 +1475,19 @@ sub format_subject_html {
 	my ($long, $short, $href, $extra) = @_;
 	$extra = '' unless defined($extra);
 
+	my $ret = '';
 	if (length($short) < length($long)) {
-		return $cgi->a({-href => $href, -class => "list subject",
-		                -title => to_utf8($long)},
+		my $autoescape = $cgi->autoEscape(undef);
+		# or just replace s/([[:cntrl:]])/?/g in -title
+		$ret = $cgi->a({-href => $href, -class => "list subject",
+		                -title => esc_attr($long)},
 		       esc_html($short) . $extra);
+		$cgi->autoEscape($autoescape); # restore original value
 	} else {
-		return $cgi->a({-href => $href, -class => "list subject"},
+		$ret = $cgi->a({-href => $href, -class => "list subject"},
 		       esc_html($long)  . $extra);
 	}
+	return $ret;
 }
 
 # format git diff header line, i.e. "diff --(git|combined|cc) ..."