Re: gitweb bug: broken "next" and other links

Dnia poniedziałek 11. lutego 2008 14:33, Wincent Colaiuta napisał:

El 11/2/2008, a las 14:02, Jakub Narebski escribió:

quoted

Wincent Colaiuta [off-list ref] writes:

quoted

Just noticed a bug (possibly bugs) in gitweb.

Look at a shortlog page like this one:

 http://repo.or.cz/w/git.git?a=shortlog

Mouse over the "next" link at the bottom and you'll see this is the  
URL:

 http://repo.or.cz/w/ARRAY(0x85a5318)?a=shortlog;pg=1

Which obviously won't work...

This is bug in repo.or.cz version of gitweb, which is slightly
modified as compared to the "stock" version. Such error would be
catched by the gitweb 'run as standalone script and check stderr'
test script.

Hmm. I don't know. I can reproduce all three of those bugs on my own  
unmodified gitweb installation from 1.5.4.

I'm sorry. You are right. I haven't seen breakage because it shows
only when you use 'pathinfo' feature and pathinfo URLs.

Below there is a fix for that; actully only second part mentioned
(and first in patch) is needed, i.e. moving setting $params{'project'}
before dealing with -replay is needed I think to fix this bug.

Could you test it please?
-- >8 --
From: Jakub Narebski <redacted>
Subject: [PATCH] gitweb: Fix bug in href(..., -replay=>1) when 'pathinfo' feature used

URLs generated by href(..., -replay=>1) (which includes 'next page'
links and alternate view links) were not created correctly when using
'pathinfo' feature (i.e. using pathinfo instead of query string to
denote project / git repository used).

This resulted in broken links such like:
  http://www.example.com/w/ARRAY(0x85a5318)?a=shortlog;pg=1
instead of:
  http://www.example.com/w/project.git?a=shortlog;pg=1

This was caused by the fact that href() always replayed params in the
arrayref form, were they multivalued or singlevalued, and the code
dealing with 'pathinfo' feature didn't deal with $params{'project'}
being arrayref.  The code was improved to use arrayref only when
needed; because 'project' parameter should be always single-valued
this fixes this bug.

Additionally setting $params{'project'} is moved before replaying
params, just in case somebody handcraft evil URL like the one below:
  http://www.example.com/w/project.git?p=otherproject.git ...

Noticed-by: Peter Oberndorfer [off-list ref]
Noticed-by: Wincent Colaiuta [off-list ref]
Signed-off-by: Jakub Narebski <redacted>
---
 gitweb/gitweb.perl |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5e88637..648ee13 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -611,17 +611,22 @@ sub href(%) {
 	);
 	my %mapping = @mapping;
 
+	$params{'project'} = $project unless exists $params{'project'};
+
 	if ($params{-replay}) {
 		while (my ($name, $symbol) = each %mapping) {
 			if (!exists $params{$name}) {
-				# to allow for multivalued params we use arrayref form
-				$params{$name} = [ $cgi->param($symbol) ];
+				# for multivalued params we use arrayref form
+				my @par = $cgi->param($symbol);
+				if (@par > 1) {
+					$params{$name} = [ @par ];
+				} else {
+					$params{$name} = $par[0];
+				}
 			}
 		}
 	}
 
-	$params{'project'} = $project unless exists $params{'project'};
-
 	my ($use_pathinfo) = gitweb_check_feature('pathinfo');
 	if ($use_pathinfo) {
 		# use PATH_INFO for project name

-- 
1.5.4