{"thread":{"id":"33448","subject":"gitweb commitdiff page - binary files with ampersands in filename?","startedAt":"2013-04-09T17:03:33Z","lastAt":"2013-04-15T21:42:13Z","messageCount":2,"participants":["Oj W","Jakub Narębski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"213674","messageId":"CAKzCUUH4m8Hn_zcE_eHB9YBjDQWPAiX17HCOVZ80f+Tx5EVdOg@mail.gmail.com","threadId":"33448","inReplyTo":null,"subject":"gitweb commitdiff page - binary files with ampersands in filename?","fromName":"Oj W","fromEmail":"ojwlists@gmail.com","sentAt":"2013-04-09T17:03:33Z","receivedAt":"2013-04-09T17:03:33Z","isPatch":false,"sender":{"key":"ojwlists@gmail.com","avatar":null},"body":"Change a binary file whose filename contains an ampersand, then view\nthe commitdiff page in gitweb.\n\nGit outputs a message like \"Binary files a/b&w.dll and b/b&w.dll differ\"\n\nGitweb format_diff_from_to_header() doesn't notice anything in that\noutput which needs escaping, and writes it directly to the XHTML 1.0\nStrict output.\n\nThen gitweb's output is invalid XML, meaning that browsers such as\nFirefox will refuse to display the page.\n\n(tested with v 1.7.9.5, but I can't see anything in latest\nhttps://github.com/git/git/blob/master/gitweb/gitweb.perl#CL2158 which\nis looking for text like \"Binary files ... differ\")\n"},{"id":"214391","messageId":"516C7435.7050203@gmail.com","threadId":"33448","inReplyTo":"CAKzCUUH4m8Hn_zcE_eHB9YBjDQWPAiX17HCOVZ80f+Tx5EVdOg@mail.gmail.com","subject":"Re: gitweb commitdiff page - binary files with ampersands in filename?","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2013-04-15T21:42:13Z","receivedAt":"2013-04-15T21:42:13Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Oj W wrote:\n\n> Change a binary file whose filename contains an ampersand, then view\n> the commitdiff page in gitweb.\n> \n> Git outputs a message like \"Binary files a/b&w.dll and b/b&w.dll differ\"\n> \n> Gitweb format_diff_from_to_header() doesn't notice anything in that\n> output which needs escaping, and writes it directly to the XHTML 1.0\n> Strict output.\n> \n> Then gitweb's output is invalid XML, meaning that browsers such as\n> Firefox will refuse to display the page.\n\nThis was because in case of binary files we don't get usual diff,\njust \"Binary files a/foo and b/foo differ\" just after extended git diff \nheaders.\n\nThere are two problems with gitweb code.  First is that git_patchset_body()\ndidn't recognize and handle this situation.  This should be fixed by the\npatch below (I have trouble testing it as git-instaweb keeps using old\ngitweb version for some reason).\n\nSecond is that format_diff_from_to_header() doesn't handle unrecognized\nextended git diff headers well - it doesn't HTML escape them.  This is to\nbe fixed.\n\n-- >8 --\n---\n gitweb/gitweb.perl |   10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1309196..33a0de1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5345,7 +5345,8 @@ sub git_patchset_body {\n \t\twhile ($patch_line = <$fd>) {\n \t\t\tchomp $patch_line;\n \n-\t\t\tlast EXTENDED_HEADER if ($patch_line =~ m/^--- |^diff /);\n+\t\t\tlast EXTENDED_HEADER\n+\t\t\t\tif ($patch_line =~ m/^--- |^diff |^Binary files .* differ$/);\n \n \t\t\tprint format_extended_diff_header_line($patch_line, $diffinfo,\n \t\t\t                                       \\%from, \\%to);\n@@ -5357,7 +5358,12 @@ sub git_patchset_body {\n \t\t\tprint \"</div>\\n\"; # class=\"patch\"\n \t\t\tlast PATCH;\n \t\t}\n-\t\tnext PATCH if ($patch_line =~ m/^diff /);\n+\t\tif ($patch_line =~ m/^Binary files .* differ$/) {\n+\t\t\tprint \"<div class=\\\"diff bin\\\">\" .\n+\t\t\t      esc_html($patch_line, -nbsp => 1) .\n+\t\t\t      \"</div>\\n\";\n+\t\t}\n+\t\tnext PATCH if ($patch_line =~ m/^diff |^Binary files .* differ$/);\n \t\t#assert($patch_line =~ m/^---/) if DEBUG;\n \n \t\tmy $last_patch_line = $patch_line;\n-- \n1.7.10.4\n"}]}