From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got-read-patch: don't hardcode SHA1_DIGEST_LENGTH
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 2 Aug 2024 11:18:57 +0200

On Fri, Aug 02, 2024 at 10:36:47AM +0200, Omar Polo wrote:
> On 2024/08/02 08:10:21 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > On Fri, Aug 02, 2024 at 12:49:35AM +0200, Omar Polo wrote:
> > > @@ -193,7 +193,11 @@ blobid(const char *line, char **blob, int git)
> > >  	if ((*blob = strndup(line, len)) == NULL)
> > >  		return got_error_from_errno("strndup");
> > >  
> > > -	if (!git && !got_parse_hash_digest(digest, *blob, GOT_HASH_SHA1)) {
> > > +	if (git)
> > > +		return NULL;
> > > +
> > > +	if (!got_parse_hash_digest(digest, *blob, GOT_HASH_SHA1) &&
> > > +	    !got_parse_hash_digest(digest, *blob, GOT_HASH_SHA256)) {
> > 
> > The order of these checks should be flipped because sha2 is longer.
> > A sha2 hash could end up "truncated" and misparsed as a sha1 hash.
> 
> That's not important, because digest is then thrown away; what we really
> care about is the length of the string, so here's a better diff

Ah, indeed. ok!

> +	if (len != got_hash_digest_string_length(GOT_HASH_SHA1) -1 &&
> +	    len != got_hash_digest_string_length(GOT_HASH_SHA256) -1) {

style nit: - 1 instead of -1

>  		/* silently ignore invalid blob ids */
>  		free(*blob);
>  		*blob = NULL;
>