threads / discuss / 33448

gitweb commitdiff page - binary files with ampersands in filename?

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

## tl;dr

2 messages between Apr 9, 2013 and Apr 15, 2013.

replies: 1people: 2as markdown or json

Oj W· Apr 9, 2013, 17:03 UTC · lore

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")

Jakub Narębski· Apr 15, 2013, 21:42 UTC · re: Oj W · lore

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

Oj W wrote:
Show 11 quoted lines
> 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

← back to recent threads