gitweb commitdiff page - binary files with ampersands in filename?

gitweb commitdiff page - binary files with ampersands in filename?

[Oj W]

Change a binary file whose filename contains an ampersand, then view
the commitdiff page in gitweb.

Git outputs a message like "Binary files a/b&w.dll and b/b&w.dll differ"

Gitweb format_diff_from_to_header() doesn't notice anything in that
output which needs escaping, and writes it directly to the XHTML 1.0
Strict output.

Then gitweb's output is invalid XML, meaning that browsers such as
Firefox will refuse to display the page.

(tested with v 1.7.9.5, but I can't see anything in latest
https://github.com/git/git/blob/master/gitweb/gitweb.perl#CL2158 which
is looking for text like "Binary files ... differ")

Re: gitweb commitdiff page - binary files with ampersands in filename?

[Jakub Narębski]

Oj W wrote:

> Change a binary file whose filename contains an ampersand, then view
> the commitdiff page in gitweb.
> 
> Git outputs a message like "Binary files a/b&w.dll and b/b&w.dll differ"
> 
> Gitweb format_diff_from_to_header() doesn't notice anything in that
> output which needs escaping, and writes it directly to the XHTML 1.0
> Strict output.
> 
> Then gitweb's output is invalid XML, meaning that browsers such as
> Firefox will refuse to display the page.

This was because in case of binary files we don't get usual diff,
just "Binary files a/foo and b/foo differ" just after extended git diff 
headers.

There are two problems with gitweb code.  First is that git_patchset_body()
didn't recognize and handle this situation.  This should be fixed by the
patch below (I have trouble testing it as git-instaweb keeps using old
gitweb version for some reason).

Second is that format_diff_from_to_header() doesn't handle unrecognized
extended git diff headers well - it doesn't HTML escape them.  This is to
be fixed.

-- >8 --
---
 gitweb/gitweb.perl |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1309196..33a0de1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5345,7 +5345,8 @@ sub git_patchset_body {
 		while ($patch_line = <$fd>) {
 			chomp $patch_line;
 
-			last EXTENDED_HEADER if ($patch_line =~ m/^--- |^diff /);
+			last EXTENDED_HEADER
+				if ($patch_line =~ m/^--- |^diff |^Binary files .* differ$/);
 
 			print format_extended_diff_header_line($patch_line, $diffinfo,
 			                                       \%from, \%to);
@@ -5357,7 +5358,12 @@ sub git_patchset_body {
 			print "</div>\n"; # class="patch"
 			last PATCH;
 		}
-		next PATCH if ($patch_line =~ m/^diff /);
+		if ($patch_line =~ m/^Binary files .* differ$/) {
+			print "<div class=\"diff bin\">" .
+			      esc_html($patch_line, -nbsp => 1) .
+			      "</div>\n";
+		}
+		next PATCH if ($patch_line =~ m/^diff |^Binary files .* differ$/);
 		#assert($patch_line =~ m/^---/) if DEBUG;
 
 		my $last_patch_line = $patch_line;
-- 
1.7.10.4