Permalink
Browse files

access(): the pattern for refs is too strict for filenames

a filename also becomes a "ref" if you use VREF/NAME.

For some reason[1], it seems some people use crazy filenames like foo(0)
or bar%20baz, and these things blow up on that test.

--

[1] viz., the lack of someone with good taste, like me, leading their
project ;-)
  • Loading branch information...
1 parent d8fe757 commit 404dafd79b27b836d9101e912ce0786bec2f09ec @sitaramc committed Jan 11, 2013
Showing with 10 additions and 7 deletions.
  1. +8 −4 src/lib/Gitolite/Conf/Load.pm
  2. +2 −3 t/invalid-refnames-filenames.t
View
12 src/lib/Gitolite/Conf/Load.pm
@@ -79,7 +79,11 @@ sub access {
$deny_rules = option( $repo, 'deny-rules' );
# sanity check the only piece the user can control
- _die "invalid characters in ref or filename: '$ref'\n" unless $ref =~ $REF_OR_FILENAME_PATT;
+ _die "invalid characters in ref or filename: '$ref'\n" unless $ref =~ m(^VREF/NAME/) or $ref =~ $REF_OR_FILENAME_PATT;
+ # apparently we can't always force sanity; at least what we *return*
+ # should be sane/safe. This pattern is based on REF_OR_FILENAME_PATT.
+ (my $safe_ref = $ref) =~ s([^-0-9a-zA-Z._\@/+ :,])(.)g;
+ trace( 2, "safe_ref $safe_ref created from $ref") if $ref ne $safe_ref;
# when a real repo doesn't exist, ^C is a pre-requisite for any other
# check to give valid results.
@@ -91,7 +95,7 @@ sub access {
# similarly, ^C must be denied if the repo exists
if ( $aa eq '^C' and not repo_missing($repo) ) {
trace( 2, "DENIED by existence" );
- return "$aa $ref $repo $user DENIED by existence";
+ return "$aa $safe_ref $repo $user DENIED by existence";
}
trace( 2, scalar(@rules) . " rules found" );
@@ -107,7 +111,7 @@ sub access {
next unless $ref =~ /^$refex/ or $ref eq 'any';
trace( 2, "DENIED by $refex" ) if $perm eq '-';
- return "$aa $ref $repo $user DENIED by $refex" if $perm eq '-';
+ return "$aa $safe_ref $repo $user DENIED by $refex" if $perm eq '-';
# $perm can be RW\+?(C|D|CD|DC)?M?. $aa can be W, +, C or D, or
# any of these followed by "M".
@@ -117,7 +121,7 @@ sub access {
return $refex if ( $perm =~ /$aaq/ );
}
trace( 2, "DENIED by fallthru" );
- return "$aa $ref $repo $user DENIED by fallthru";
+ return "$aa $safe_ref $repo $user DENIED by fallthru";
}
sub git_config {
View
5 t/invalid-refnames-filenames.t
@@ -9,7 +9,7 @@ use Gitolite::Test;
# invalid refnames
# ----------------------------------------------------------------------
-try "plan 57";
+try "plan 56";
try "DEF POK = !/DENIED/; !/failed to push/";
confreset; confadd '
@@ -84,8 +84,7 @@ glt push u1 origin HEAD
tc aa=bb
glt push u1 origin HEAD
/To file:///aa/
- /invalid characters in ref or filename: \\'VREF/NAME/aa=bb/
- reject
+ POK; /HEAD -> master/
# push to branch dd,ee ok
git reset --hard HEAD^

0 comments on commit 404dafd

Please sign in to comment.