Re: [PATCH] unpack_sha1_header(): detect malformed object header

Jeff King [off-list ref] writes:

This part I don't understand, though. We clearly need to look for the
NUL. But why do we need to look for the space? The loop in
parse_sha1_header() can easily detect this as it looks for the end of
the type name (and if it hits the end-of-string, can bail as in your
original patch).
I.e., the root of the problem is that we pass parse_sha1_header() a the
"ptr" half of a ptr/len buffer, and it has no idea how much we read.
But once we get it that information (either by passing the length, or by
ensuring that the buffer is NUL-terminated, it should be easy for it to
do the right thing.

Yup.

Anyway, here's my ptr/len version (which passes the length back out of
unpack_sha1_header via an in/out pointer). After thinking on it, though,
I'm of the opinion that we're better off just ensuring that "hdr" is
NUL-terminated. We end up assuming that anyway later, since we have to
know how much of the header buffer was consumed by parsing.

I'd agree, not because I didn't first go in this <ptr,len> route
myself, but because the attached change does look quite invasive.
Also, I think it is OK to ask unpack_*_header() to fail if what it
turns can no way be a header, e.g. lacks NUL termination.

Do note the final call below in the streaming loose-open code, which
exhibits that, but also seems to call parse_sha1_header() without
checking its return value. I think that needs fixed regardless of the
approach.

Good that your attempt to signature-changing change caught it.  I'll
take a further look.

Thanks.