Junio C Hamano <gitster@xxxxxxxxx> writes:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>>> We now abort the loop if inflate() returns Z_BUF_ERROR without
>>> consuming the entire input buffer it was given, or has filled
>>> the entire output buffer but has not yet returned Z_STREAM_END.
>>> Either state is a clear indicator that this loop is not working
>>> as expected, and should not continue.
>>
>> When the inflated contents is of size 0, avail_out would be 0 and avail_in
>> would still have something because the input stream needs to have the end
>> of stream marker that is more than zero byte long.
>
> After thinking about this a bit more, I am reasonably sure that this is
> it. The contents does not have to be a 0-length string, but you would hit
> this if the pure-data portion of the deflated stream aligns at the end of
> your (un)pack window and it happens to require another use_pack() to move
> the window to read the end-of-stream signal. In that situation, the
> output buffer has already been filled, but you haven't read the input
> stream fully. Would't the new check incorrectly trigger in such a case?
>
>>> st = git_inflate(&stream, Z_FINISH);
>>> + if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
>>> + break;
>
> We won't see this on 64-bit platforms because we use larger (un)pack
> window and the condition is much less likely to be met.
Perhaps it would be as simple as this?
We deliberately give one byte more than what we expect to see and error
out when we do get that extra byte filled.
sha1_file.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 4cc8939..8c9f392 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1344,7 +1344,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
off_t curpos)
{
const unsigned char *data;
- unsigned char delta_head[20], *in;
+ unsigned char delta_head[21], *in;
z_stream stream;
int st;
@@ -1357,13 +1357,14 @@ unsigned long get_size_from_delta(struct packed_git *p,
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
- if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
- break;
+ if (!stream.avail_out)
+ break; /* the payload is larger than it should be! */
curpos += stream.next_in - in;
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
- stream.total_out < sizeof(delta_head));
+ stream.total_out < (sizeof(delta_head) - 1));
git_inflate_end(&stream);
- if ((st != Z_STREAM_END) && stream.total_out != sizeof(delta_head)) {
+ if ((st != Z_STREAM_END) &&
+ stream.total_out != sizeof(delta_head) - 1) {
error("delta data unpack-initial failed");
return 0;
}
@@ -1589,15 +1590,15 @@ static void *unpack_compressed_entry(struct packed_git *p,
buffer[size] = 0;
memset(&stream, 0, sizeof(stream));
stream.next_out = buffer;
- stream.avail_out = size;
+ stream.avail_out = size + 1;
git_inflate_init(&stream);
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
- if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
- break;
+ if (!stream.avail_out)
+ break; /* the payload is larger than it should be! */
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR);
git_inflate_end(&stream);
--
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