{"thread":{"id":"36320","subject":"[PATCH] gitweb: Improve diffs when filenames contain problem characters","startedAt":"2014-03-31T02:05:11Z","lastAt":"2014-03-31T02:05:11Z","messageCount":1,"participants":["Andrew Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"238108","messageId":"1394DF8B-FD0D-4E5E-9BDE-79B487A725B6@kellerfarm.com","threadId":"36320","inReplyTo":null,"subject":"[PATCH] gitweb: Improve diffs when filenames contain problem characters","fromName":"Andrew Keller","fromEmail":"andrew@kellerfarm.com","sentAt":"2014-03-31T02:05:11Z","receivedAt":"2014-03-31T02:05:11Z","isPatch":true,"sender":{"key":"andrew@kellerfarm.com","avatar":"https://avatars.githubusercontent.com/u/304204?v=4"},"body":"When formatting a diff header line, be sure to escape the raw output from git\nfor use as HTML.  This ensures that when \"problem characters\" (&, <, >, ?, etc.)\nexist in filenames, gitweb displays them as text, rather than letting the\nbrowser interpret them as HTML.\n\nReported-by: Dongsheng Song <dongsheng.song@gmail.com>\nSigned-off-by: Andrew Keller <andrew@kellerfarm.com>\n---\nSteps to reproduce:\n\n1)  Create a repository that contains a commit that adds a file:\n    * with an ampersand in the filename\n    * with binary contents\n2)  Make the repository visible to gitweb\n3)  In gitweb, navigate to the page that shows the diff for the commit\n    that added the file\n\nBehavior without patch: Page stops rendering when it gets to one of\nthe instances of the filename (in the diff header, to be specific), and\na light-red box appears a the top of the page, saying something like\nthis:\n\n    This page contains the following errors:\n\n    error on line 67 at column 22: xmlParseEntityRef: no name\n\n    Below is a rendering of the page up to the first error.\n\n\n(This particular error is what you get with an unescaped ampersand)\n\nBehavior with patch: You see the ampersand in the file name, and the\npage renders as one would expect.\n\n\nOther notes:\n\nSeveral helper methods exist for escaping HTML, and I was unsure\nabout which one to use.  Although this patch fixes the problem,\nit is possible that it may be more correct to use, for example, the\n'esc_html' helper method instead of interacting with $cgi directly.\n\nThe first hunk in the diff seems to be all that's required to experience\nthe good behavior, however upon inspecting the code, it seems\nprudent to also include the second one.  It's a nearby section of code,\ndoing similar work, and is called from the same function as the\nformer, and with similar arguments.\n\n\nThanks,\nAndrew Keller\n\n\n gitweb/gitweb.perl |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 79057b7..6c559f8 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2223,6 +2223,8 @@ sub format_git_diff_header_line {\n \tmy $diffinfo = shift;\n \tmy ($from, $to) = @_;\n \n+\t$line = $cgi->escapeHTML($line);\n+\n \tif ($diffinfo->{'nparents'}) {\n \t\t# combined diff\n \t\t$line =~ s!^(diff (.*?) )\"?.*$!$1!;\n@@ -2259,6 +2261,8 @@ sub format_extended_diff_header_line {\n \tmy $diffinfo = shift;\n \tmy ($from, $to) = @_;\n \n+\t$line = $cgi->escapeHTML($line);\n+\n \t# match <path>\n \tif ($line =~ s!^((copy|rename) from ).*$!$1! && $from->{'href'}) {\n \t\t$line .= $cgi->a({-href=>$from->{'href'}, -class=>\"path\"},\n-- \n1.7.7.1\n"}]}