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

[Jakub Narebski <jnareb@gmail.com>]

Mark A Rada <marada@uwaterloo.ca> writes:

> ------->8-------------
> From: Mark Rada <marada@uwaterloo.ca>
> Date: Sat, 1 Aug 2009 13:24:03 -0400
> Subject: [PATCH 2/2] gitweb: support to globally enable/disable a snapshot format
> 
> I added a 'disabled' variable to to the $known_snapshot_formats 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 <marada@uwaterloo.ca>

O.K.  I think this patch now covers everything needed.  Well, except
documentation.

> ---
>   gitweb/gitweb.perl |   17 ++++++++++++-----
>   1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3398163..a59569f 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -167,27 +167,31 @@ our %known_snapshot_formats = (

At the beginning of %known_snapshot_formats definition there is a
comment with description of structure of this hash.  You should have
updated it, for example in the following way:

@@ -168,7 +168,8 @@ our %known_snapshot_formats = (
 	# 	'suffix' => filename suffix,
 	# 	'format' => --format for git-archive,
 	# 	'compressor' => [compressor command and arguments]
-	# 	                (array reference, optional)}
+	# 	                (array reference, optional),
+	# 	'disabled' => boolean (optional)}
 	#
 	'tgz' => {
 		'display' => 'tar.gz',

The above is, of course, not the only possible way.

>   		'type' => 'application/x-gzip',
>   		'suffix' => '.tar.gz',
>   		'format' => 'tar',
> -		'compressor' => ['gzip']},
> +		'compressor' => ['gzip'],
> +		'disabled' => 0},
> 
>   	'tbz2' => {
>   		'display' => 'tar.bz2',
>   		'type' => 'application/x-bzip2',
>   		'suffix' => '.tar.bz2',
>   		'format' => 'tar',
> -		'compressor' => ['bzip2']},
> +		'compressor' => ['bzip2'],
> +		'disabled' => 0},
> 
>   	'txz' => {
>   		'display' => 'tar.xz',
>   		'type' => 'application/x-xz',
>   		'suffix' => '.tar.xz',
>   		'format' => 'tar',
> -		'compressor' => ['xz']},
> +		'compressor' => ['xz'],
> +		'disabled' => 0},
> 
>   	'zip' => {
>   		'display' => 'zip',
>   		'type' => 'application/x-zip',
>   		'suffix' => '.zip',
> -		'format' => 'zip'},
> +		'format' => 'zip',
> +		'disabled' => 0},
>   );

Although I though that we don't need to put "'disabled' => 0", as 
the fact that 'disabled' key does not exist means that it is enabled.

But if you chose to have all known formats not disabled, then I think
it is a correct solution.  (And then choosing between 'enabled' and
'disabled' is mainly a matter of taste.)

> 
>   # Aliases so we understand old gitweb.snapshot values in repository
> @@ -502,7 +506,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++";
> @@ -5171,6 +5176,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}{'disabled'}) {
> +		die_error(403, "Snapshot format not allowed");
>   	}
> 
>   	if (!defined $hash) {

Now I think that covers everything: preventing displaying known but
disabled snapshot formats in snapshots links in gitweb, and preventing
using known but disabled snapshot format.

-- 
Jakub Narebski

Git User's Survey 2009
http://tinyurl.com/GitSurvey2009