[PATCH 1/2] gitweb: Fix bug in evaluate_path_info

Jakub Narebski· Oct 13, 2010, 11:33 UTC · lore

There was bug in parsing "project/:/file" and "project/:/" path_info URLs, with implicit HEAD as 'hash_base'.

For such URLs the refname is empty, and before this fix regexp for parsing path_info fragment assumed that it is always non-empty.

Refname cannot contain ':', as per 'git check-ref-format'.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This was detected by t9500-gitweb-standalone-no-errors test script,
but only after applying second patch in this series.

Unfortunately it is not easy to detect that gitweb generated correct view; t9500 script only checks if Perl doesn't show any errors or warnings. That is why I have not added test for this issue (yet).

 gitweb/gitweb.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Show changes to gitweb/gitweb.perl +2 −2

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 15b5bb9..9116a0e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -780,10 +780,10 @@ sub evaluate_path_info {
 		'history',
 	);
 
-	# we want to catch
+	# we want to catch, among others
 	# [$hash_parent_base[:$file_parent]..]$hash_parent[:$file_name]
 	my ($parentrefname, $parentpathname, $refname, $pathname) =
-		($path_info =~ /^(?:(.+?)(?::(.+))?\.\.)?(.+?)(?::(.+))?$/);
+		($path_info =~ /^(?:(.+?)(?::(.+))?\.\.)?([^:]+?)?(?::(.+))?$/);
 
 	# first, analyze the 'current' part
 	if (defined $pathname) {

Jakub Narebski· Oct 13, 2010, 11:35 UTC · re: Jakub Narebski · lore

[PATCH 2/2] gitweb: Improve behavior for actionless path_info gitweb URLs

Eli Barzilay noticed that

  http://server/gitweb/project/<commit-sha1>

link goes to 'shortlog' view, while 'commit' view would be more useful, but that 'shortlog' action is more apropriate for

  http://server/gitweb/project/<commit-sha1>..<other-commit-sha1>

links.

Therefore for the case when we don't have either action, or filename, or parent hash [base] in path_info-based URL, i.e. for

  http://server/gitweb/project/<object-id>

link, instead of using 'shortlog' view we allow dispatch() subroutine to detect type of object and use appropriate action (in most case it would be either 'commit' action, or 'tag', or 'tree' for top directory).

Requested-by: Eli Barzilay <eli@barzilay.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Tested-by: Eli Barzilay <eli@barzilay.org>
---
This is one-line (discounting comments and braces) alternative
solution to the one proposed by Eli in the

  http://thread.gmane.org/gmane.comp.version-control.git/150527

thread from 8 July 2010.

A side-effect of this patch was discovering (and fixing in previous commit) long standing bug in corner case of path_info URLs parsing.

 gitweb/gitweb.perl |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Show changes to gitweb/gitweb.perl +9 −2

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9116a0e..928b5c0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -819,8 +819,15 @@ sub evaluate_path_info {
 		# hash_base instead. It should also be noted that hand-crafted
 		# links having 'history' as an action and no pathname or hash
 		# set will fail, but that happens regardless of PATH_INFO.
-		$input_params{'action'} ||= "shortlog";
-		if (grep { $_ eq $input_params{'action'} } @wants_base) {
+		if (defined $parentrefname) {
+			# if there is parent let the default be 'shortlog' action
+			# (for http://git.example.com/repo.git/A..B links); if there
+			# is no parent, dispatch will detect type of object and set
+			# action appropriately if required (if action is not set)
+			$input_params{'action'} ||= "shortlog";
+		}
+		if ($input_params{'action'} &&
+		    grep { $_ eq $input_params{'action'} } @wants_base) {
 			$input_params{'hash_base'} ||= $refname;
 		} else {
 			$input_params{'hash'} ||= $refname;

patch, 2 partsgitweb: Fix bug in evaluate_path_info