threads / patch / 9210

patchgitweb: More detailed error messages for snapshot format

Subject: [PATCH] gitweb: More detailed error messages for snapshot format

## tl;dr

3 messages between Jul 24, 2007 and Jul 24, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Jakub Narebski· Jul 24, 2007, 21:00 UTC · lore

Improve error messages for snapshot format in git_snapshot: distinguish between situation where snapshots are turned off, where snapshot format ('sf') parameter is invalid, where given snapshot format does not exist in %known_snapshot_formats hash, and where gitweb was given unsupported snapshot format.

While at it, use first from all supported snapshots format as default, if no snapshot format was provided.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
Show changes to gitweb/gitweb.perl +10 −3
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fdfce31..708448c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4317,9 +4317,16 @@ sub git_snapshot {
 	@supported_fmts = filter_snapshot_fmts(@supported_fmts);
 
 	my $format = $cgi->param('sf');
-	unless ($format =~ m/[a-z0-9]+/
-	        && exists($known_snapshot_formats{$format})
-	        && grep($_ eq $format, @supported_fmts)) {
+	if (!@supported_fmts) {
+		die_error('403 Permission denied', "Permission denied");
+	}
+	# default to first supported snapshot format
+	$format ||= $supported_fmts[0];
+	if ($format !~ m/[a-z0-9]+/) {
+		die_error(undef, "Invalid snapshot format parameter");
+	} elsif (!exists($known_snapshot_formats{$format})) {
+		die_error(undef, "Unknown snapshot format");
+	} elsif (!grep($_ eq $format, @supported_fmts)) {
 		die_error(undef, "Unsupported snapshot format");
 	}
 

-- 
1.5.2.4
Matt McCutchen· Jul 24, 2007, 21:50 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: More detailed error messages for snapshot format

On 7/24/07, Jakub Narebski <jnareb@gmail.com> wrote:
Show 8 quoted lines
> Improve error messages for snapshot format in git_snapshot:
> distinguish between situation where snapshots are turned off, where
> snapshot format ('sf') parameter is invalid, where given snapshot
> format does not exist in %known_snapshot_formats hash, and where
> gitweb was given unsupported snapshot format.
>
> While at it, use first from all supported snapshots format as default,
> if no snapshot format was provided.

I tested the patch and it worked properly: the errors appeared at the appropriate times and the first supported format was used as the default.

I also noticed that the check for an "invalid" format requires that it contain at least one good character, not that it contain no bad characters, which was what I originally meant. It would be nice to fix the check, or the check could even be removed altogether since gitweb refuses any format that isn't in %known_snapshot_formats . (I think I was misguidedly paranoid to add it in the first place.)

Jakub Narebski· Jul 24, 2007, 23:19 UTC · re: Matt McCutchen · lore

[PATCH (amend)] gitweb: More detailed error messages for snapshot format

Improve error messages for snapshot format in git_snapshot: distinguish between situation where snapshots are turned off, where snapshot format ('sf') parameter is invalid, where given snapshot format does not exist in %known_snapshot_formats hash, and where gitweb was given unsupported snapshot format.

While at it, use first from all supported snapshots format as default, if no snapshot format was provided.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Matt McCutchen wrote:
> On 7/24/07, Jakub Narebski <jnareb@gmail.com> wrote:
> > Improve error messages for snapshot format in git_snapshot:
> > distinguish between situation where snapshots are turned off, where
> > snapshot format ('sf') parameter is invalid,
[...]
Show 6 quoted lines
> I also noticed that the check for an "invalid" format requires that it
> contain at least one good character, not that it contain no bad
> characters, which was what I originally meant.  It would be nice to
> fix the check, or the check could even be removed altogether since
> gitweb refuses any format that isn't in %known_snapshot_formats .  (I
> think I was misguidedly paranoid to add it in the first place.)
This fixes the check (although perhaps the check should be removed).
 gitweb/gitweb.perl |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
Show changes to gitweb/gitweb.perl +10 −3
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fdfce31..0acd0ca 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4317,9 +4317,16 @@ sub git_snapshot {
 	@supported_fmts = filter_snapshot_fmts(@supported_fmts);
 
 	my $format = $cgi->param('sf');
-	unless ($format =~ m/[a-z0-9]+/
-	        && exists($known_snapshot_formats{$format})
-	        && grep($_ eq $format, @supported_fmts)) {
+	if (!@supported_fmts) {
+		die_error('403 Permission denied', "Permission denied");
+	}
+	# default to first supported snapshot format
+	$format ||= $supported_fmts[0];
+	if ($format !~ m/^[a-z0-9]+$/) {
+		die_error(undef, "Invalid snapshot format parameter");
+	} elsif (!exists($known_snapshot_formats{$format})) {
+		die_error(undef, "Unknown snapshot format");
+	} elsif (!grep($_ eq $format, @supported_fmts)) {
 		die_error(undef, "Unsupported snapshot format");
 	}
 

-- 
1.5.2.4

← back to recent threads