Re: [PATCH 2/2] gitweb: support to globally enable/disable a snapshot format
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:51:33
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
quoted hunk ↗ jump to hunk
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 = ( 'txz' => { 'display' => 'tar.xz', 'type' => 'application/x-xz', 'suffix' => '.tar.xz', 'format' => 'tar', - 'compressor' => ['xz']}, + 'compressor' => ['xz'], + 'enabled' => 1},
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.
quoted hunk ↗ jump to hunk
# 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");
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