Subject: Re: [RFC/PATCH v4 1/2] gitweb: check given hash before trying to create snapshot

From: Jakub Narebski <[email-masked]>
Date: Sun, 13 Sep 2009 17:03:45 +0200

On Sun, 13 Sep 2009, Junio C Hamano wrote:
> Mark Rada <[email-masked]> writes:
>> On 09-09-12 11:30 PM, Junio C Hamano wrote:

>>>> @@ -5196,8 +5202,9 @@ sub git_snapshot {
>>>>  		die_error(403, "Unsupported snapshot format");
>>>>  	}
>>>>  
>>>> -	if (!defined $hash) {
>>>> -		$hash = git_get_head_hash($project);
>>>> +	my $full_hash = git_get_full_hash($project, $hash);
>>>> +	if (!$full_hash) {
>>>> +		die_error(404, 'Hash id was not valid');
>>>>  	}
>>> 
>>> This is in the context of "snapshot", so obviously you care more about
>>> just "such an object exists", don't you?  You also want it to be a
>>> tree-ish.  Try giving it $hash = 'junio-gpg-pub' and see how it breaks.
>>  
>> You have confused me. How is using 'junio-gpg-pub' different from the 
>> second test case that tries to use 'frizzumFrazzum'?
> 
> junio-gpg-pub tag exists in git.git but it tags a blob not a tree.
> 
> 	$ git rev-parse junio-gpg-pub
>         6019c27d966fe3ce8adcc0e9f12078eef96ca6ef
>         $ git archive junio-gpg-pub
>         fatal: not a tree object

So the proper solution with respect to snapshot parameters validation
would be to use

	my $type = git_get_type("$hash^{}");

and check it:
 * if $type is empty or undef (if it is false-ish) then requested object
   does not exist and we return '404 - No such object' (or something like
   that)
 * if $type is 'blob' then we return '400 - Object is not a tree-ish'
   (or something like that)
 * otherwise $type is 'commit' or 'tree'

-- 
Jakub Narebski
Poland