[PATCH 1/4] filter_ref: avoid overwriting ref->old_sha1 with garbage

If the server supports allow_tip_sha1_in_want, then fetch-pack's filter_refs function tries to check whether a ref is a request for a straight sha1 by running:

  if (get_sha1_hex(ref->name, ref->old_sha1))
	  ...

I.e., we are using get_sha1_hex to ask "is this ref name a sha1?". If it is true, then the contents of ref->old_sha1 will end up unchanged. But if it is false, then get_sha1_hex makes no guarantees about what it has written. With a ref name like "abcdefoo", we would overwrite 3 bytes of ref->old_sha1 before realizing that it was not a sha1.

This is likely not a problem in practice, as anything in refs->name (besides a sha1) will start with "refs/", meaning that we would notice on the first character that there is a problem. Still, we are making assumptions about the state left in the output when get_sha1_hex returns an error (e.g., it could start from the end of the string, or error check the values only once they were placed in the output). It's better to be defensive.

We could just check that we have exactly 40 characters of sha1. But let's be even more careful and make sure that we have a 40-char hex refname that matches what is in old_sha1. This is perhaps overly defensive, but spells out our assumptions clearly.

Signed-off-by: Jeff King <peff@peff.net>
---
This one is not necessary for the others, of course, and we can drop it
if you disagree with the reasoning.

I wonder if the thinking in the original was that it was our responsibility here to make sure that ref->old_sha1 was filled in. It is always filled in by the caller who gives us "sought", which makes sense to me (this matches the rest of the "sought" entries, which come from reading the remote's ref list, and of course must fill in old_sha1 from that list).

 fetch-pack.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)