[PATCH 2/2] gitweb: support to globally enable/disable a snapshot format

On 1-Aug-09, at 4:14 AM, Jakub Narebski wrote:

"J.H." [off-list ref] writes:

quoted

Well you can always call xz with -[1-9] to change the compression
level (same as gzip and bzip2) though I think a full disabling would
be 'more' preferable, though I'm not sure I like Jakub's suggestion  
of
just deleting it after the fact, it would work.

[...]

The problem is that 'keys %known_snapshot_formats' serves also as list
of allowed snapshot formats, if project specific override is enabled.
We can add another optional flag ('disabled' => 1) if you don't want
to delete from %known_snapshot_formats in $GITWEB_CONFIG, though I
don't know if it is worth it.  Anyway such mechanism can be added, and
IMHO should be added, in a separate commit.

Is this correct?


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


----------->8--------------
From: Mark Rada <redacted>
Date: Sat, 1 Aug 2009 13:24:03 -0400
Subject: [PATCH 2/2] gitweb: support to globally enable/disable a  
snapshot format

I added an 'enabled' variable to to the $known_snapshot_format keys so
that a Gitweb administrator can globally enable or disable a specific
format for snapshots.

All formats are enabled by default because project specific overriding
is disabled by default.

Signed-off-by: Mark Rada <redacted>
---
  gitweb/gitweb.perl |   14 ++++++++++----
  1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3398163..0a9cec6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -167,27 +167,31 @@ our %known_snapshot_formats = (
  		'type' => 'application/x-gzip',
  		'suffix' => '.tar.gz',
  		'format' => 'tar',
-		'compressor' => ['gzip']},
+		'compressor' => ['gzip'],
+		'enabled' => 1},

  	'tbz2' => {
  		'display' => 'tar.bz2',
  		'type' => 'application/x-bzip2',
  		'suffix' => '.tar.bz2',
  		'format' => 'tar',
-		'compressor' => ['bzip2']},
+		'compressor' => ['bzip2'],
+		'enabled' => 1},

  	'txz' => {
  		'display' => 'tar.xz',
  		'type' => 'application/x-xz',
  		'suffix' => '.tar.xz',
  		'format' => 'tar',
-		'compressor' => ['xz']},
+		'compressor' => ['xz'],
+		'enabled' => 1},

  	'zip' => {
  		'display' => 'zip',
  		'type' => 'application/x-zip',
  		'suffix' => '.zip',
-		'format' => 'zip'},
+		'format' => 'zip',
+		'enabled' => 1},
  );

  # Aliases so we understand old gitweb.snapshot values in repository

@@ -5171,6 +5175,8 @@ sub git_snapshot {
  		die_error(400, "Unknown snapshot format");
  	} elsif (!grep($_ eq $format, @snapshot_fmts)) {
  		die_error(403, "Unsupported snapshot format");
+	} elsif (!$known_snapshot_formats{$format}{'enabled'}) {
+		die_error(403, "Snapshot format not allowed");
  	}

  	if (!defined $hash) {

-- 
1.6.4

By the way, somehow [PATCH 2/2] doesn't look for me as if it was reply
to [PATCH 1/2] email...

Mark A Rada [off-list ref] writes:

On 1-Aug-09, at 4:14 AM, Jakub Narebski wrote:

quoted

"J.H." [off-list ref] writes:

quoted

Well you can always call xz with -[1-9] to change the compression
level (same as gzip and bzip2) though I think a full disabling would
be 'more' preferable, though I'm not sure I like Jakub's suggestion
of just deleting it after the fact, it would work.

[...]

The problem is that 'keys %known_snapshot_formats' serves also as list
of allowed snapshot formats, if project specific override is enabled.
We can add another optional flag ('disabled' => 1) if you don't want
to delete from %known_snapshot_formats in $GITWEB_CONFIG, though I
don't know if it is worth it.  Anyway such mechanism can be added, and
IMHO should be added, in a separate commit.

Is this correct?

Thanks for doing it.

It is correct, although I had in mind slightly different solution

I'd rather use here, taking for example 'txz' as "known but disabled"

  +		'compressor' => ['xz'],
  +		'disabled' => 1},

  	'zip' => {
  		'display' => 'zip',
  		'type' => 'application/x-zip',
  		'suffix' => '.zip',
		'format' => 'zip'},
  );

And then you would not need to modify the rest of snapshot formats,
making use of the fact that $hash{'key'} is false-ish, if 'key' does
not exist in hash.

Also, this should be documented at least in comments.
 

This would be modified then to read:

  +	} elsif ($known_snapshot_formats{$format}{'disabled'}) {
  +		die_error(403, "Snapshot format not allowed");

Also, we want not only protect againts *using* known but disabled
format, but also do not display it as one of formats available, even
if project specific override is supprted for snapshot, and projects
list it in gitweb.snapshot:

@@ -509,7 +509,8 @@ sub filter_snapshot_fmts {
 		exists $known_snapshot_format_aliases{$_} ?
 		       $known_snapshot_format_aliases{$_} : $_} @fmts;
 	@fmts = grep {
-		exists $known_snapshot_formats{$_} } @fmts;
+		exists $known_snapshot_formats{$_} &&
+		!$known_snapshot_formats{$_}{'disabled'} } @fmts;
 }
 
 our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Mark A Rada [off-list ref] writes:

On 1-Aug-09, at 4:14 AM, Jakub Narebski wrote:

quoted

"J.H." [off-list ref] writes:

quoted

Well you can always call xz with -[1-9] to change the compression
level (same as gzip and bzip2) though I think a full disabling would
be 'more' preferable, though I'm not sure I like Jakub's suggestion
of just deleting it after the fact, it would work.

[...]

The problem is that 'keys %known_snapshot_formats' serves also as list
of allowed snapshot formats, if project specific override is enabled.
We can add another optional flag ('disabled' => 1) if you don't want
to delete from %known_snapshot_formats in $GITWEB_CONFIG, though I
don't know if it is worth it.  Anyway such mechanism can be added, and
IMHO should be added, in a separate commit.

Is this correct?

Thanks for doing it.

It is correct, although I had in mind slightly different solution

I'd rather use here, taking for example 'txz' as "known but disabled"

  +		'compressor' => ['xz'],
  +		'disabled' => 1},

  	'zip' => {
  		'display' => 'zip',
  		'type' => 'application/x-zip',
  		'suffix' => '.zip',
		'format' => 'zip'},
  );

And then you would not need to modify the rest of snapshot formats,
making use of the fact that $hash{'key'} is false-ish, if 'key' does
not exist in hash.

Also, this should be documented at least in comments.
 

This would be modified then to read:

  +	} elsif ($known_snapshot_formats{$format}{'disabled'}) {
  +		die_error(403, "Snapshot format not allowed");

Also, we want not only protect againts *using* known but disabled
format, but also do not display it as one of formats available, even
if project specific override is supprted for snapshot, and projects
list it in gitweb.snapshot:

@@ -509,7 +509,8 @@ sub filter_snapshot_fmts {
 		exists $known_snapshot_format_aliases{$_} ?
 		       $known_snapshot_format_aliases{$_} : $_} @fmts;
 	@fmts = grep {
-		exists $known_snapshot_formats{$_} } @fmts;
+		exists $known_snapshot_formats{$_} &&
+		!$known_snapshot_formats{$_}{'disabled'} } @fmts;
 }
 
 our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";

-- 
Jakub Narebski
Poland
ShadeHawk on #git