gitweb bug: broken "next" and other links

gitweb bug: broken "next" and other links

[Wincent Colaiuta]

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

Same if you look at the "log" view:

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

There the "next" links point at URLs like:

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

But it's not only "next" links that are broken. Some random clicking  
around discovered ones like this:

Looking at a commitdiff, eg:

http://repo.or.cz/w/git.git?a=commitdiff;h=94bf9f7c37cca0241785a5f4e54e5cc98e175244

The "raw" link is:

http://repo.or.cz/w/ARRAY(0x8579c0c)?a=commitdiff_plain;h=94bf9f7c37cca0241785a5f4e54e5cc98e175244

There may be others, but that's what I've found so far.

Cheers,
Wincent

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

[Jakub Narebski]

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.

I have CC-ed Pasky.
-- 
Jakub Narebski
Poland
ShadeHawk on #git

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

[Wincent Colaiuta]

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.

Cheers,
Wincent

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

[Jakub Narebski]

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

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

[Jakub Narebski]

On Mon, 11 Feb 2008, Jakub Narebski wrote:

> 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

Actually the error is there even without using 'pathinfo' feature,
namely if you use pathinfo to provide project, for example:
  http://www.example.com/w/project.git?a=shortlog
then 'next' link on the page lacks project (instead of having
dereferenced anonymous array reference, i.e. ARRAY(sth)), like below
  http://www.example.com/w/?a=shortlog;pg=1

But I could not come up with automated test which would _fail_
on this error. So please check, and reply if this patch helps.


Below there is hand-crafted minimal fixing patch.

> 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,17 @@ 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) ];
>  			}
>  		}
>  	}
>  
> -	$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
> 
> 

-- 
Jakub Narebski
Poland

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

[Jakub Narebski]

Jakub Narebski wrote:
> Actually the error is there even without using 'pathinfo' feature,
> namely if you use pathinfo to provide project, for example:
>   http://www.example.com/w/project.git?a=shortlog
> then 'next' link on the page lacks project (instead of having
> dereferenced anonymous array reference, i.e. ARRAY(sth)), like below
>   http://www.example.com/w/?a=shortlog;pg=1

Errr... what I forgot to write here is that I have checked that
the patch send (the first version) fixes this bug, as checked
"by hand", i.e. by examining gitweb running as Apache CGI module.

> But I could not come up with automated test which would _fail_
> on this error. So please check, and reply if this patch helps.

-- 
Jakub Narebski
Poland

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

[Wincent Colaiuta]

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

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

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

Cheers,
Wincent

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

[Jakub Narebski]

Dnia wtorek 12. lutego 2008 13:24, Wincent Colaiuta napisał:
> El 11/2/2008, a las 16:30, Jakub Narebski escribió:
>>
>> 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?
> 
> 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

Could yu please give an URL for the page containing broken
link, relevant parts of gitweb configuration (gitweb_config.perl),
and gitweb parts of repository configuration, if there are any?

This would help in reproducing this bug, and correcting it.
-- 
Jakub Narebski
Poland

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

[Wincent Colaiuta]

El 12/2/2008, a las 14:10, Jakub Narebski escribió:

> Dnia wtorek 12. lutego 2008 13:24, Wincent Colaiuta napisał:
>> El 11/2/2008, a las 16:30, Jakub Narebski escribió:
>>>
>>> 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?
>>
>> 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
>
> Could yu please give an URL for the page containing broken
> link

Any commitdiff page on repo.or.cz:

eg.

http://repo.or.cz/w/git.git?a=commitdiff;h=94bf9f7c37cca0241785a5f4e54e5cc98e175244

Any commitdiff page on my own gitweb install:

eg.

http://git.wincent.com/wikitext.git?a=commitdiff;h=f29d56269a1c3bd4a970897397470f41553a64f9

> relevant parts of gitweb configuration (gitweb_config.perl),
> and gitweb parts of repository configuration, if there are any?

Obviously I can't comment about the repo.or.cz config, but in my local  
install gitweb was built with:

make prefix=/usr/local \
  GITWEB_PROJECTROOT=/pub/git/path_to_public_repos \
  GITWEB_LIST=/pub/git/conf/gitweb-projects \
  GITWEB_STRICT_EXPORT=1 \
  GITWEB_CSS="/gitweb.css" \
  GITWEB_LOGO="/git-logo.png" \
  GITWEB_FAVICON="/git-favicon.png" \
  GITWEB_CONFIG="/pub/git/conf/gitweb.conf" \
  gitweb/gitweb.cgi

And gitweb.conf contains:

$feature{'search'}{'default'} = [undef];
$feature{'blame'}{'default'} = [undef];
$feature{'pickaxe'}{'default'} = [undef];
$feature{'grep'}{'default'} = [undef];
$feature{'pathinfo'}{'default'} = [1];
$my_uri = "http://git.wincent.com";
$home_link = "http://git.wincent.com/";
$site_name = "git.wincent.com";

So all pretty stock standard, I think.

Cheers,
Wincent

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

[Jakub Narebski]

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.

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

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

[Wincent Colaiuta]

El 15/2/2008, a las 22:16, Jakub Narebski escribió:

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

Just re-tested again, this time applying on top of 1.5.4.2 and  
definitely doesn't work for me.

