Re: [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname

[Jakub Narebski <jnareb@gmail.com>]

Dnia wtorek 26. września 2006 06:11, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > @@ -387,16 +400,37 @@ sub href(%) {
> >  ## 
======================================================================
> >  ## validation, quoting/unquoting and escaping
> >  
> > -sub validate_input {
> > -	my $input = shift;
> > +sub validate_pathname {
> > +	my $input = shift || return undef;
> >  
> > -	if ($input =~ m/^[0-9a-fA-F]{40}$/) {
> > -		return $input;
> > +	# no '.' or '..' as elements of path, i.e. no '.' nor '..'
> > +	# at the beginning, at the end, and between slashes.
> > +	if ($input =~ m!(^|/)(|\.|\.\.)(/|$)!) {
> > +		return undef;
> >  	}
> > -	if ($input =~ m/(^|\/)(|\.|\.\.)($|\/)/) {
> > +	# no doubled slashes
> > +	if ($input =~ m!//!) {
> >  		return undef;
> >  	}
> 
> I do not think you need the second check for double-slash.  The
> pattern you borrowed from the original:
> 
> 	/(^|\/)(|\.|\.\.)($|\/)/)
> 
> cleverly matches an empty string with $2, so you already match
> double-slash with $1 = '/' $2 = '' $3 = '/', don't you?

Do I need to resend patch, then, to remove this unnecessary check?

> > +	# it must be correct pathname
> > +	$input = validate_pathname($input)
> > +		or return undef;
> > +	# restrictions on ref name according to git-check-ref-format
> > +	if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
> >  		return undef;
> >  	}
> 
> Why would you need validate_pathname here?

refname _must_ be a valid pathname, no? It means for example that it
cannot have double slashes, not NUL (the only thinkg not covered by
git-check-ref-format restrictions). Well, we could add that to regexp
instead...

-- 
Jakub Narebski
Poland