Re: gitweb commitdiff page - binary files with ampersands in filename?
- From
Jakub Narębski <jnareb@gmail.com>
- Date
- Apr 15, 2013, 21:42 UTC
- Message-ID
- <516C7435.7050203@gmail.com>
- In-Reply-To
- <CAKzCUUH4m8Hn_zcE_eHB9YBjDQWPAiX17HCOVZ80f+Tx5EVdOg@mail.gmail.com>
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