madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.55k stars 2.43k forks source link

Small code improvement needed in zran.c to avoid an issue #922

Closed CyberGonzo closed 6 months ago

CyberGonzo commented 6 months ago

I believe I spotted an issue in: https://github.com/madler/zlib/blob/master/examples/zran.c (v 1.4)

deflate_index_extract() can produce an error even though all requested len bytes could be inflated.

Here:

        if (strm.avail_in || ungetc(getc(in), in) != EOF) {
            // There's more after the gzip trailer. Use inflate to skip the
            // gzip header and resume the raw inflate there.

I think it would be appropriate to also check left

        if (left && 
           (strm.avail_in || ungetc(getc(in), in) != EOF)) {
            // There's more after the gzip trailer. Use inflate to skip the
            // gzip header and resume the raw inflate there.

I have a file where deflate_index_extract() proceeds here, even though I have all that I need, and next ret = inflate(&strm, Z_BLOCK); fails, so an error is returned instead of the amount of bytes that could be deflated

madler commented 6 months ago

I did a deflate_index_extract() right up to a gzip member boundary, and there was no problem. Can you provide a small program that replicates the issue you are seeing?

CyberGonzo commented 6 months ago

Hi Mark,

Using my port as part of a bigger project: I just went through all my *.gz files to test (luckily not too many) and I only found one where the problem occurs. It happens to be a split file (.gz.001, .gz.002, ..) and I don't know what application I used to create it back in the days.

I just cat the files together into one .gz file (copy /b .00? newfile.gz) and the problem went away. So .. I need to look into that again. Content wise all is fine, all checksums always match, but I'll look into why the code proceeds into the error situation in case of the split variant. Probably something silly.

Still a good suggestion though. I don't see why deflate_index_extract() should proceed there ? Ok there is still data in strm.avail_in (it seems) but len is 0, so all requested data has been inflated.

It's a head-scratcher but .. my problem it seems.

madler commented 6 months ago

Ok, thanks.