Re: [PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:27

Jay Soffian [off-list ref] writes:
When gitweb is used as a DirectoryIndex, it attempts to strip
PATH_INFO on its own, as $cgi->url() fails to do so.

However, it fails to account for the fact that PATH_INFO has
already been URL-decoded by the web server, but the value
returned by $cgi->url() has not been. This causes the stripping
to fail whenever the URL contains encoded characters.

To see this in action, setup gitweb as a DirectoryIndex and
then use it on a repository with a directory containing a
space in the name. Navigate to tree view, examine the gitweb
generated html and you'll see a link such as:

  <a href="/test.git/tree/HEAD:/directory with spaces">directory with spaces</a>

When clicked on, the browser will URL-encode this link, giving
a $cgi->url() of the form:

   /test.git/tree/HEAD:/directory%20with%20spaces

While PATH_INFO is:

   /test.git/tree/HEAD:/directory with spaces

Fix this by calling unescape() on both $my_url and $my_uri before
stripping PATH_INFO from them.

Signed-off-by: Jay Soffian <redacted>
---
Thanks.  From a cursory look, with the help from the explanation in
the proposed commit log message, the change looks sensible.

I wonder if a breakage like this is something we can catch in one of
the t95xx series of tests, though.

Jakub, Ack?
quoted hunk
 gitweb/gitweb.perl | 5 +++++
 1 file changed, 5 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3d6a705388..7f8c1878d4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -54,6 +54,11 @@ sub evaluate_uri {
 	# to build the base URL ourselves:
 	our $path_info = decode_utf8($ENV{"PATH_INFO"});
 	if ($path_info) {
+		# $path_info has already been URL-decoded by the web server, but
+		# $my_url and $my_uri have not. URL-decode them so we can properly
+		# strip $path_info.
+		$my_url = unescape($my_url);
+		$my_uri = unescape($my_uri);
 		if ($my_url =~ s,\Q$path_info\E$,, &&
 		    $my_uri =~ s,\Q$path_info\E$,, &&
 		    defined $ENV{'SCRIPT_NAME'}) {

Re: [PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:54:29

On Thu, 9 Aug 2012, Junio C Hamano wrote:
Jay Soffian [off-list ref] writes:
quoted
When gitweb is used as a DirectoryIndex, it attempts to strip
PATH_INFO on its own, as $cgi->url() fails to do so.

However, it fails to account for the fact that PATH_INFO has
already been URL-decoded by the web server, but the value
returned by $cgi->url() has not been. This causes the stripping
to fail whenever the URL contains encoded characters.

To see this in action, setup gitweb as a DirectoryIndex and
then use it on a repository with a directory containing a
space in the name. Navigate to tree view, examine the gitweb
generated html and you'll see a link such as:

  <a href="/test.git/tree/HEAD:/directory with spaces">directory with spaces</a>

When clicked on, the browser will URL-encode this link, giving
a $cgi->url() of the form:

   /test.git/tree/HEAD:/directory%20with%20spaces

While PATH_INFO is:

   /test.git/tree/HEAD:/directory with spaces

Fix this by calling unescape() on both $my_url and $my_uri before
stripping PATH_INFO from them.

Signed-off-by: Jay Soffian <redacted>
---
Thanks.  From a cursory look, with the help from the explanation in
the proposed commit log message, the change looks sensible.

I wonder if a breakage like this is something we can catch in one of
the t95xx series of tests, though.
No, it is unfortunately not possible with current test infrastructure
for gitweb.  The gitweb_run from t/gitweb-lib.sh allows to set
PATH_INFO and QUERY_STRING, but does not allow to set up URL.

That might change in the future...
Jakub, Ack?
Acked-by: Jakub Narebski <redacted>

Uf ut us bot too late...


-- 
Jakub Narebski
Poland
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help