[PATCH 5/6] gitweb.perl changed (and tested) to return HTTP-404 when missing objects are requested via snapshot

[Jim Klimov <jim@jimklimov.com>]

---
 gitweb/gitweb.perl                        | 62 +++++++++++++++++++++++++------
 t/t9502-gitweb-standalone-parse-output.sh | 24 ++++++------
 2 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fc5b62d..2369ae3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7418,20 +7418,60 @@ sub git_snapshot {
 		%latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 	}
 
-	print $cgi->header(
-		-type => $known_snapshot_formats{$format}{'type'},
-		-content_disposition => 'inline; filename="' . $filename . '"',
-		%co ? (-last_modified => $latest_date{'rfc2822'}) : (),
-		-status => '200 OK');
-
 	printf STDERR "Starting git-archive: $cmd\n" if $DEBUG;
-	open my $fd, "-|", $cmd
-		or die_error(500, "Execute git-archive failed");
+	my $fd;
+	if ( ! open $fd, "-|", $cmd ) {
+		print $cgi->header(-status => '500 Execute git-archive failed');
+		die_error(500, "Execute git-archive failed");
+		return;
+	}
 	printf STDERR "Started git-archive...\n" if $DEBUG;
-	binmode STDOUT, ':raw';
-	print <$fd>;
-	binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+	my $tempByte;
+	my $readSize = read ($fd, $tempByte, 1);
+	my $retCode = 200;
+	if ( defined $readSize ) {
+		if ( $readSize > 0 ) {
+			print $cgi->header(
+				-type => $known_snapshot_formats{$format}{'type'},
+				-content_disposition => 'inline; filename="' . $filename . '"',
+				%co ? (-last_modified => $latest_date{'rfc2822'}) : (),
+				-status => '200 OK' );
+			binmode STDOUT, ':raw';
+			print $tempByte;
+			if ( ! print <$fd> ) {
+				$retCode = 503;
+			}
+			binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+		} else {
+			$retCode = 404;
+		}
+	} else {
+		$readSize = -1;
+		$retCode = 500;
+	}
+
 	close $fd;
+	my $retError = "" ;
+	if ( ($? >> 8) != 0 ) {
+		$retCode = 500;
+		if ( $readSize == 0 ) {
+			# We had empty but not failed read - re-inspect stderr
+			$retError = `$cmd 2>&1`;
+			if ( $retError =~ /did not match any/ ) {
+				$retCode = 404;
+			}
+		}
+	}
+	if ( $retError ne "" ) {
+		$retError = "<br/><pre>$retError</pre><br/>";
+	}
+
+	if ( $retCode == 404 ) {
+		die_error(404, "Not Found - maybe requested objects absent in git path?" . "$retError");
+	} elsif ( $retCode == 500 ) {
+		die_error(500, "Failed to transmit output from git-archive" . "$retError");
+	}
+
 	printf STDERR "Finished posting output of git-archive...\n" if $DEBUG;
 }
 
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 11a116f..a2ae5c4 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -156,21 +156,21 @@ test_expect_success 'snapshot certain objects: have expected content in master b
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
-test_expect_success 'snapshot certain objects: have expected content in master branch - subdir name is required in requested nested path (bad path - empty output)' '
+test_expect_success 'snapshot certain objects: have expected content in master branch - subdir name is required in requested nested path (bad path - empty output and/or HTTP-404)' '
 	rm -f gitweb.body file_list &&
 	BRANCH=master &&
 	gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=third" &&
-	[ ! -s gitweb.body ]
+	[ ! -s gitweb.body -o -n "`head -1 gitweb.headers | egrep "^Status: 404 "`" ]
 '
-test_debug 'cat gitweb.headers && cat file_list'
+test_debug 'cat gitweb.headers && ls -la gitweb.body file_list || true'
 
-test_expect_success 'snapshot certain objects: have expected content in master branch - correct subdir name is required in requested nested path (bad path - empty output)' '
+test_expect_success 'snapshot certain objects: have expected content in master branch - correct subdir name is required in requested nested path (bad path - empty output and/or HTTP-404)' '
 	rm -f gitweb.body file_list &&
 	BRANCH=master &&
 	gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir1/third" &&
-	[ ! -s gitweb.body ]
+	[ ! -s gitweb.body -o -n "`head -1 gitweb.headers | egrep "^Status: 404 "`" ]
 '
-test_debug 'cat gitweb.headers && cat file_list'
+test_debug 'cat gitweb.headers && ls -la gitweb.body file_list || true'
 
 test_expect_success 'snapshot certain objects: have expected content in master branch - can request filenames with spaces (backslash + HTML-escape)' '
 	rm -f gitweb.body file_list &&
@@ -223,21 +223,21 @@ test_expect_success 'snapshot certain objects: have only expected content in ref
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
-test_expect_success 'snapshot certain objects: have expected content in xx/test branch - request for only absent subdir dir2/ fails (empty output)' '
+test_expect_success 'snapshot certain objects: have expected content in xx/test branch - request for only absent subdir dir2/ fails (empty output and/or HTTP-404)' '
 	rm -f gitweb.body file_list &&
 	BRANCH=xx/test &&
 	gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir2" &&
-        [ ! -s "gitweb.body" ]
+	[ ! -s gitweb.body -o -n "`head -1 gitweb.headers | egrep "^Status: 404 "`" ]
 '
-test_debug 'cat gitweb.headers'
+test_debug 'cat gitweb.headers && ls -la gitweb.body file_list || true'
 
-test_expect_success 'snapshot certain objects: have expected content in xx/test branch - request for file /foo and absent subdir dir2/ also fails (empty output)' '
+test_expect_success 'snapshot certain objects: have expected content in xx/test branch - request for file /foo and absent subdir dir2/ also fails (empty output and/or HTTP-404)' '
 	rm -f gitweb.body file_list &&
 	BRANCH=xx/test &&
 	gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir2%20foo" &&
-        [ ! -s "gitweb.body" ]
+	[ ! -s gitweb.body -o -n "`head -1 gitweb.headers | egrep "^Status: 404 "`" ]
 '
-test_debug 'cat gitweb.headers'
+test_debug 'cat gitweb.headers && ls -la gitweb.body file_list || true'
 
 test_expect_success 'snapshot certain objects: have expected content in xx/test branch - have /foo file (and only it)' '
 	rm -f gitweb.body file_list &&
-- 
2.1.4