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

From: Jakub Narebski <jnareb@gmail.com>
Date: Feb 15, 2008, 21:16 UTC
Message-ID: <200802152216.42533.jnareb@gmail.com>
In-Reply-To: <E16BED2E-C146-44D8-BD90-ECF0DF89CA35@wincent.com>

On Tue, 12 Feb 2008, Wincent Colaiuta wrote:

> El 11/2/2008, a las 16:30, Jakub Narebski escribió:

>> 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.

Below there is minimal patch which I am using, which only moves setting $params{'project'}, and does not affect replay.

Show 8 quoted lines

>> Could you test it please?
> 
> Your patch fixes the "next" links in the shortlog and log views.
> 
> It doesn't fix the broken "raw" links in the commitdiff view. I'm  
> still seeing links like:
> 
> http://example.com/ARRAY(0x8c97f64)?a=commitdiff_plain;h=f29d56269a1c3bd4a970897397470f41553a64f9

WORKSFORME. I could not reproduce this error with the patch below applied. I think that the previous version of patch should give the same result; should also fix this bug.

Besides, both "next" and "raw" links are generated using the same mechanism. It would be strange if one of them broke and other didn't.

-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Date: Thu, 14 Feb 2008 09:22:30 +0100
Subject: [PATCH] gitweb: Fix bug in href(..., -replay=>1) when using 'pathinfo' form

URLs generated by href(..., -replay=>1) (which includes 'next page' links and alternate view links) didn't set project info correctly when current page URL is in pathinfo form.

This resulted in broken links such like:
  http://www.example.com/w/ARRAY(0x85a5318)?a=shortlog;pg=1
if the 'pathinfo' feature was used, or
  http://www.example.com/w/?a=shortlog;pg=1
if it wasn't, instead of correct:
  http://www.example.com/w/project.git?a=shortlog;pg=1

This was caused by the fact that href() always replays params in the arrayref form, were they multivalued or singlevalued, and the code dealing with 'pathinfo' feature couldn't deal with $params{'project'} being arrayref.

Setting $params{'project'} is moved before replaying params; this ensures that 'project' parameter is processed correctly.

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 |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5e88637..a89b478 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -611,6 +611,8 @@ 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}) {
@@ -620,8 +622,6 @@ sub href(%) {
 		}
 	}
 
-	$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