Re: [PATCH] fetch-pack: fix unadvertised requests validation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Gabriel Souza Franco <gabrielfrancosouza@xxxxxxxxx> writes:
>
>> Check was introduced in b791642 (filter_ref: avoid overwriting
>> ref->old_sha1 with garbage, 2015-03-19), but was always false because
>> ref->old_oid.hash is empty in this case. Instead copy sha1 from ref->name.
>>
>> Signed-off-by: Gabriel Souza Franco <gabrielfrancosouza@xxxxxxxxx>
>> ---
>
> Peff, that commit points me at your direction.  And I can see the
> original patch avoids overwriting old_sha1 by saving the result from
> get_sha1_hex() in a temporary, it is true that old_sha1 is not
> updated from the temporary.
>
> The original code before b791642 wanted to say "if ref->name is not
> 40-hex, continue, and otherwise, do the ref->matched thing" and an
> implementation of b791642 that is more faithful to the original
> would indeed have been the result of applying this patch from
> Gabriel, but I am scratching my head why we have hashcmp() there.
>
> Was it to avoid adding the same thing twice to the resulting list,
> or something?

Nah, I think you just misspelt hashcpy() as hashcmp().  The original
wanted to get the binary representation of the hex in old_sha1 when
it continued, and you wanted to delay the touching of old_sha1 until
we make sure that the input is valid 40-hex, so something like this
is what Gabriel wants to do (which I agree with), isn't it?

 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 058c258..bb5237f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -551,7 +551,7 @@ static void filter_refs(struct fetch_pack_args *args,
 				continue;
 			if (get_sha1_hex(ref->name, sha1) ||
 			    ref->name[40] != '\0' ||
-			    hashcmp(sha1, ref->old_sha1))
+			    hashcpy(ref->old_sha1, sha1))
 				continue;
 
 			ref->matched = 1;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]