Han Xin <chiyutianyi@xxxxxxxxx> writes: >> I am not sure if this is not loosening the error checking in the >> dry-run case, though. In the original code, we set the avail_out >> to the total expected size so >> >> (1) if the caller gives too small a size, git_inflate() would stop >> at stream.total_out with ret that is not STREAM_END nor OK, >> bypassing the "break", and we catch the error. >> >> (2) if the caller gives too large a size, git_inflate() would stop >> at the true size of inflated zstream, with STREAM_END and would >> not hit this "break", and we catch the error. >> >> With the new code, since we keep refreshing avail_out (see below), >> git_inflate() does not even learn how many bytes we are _expecting_ >> to see. Is the error checking in the loop, with the updated code, >> catch the mismatch between expected and actual size (plausibly >> caused by a corrupted zstream) the same way as we do in the >> non dry-run code path? >> > > Unlike the original implementation, if we get a corrupted zstream, we > won't break at Z_BUFFER_ERROR, maybe until we've read all the > input. I think it can still catch the mismatch between expected and > actual size when "fill(1)" gets an EOF, if it's not too late. That is only one half of the two possible failure cases, i.e. input is shorter than the expected size. If the caller specified size is smaller than what the stream inflates to, I do not see the new code to be limiting the .avail_out near the end of the iteration, which would be necessary to catch such an error, even if we are not interested in using the inflated contents, no?
