Re: [PATCH v5 1/2] fetch-pack: redact packfile urls in traces

Ivan Frade [off-list ref] writes:

On Thu, Oct 28, 2021 at 4:21 PM Junio C Hamano [off-list ref] wrote:

quoted

"Ivan Frade via GitGitGadget" [off-list ref] writes:

quoted

quoted

+     len = strspn(buffer, "0123456789abcdefABCDEF");
+     if (!(len == 40 || len == 64) || buffer[len] != ' ')
+             return NULL; /* required "<hash>SP" not seen */

People may have comments on hardcoded 40/64 here and offer a better
way to write it ;-)

Latest version uses the_hash_algo->hexsz:

+       if (len != (int)the_hash_algo->hexsz || buffer[len] != ' ')
+               return NULL; /* required "<hash>SP" not seen */

Thanks!

OK.  If the <hash> is given by this side (as opposed to "you started
to talk to a remote, and it turns out that you are still talking
SHA-1 but the other side talks SHA-256 and their <hash> size that is
64 does not match your 40" case), then checking against
the_hash_algo->hexsz should be sufficient.  The original suggestion
was tried both because I didn't know where <hash> originates, and we
would want to redact even in such a hash type mismatch case.

Thanks.  Will take a look at the updated one.

OK.  If the <hash> is given by this side (as opposed to "you started
to talk to a remote, and it turns out that you are still talking
SHA-1 but the other side talks SHA-256 and their <hash> size that is
64 does not match your 40" case), then checking against
the_hash_algo->hexsz should be sufficient.  The original suggestion
was tried both because I didn't know where <hash> originates, and we
would want to redact even in such a hash type mismatch case.

Thanks.  Will take a look at the updated one.

This is when reading from the remote, so <hash> comes from the other
side. I don't think that the remote sending the wrong hash size (and
then needing to redact) is a big concern, but there is definitely no
harm in checking for both (and commenting that these are the SHA-1 and
SHA-256 hash sizes).