From: Junio C Hamano <junkio@cox.net>
Date: Tue, 26 Sep 2006 04:11:30 GMT
Subject: Re: [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname
Message-ID: <7v1wpzw9l9.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200609260157.03035.jnareb@gmail.com>

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?

> +	# 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?