Skip to content

Commit

Permalink
gitweb: Fix handling of whitespace in generated links
Browse files Browse the repository at this point in the history
When creating path_info part of link, don't encode space as '+', because
while $cgi->param('foo') translates '+' in query param to ' ', neither
$ENV{'PATH_INFO'} nor $cgi->path_info() do.

This fixes the issue with pathnames with embedded whitespace and
$feature{'pathinfo'} / path_info links.  It is done by using newly
introduced esc_path_info() instead of esc_url() in href() subroutine.

Also while links are more clear not escaping space (' ') characters in
generated links, the trailing space must be URI-encoded, otherwise would
get discarded.

Issue noticed thanks to John 'Warthog9' Hawley.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jakub Narebski authored and Junio C Hamano committed Dec 14, 2010
1 parent c6d059b commit 67976c6
Showing 1 changed file with 23 additions and 8 deletions.
31 changes: 23 additions & 8 deletions gitweb/gitweb.perl
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ sub href {
$href =~ s,/$,,;

# Then add the project name, if present
$href .= "/".esc_url($params{'project'});
$href .= "/".esc_path_info($params{'project'});
delete $params{'project'};

# since we destructively absorb parameters, we keep this
Expand All @@ -1196,7 +1196,8 @@ sub href {
# Summary just uses the project path URL, any other action is
# added to the URL
if (defined $params{'action'}) {
$href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary';
$href .= "/".esc_path_info($params{'action'})
unless $params{'action'} eq 'summary';
delete $params{'action'};
}

Expand All @@ -1206,33 +1207,33 @@ sub href {
|| $params{'hash_parent'} || $params{'hash'});
if (defined $params{'hash_base'}) {
if (defined $params{'hash_parent_base'}) {
$href .= esc_url($params{'hash_parent_base'});
$href .= esc_path_info($params{'hash_parent_base'});
# skip the file_parent if it's the same as the file_name
if (defined $params{'file_parent'}) {
if (defined $params{'file_name'} && $params{'file_parent'} eq $params{'file_name'}) {
delete $params{'file_parent'};
} elsif ($params{'file_parent'} !~ /\.\./) {
$href .= ":/".esc_url($params{'file_parent'});
$href .= ":/".esc_path_info($params{'file_parent'});
delete $params{'file_parent'};
}
}
$href .= "..";
delete $params{'hash_parent'};
delete $params{'hash_parent_base'};
} elsif (defined $params{'hash_parent'}) {
$href .= esc_url($params{'hash_parent'}). "..";
$href .= esc_path_info($params{'hash_parent'}). "..";
delete $params{'hash_parent'};
}

$href .= esc_url($params{'hash_base'});
$href .= esc_path_info($params{'hash_base'});
if (defined $params{'file_name'} && $params{'file_name'} !~ /\.\./) {
$href .= ":/".esc_url($params{'file_name'});
$href .= ":/".esc_path_info($params{'file_name'});
delete $params{'file_name'};
}
delete $params{'hash'};
delete $params{'hash_base'};
} elsif (defined $params{'hash'}) {
$href .= esc_url($params{'hash'});
$href .= esc_path_info($params{'hash'});
delete $params{'hash'};
}

Expand Down Expand Up @@ -1265,6 +1266,9 @@ sub href {
}
$href .= "?" . join(';', @result) if scalar @result;

# final transformation: trailing spaces must be escaped (URI-encoded)
$href =~ s/(\s+)$/CGI::escape($1)/e;

return $href;
}

Expand Down Expand Up @@ -1347,6 +1351,17 @@ sub esc_param {
return $str;
}

# the quoting rules for path_info fragment are slightly different
sub esc_path_info {
my $str = shift;
return undef unless defined $str;

# path_info doesn't treat '+' as space (specially), but '?' must be escaped
$str =~ s/([^A-Za-z0-9\-_.~();\/;:@&= +]+)/CGI::escape($1)/eg;

return $str;
}

# quote unsafe chars in whole URL, so some characters cannot be quoted
sub esc_url {
my $str = shift;
Expand Down

0 comments on commit 67976c6

Please sign in to comment.