Possible bug in revision link generation in human readable diff mode

[Michael Sims]

Hi,

I apologize in advance if this has been brought up here before; I searched the list
archives but couldn't find any reference to the problem I'm having.

It looks like there may be a bug in the way CVSweb generates links to specific
revisions when showing a human-readable, or colored diff (f=h).  This problem
appears in the latest cvsweb Debian package (3.0.2-1 in "testing") and also in the
latest available from the CVSweb site (3.0.4).  When viewing a colored diff between
two revisions in "Show all branches" mode, the various links to specific revisions
lack the question mark delimiter between the file path and the query string.  For
example, a link such as this may be generated:

http://host/cgi-bin/cvsweb/module/file.php;rev=1.15

instead of:

http://host/cgi-bin/cvsweb/module/file.php?;rev=1.15
or
http://host/cgi-bin/cvsweb/module/file.php?rev=1.15

The two main revision links are generated this way, as well as any links that point
to specific lines in various revisions.  Clicking on these links return a "no such
file or directory" error.

When viewing a colored diff on a branch this problem does not occur, because in that
case the link is structured like the following:

http://host/cgi-bin/cvsweb/module/file.php?only_with_tag=TAG;rev=1.15

It seems that when the only_with_tag parameter is propagated the question mark is
retained, which avoids the apparent bug.

I haven't had a chance to get a real understanding of the code, but this simple
patch seems to address the problem here:

diff -u cvsweb.cgi.orig cvsweb.cgi
--- cvsweb.cgi.orig     2004-11-16 14:55:48.000000000 -0600
+++ cvsweb.cgi  2004-11-16 14:56:44.000000000 -0600
@@ -3634,7 +3634,7 @@
   $date1 = defined($date1) ? ', ' . htmlquote($date1) : '';
   $date2 = defined($date2) ? ', ' . htmlquote($date2) : '';

-  my $link = uri_escape($filename) . $query;
+  my $link = uri_escape($filename) . ($query ? $query : '?');

   # Using <table style=\"border: none\" here breaks NS 4.x badly...
   print <<EOF;

If a CVS tag has been selected then the $query variable will contain the needed
question mark, otherwise it will be empty.  My patch above just makes sure the
question mark is present if $query is empty.  This seems to match some similar
constructs elsewhere in the code.

So, does this appear to be a bug, or is there something wrong with my
setup/configuration/expectations?  It looks like this was introduced in CVS revision
1.219 of cvsweb.cgi.  Thanks in advance...

___________________________________________
Michael Sims
Project Analyst - Information Technology
Crye-Leike Realtors
___________________________________________