When formatting a diff header line, be sure to escape the raw output from git
for use as HTML. This ensures that when "problem characters" (&, <, >, ?, etc.)
exist in filenames, gitweb displays them as text, rather than letting the
browser interpret them as HTML.
Reported-by: Dongsheng Song <dongsheng.song@xxxxxxxxx>
Signed-off-by: Andrew Keller <andrew@xxxxxxxxxxxxxx>
---
Steps to reproduce:
1) Create a repository that contains a commit that adds a file:
* with an ampersand in the filename
* with binary contents
2) Make the repository visible to gitweb
3) In gitweb, navigate to the page that shows the diff for the commit
that added the file
Behavior without patch: Page stops rendering when it gets to one of
the instances of the filename (in the diff header, to be specific), and
a light-red box appears a the top of the page, saying something like
this:
This page contains the following errors:
error on line 67 at column 22: xmlParseEntityRef: no name
Below is a rendering of the page up to the first error.
(This particular error is what you get with an unescaped ampersand)
Behavior with patch: You see the ampersand in the file name, and the
page renders as one would expect.
Other notes:
Several helper methods exist for escaping HTML, and I was unsure
about which one to use. Although this patch fixes the problem,
it is possible that it may be more correct to use, for example, the
'esc_html' helper method instead of interacting with $cgi directly.
The first hunk in the diff seems to be all that's required to experience
the good behavior, however upon inspecting the code, it seems
prudent to also include the second one. It's a nearby section of code,
doing similar work, and is called from the same function as the
former, and with similar arguments.
Thanks,
Andrew Keller
gitweb/gitweb.perl | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 79057b7..6c559f8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2223,6 +2223,8 @@ sub format_git_diff_header_line {
my $diffinfo = shift;
my ($from, $to) = @_;
+ $line = $cgi->escapeHTML($line);
+
if ($diffinfo->{'nparents'}) {
# combined diff
$line =~ s!^(diff (.*?) )"?.*$!$1!;
@@ -2259,6 +2261,8 @@ sub format_extended_diff_header_line {
my $diffinfo = shift;
my ($from, $to) = @_;
+ $line = $cgi->escapeHTML($line);
+
# match <path>
if ($line =~ s!^((copy|rename) from ).*$!$1! && $from->{'href'}) {
$line .= $cgi->a({-href=>$from->{'href'}, -class=>"path"},
--
1.7.7.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html