[RFC/PATCH v2] gitweb: check given hash before trying to create snapshot

[RFC/PATCH v2] gitweb: check given hash before trying to create snapshot

[Mark Rada]

I squashed the two commits together and I also used Jakub's idea to
create a wrapper for git_get_head_hash.

--
Mark Rada (ferrous26)
marada@uwaterloo.ca


--->8---
Make things nicer in cases when you hand craft the snapshot URL but
make a typo in defining the hash variable (e.g. netx instead of next);
you will now get an error message instead of a broken tarball.

To maintain backwards compatibility, git_get_head_hash is now a wrapper
for git_get_full_hash, as suggested by Jakub Narebski.

Tests for t9501 are included to demonstrate changed functionality.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
 gitweb/gitweb.perl                       |   13 ++++++++++---
 t/t9501-gitweb-standalone-http-status.sh |   29 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b2193..a40a50d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,11 +1983,17 @@ sub quote_command {
 
 # get HEAD ref of given project as hash
 sub git_get_head_hash {
+	return git_get_full_hash(shift, 'HEAD');
+}
+
+# given a project and tree-ish, returns full hash
+sub git_get_full_hash {
 	my $project = shift;
+	my $hash = shift;
 	my $o_git_dir = $git_dir;
 	my $retval = undef;
 	$git_dir = "$projectroot/$project";
-	if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
+	if (open my $fd, '-|', git_cmd(), 'rev-parse', '--verify', $hash) {
 		my $head = <$fd>;
 		close $fd;
 		if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
@@ -5196,8 +5202,9 @@ sub git_snapshot {
 		die_error(403, "Unsupported snapshot format");
 	}
 
-	if (!defined $hash) {
-		$hash = git_get_head_hash($project);
+	my $snapshot = git_get_full_hash($project, $hash);
+	if (!$snapshot) {
+		die_error(400, "Not a valid hash id: $hash");
 	}
 
 	my $name = $project;
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index d0ff21d..81abfae 100644
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -75,4 +75,33 @@ test_expect_success \
 test_debug 'cat gitweb.output'
 
 
+# ----------------------------------------------------------------------
+# snapshot hash ids
+
+test_expect_success \
+	'snapshots: bad treeish id' \
+	'gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
+	grep "400 - Not a valid hash id:" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+	'snapshots: good treeish id' \
+	'gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+	'snapshots: bad object id' \
+	'gitweb_run "p=.git;a=snapshot;h=abcdef01234;sf=tgz" &&
+	grep "400 - Not a valid hash id:" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+	'snapshots: good object id' \
+	'ID=`git rev-parse --verify HEAD` &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
 test_done
-- 
1.6.4.2