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

[Jakub Narebski <jnareb@gmail.com>]

Dnia poniedziałek 11. lutego 2008 14:33, Wincent Colaiuta napisał:
> El 11/2/2008, a las 14:02, Jakub Narebski escribió:
>> Wincent Colaiuta <win@wincent.com> writes:
>>
>>> 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 <jnareb@gmail.com>
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 <kumbayo84@arcor.de>
Noticed-by: Wincent Colaiuta <win@wincent.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 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