The "next" links are fixed, the "raw" links still exhibit the same  
problem.

eg. here's a commitdiff page with a broken "raw" link from my  
installation:

http://git.wincent.com/wikitext.git?a=commitdiff;h=03b8fda6594aa7e6fab3b95b66a4f3c178d6a156

And here's a shortlog page with now-working "next" links:

http://git.wincent.com/wikitext.git?a=shortlog;h=03b8fda6594aa7e6fab3b95b66a4f3c178d6a156

Cheers,
Wincent

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

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> ...
> Just re-tested again, this time applying on top of 1.5.4.2 and  
> definitely doesn't work for me.

<tongue-in-cheek>

Gee, don't you people have anything better to do than tracking
just-off-the-press git updates?

</tongue-in-cheek>

Thanks for your continued support and bug reports ;-).

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

[Jakub Narebski]

Dnia niedziela 17. lutego 2008 11:51, Wincent Colaiuta napisał:
> El 15/2/2008, a las 22:16, Jakub Narebski escribió:
> >
> > 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.
> 
> Just re-tested again, this time applying on top of 1.5.4.2 and  
> definitely doesn't work for me.
> 
> The "next" links are fixed, the "raw" links still exhibit the same  
> problem.
> 
> eg. here's a commitdiff page with a broken "raw" link from my  
> installation:
> 
> http://git.wincent.com/wikitext.git?a=commitdiff;h=03b8fda6594aa7e6fab3b95b66a4f3c178d6a156
> 
> And here's a shortlog page with now-working "next" links:
> 
> http://git.wincent.com/wikitext.git?a=shortlog;h=03b8fda6594aa7e6fab3b95b66a4f3c178d6a156

Could you reproduce this error with git.git repository (after any of
the two patches, but please tell which)? Please provide an URL to the
page, and fragment of HTML with wrong link.

BTW. I have just checked above link, and the "raw" link is most
certainly not broken. Were you using updated gitweb (with patch),
and reloaded page?


Junio, would you consider applying one of those patches? Any of them
should fix the bug; the second one is less intrusive.
-- 
Jakub Narebski
Poland

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

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Junio, would you consider applying one of those patches? Any of them
> should fix the bug; the second one is less intrusive.

Surely.

By "the second one" do you mean this one?

    Subject: [PATCH] gitweb: Add new option -nohtml to quot_xxx subroutines
    Date: Sun, 17 Feb 2008 18:48:13 +0100
    Message-ID: <20080217174812.30454.86822.stgit@localhost.localdomain>

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

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Junio, would you consider applying one of those patches? Any of them
> > should fix the bug; the second one is less intrusive.
> 
> Surely.
> 
> By "the second one" do you mean this one?
> 
>     Subject: [PATCH] gitweb: Add new option -nohtml to quot_xxx subroutines
>     Date: Sun, 17 Feb 2008 18:48:13 +0100
>     Message-ID: <20080217174812.30454.86822.stgit@localhost.localdomain>

The above is purely meant to further enhance 850b90a51d5
  gitweb: Fix displaying unchopped argument in chop_and_escape_str


No, I meant:
  Subject: [PATCH] gitweb: Fix bug in href(..., -replay=>1) when 'pathinfo' feature used
  Date: Thu, 14 Feb 2008 09:22:30 +0100
  Message-Id: <200802152216.42533.jnareb@gmail.com>

It is inside other email (Re: gitweb bug: broken "next" and other links),
so I can resend it...

-- 
Jakub Narebski
Poland

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

[Wincent Colaiuta]

El 19/2/2008, a las 0:32, Jakub Narebski escribió:

> Dnia niedziela 17. lutego 2008 11:51, Wincent Colaiuta napisał:
>> El 15/2/2008, a las 22:16, Jakub Narebski escribió:
>>>
>>> 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.
>>
>> Just re-tested again, this time applying on top of 1.5.4.2 and
>> definitely doesn't work for me.
>>
>> The "next" links are fixed, the "raw" links still exhibit the same
>> problem.
>>
>> eg. here's a commitdiff page with a broken "raw" link from my
>> installation:
>>
>> http://git.wincent.com/wikitext.git?a=commitdiff;h=03b8fda6594aa7e6fab3b95b66a4f3c178d6a156
>>
>> And here's a shortlog page with now-working "next" links:
>>
>> http://git.wincent.com/wikitext.git?a=shortlog;h=03b8fda6594aa7e6fab3b95b66a4f3c178d6a156
>
> Could you reproduce this error with git.git repository (after any of
> the two patches, but please tell which)? Please provide an URL to the
> page, and fragment of HTML with wrong link.
>
> BTW. I have just checked above link, and the "raw" link is most
> certainly not broken. Were you using updated gitweb (with patch),
> and reloaded page?

Well I'll be darned. Looks like my browser (Safari) was showing me an  
older version of the commitdiff page. Crazy browser.

I had confirmed that both links were not working, then applied your  
patch, then reloaded both pages (or so I thought) and saw that the  
commitdiff page was still broken.

I went through this cycle at least twice (once when you first the  
patch, and again days later when I reapplied and retested) and both  
times got the same results.

But now having left the server alone (patch still applied) I revisit  
the link and you're right: it works.

Sorry for the confusion and thanks for fixing the bug. Darn browser.

Cheers,
Wincent