Zlib header stores incorrect wbits value when hist_bits is set to other values than 15.

[rhpvorderman]

This is an issue I encountered when working on the python bindings.

When creating a isal_zstream and setting gzip_flag to IGZIP_ZLIB it can only be decompressed with zlib if wbits is set to 15, regardless of the hist_bits parameter on the isal_zstream.
For example, using gzip_flag=IGZIP_ZLIB and hist_bits=10, it can still only be decompressed using zlib and wbits=15

I had to add a workaround in my test suite: https://github.com/pycompression/python-isal/blob/374fe82ddd3f87c0316f923282787097d5817983/tests/test_compat.py#L102

I have made sure the isal_zstream was set with the right parameters before posting this bug. There is some conversion code that makes the ISA-L interface accessible to a python zlib interface, but I made sure the issue is not in the conversion.

What does work properly:

• The IGZIP_GZIP and IGZIP_DEFLATE flags do not cause problems.
• Creating zlib data using zlib and decompressing with ISA-L with corresponding window bits.
• Creating zlib data using ISA-L and decompressing with ISA-L with corresponding window bits.

In these cases using a window size for decompression that is smaller than the window size for compressing leads to an error, as expected. Using the same window sizes works correctly.

Regardless this is not a dramatically important issue, as there is almost no use case for using a window size less than MAX_HIST_BITS.

[gbtucker]

The flags IGZIP_{DEFLATE, GZIP, ZLIB, etc.} should only change what kind of header/trailer is written on compression or expected on decompression and shouldn't have any relationship to the max amount of history set. So I'm not sure what could cause this issue. I'll try to reproduce to a simple test case.

[rhpvorderman]

I found the issue. The first byte should contain the CM (compression method) and CINFO (windowbits minus 8) flag. That means that for wbits=15 the first byte should be 78 in hex. For wbits=9 it should be 18 in hex.

But:

>>> isal_zlib.compress(b'bla').hex()
'78014bca49040002620130'
>>> isal_zlib.compress(b'bla', wbits=9).hex()
'78014bca49040002620130'

The first byte is always 78, regardless of wbits. So the error is in the zlib header generation code.

[rhpvorderman]

I found the issue. The zlib header is defined as an external constant here:

isa-l/igzip/igzip.c

Line 78 in 1db0363

extern const uint8_t zlib_hdr[];

Then it is passed here:

isa-l/igzip/igzip.c

Line 1720 in 1db0363

hdr = zlib_hdr;

It is defined as a constant here:

isa-l/igzip/hufftables_c.c

Line 42 in 1db0363

const uint8_t zlib_hdr[] = { 0x78, 0x01 };

Unlike the gzip header which does not contain window size, the zlib header does. The zlib header should thus change with window size.

[gbtucker]

Thanks @rhpvorderman for looking into this. That's correct. Currently when using the IGZIP_ZLIB flag to make a zlib (rfc1950) bitstream, we always write 78,01 as the zlib header marking the bitstream as a 32k max window even when actually restricting the window size with hist_bits. So it is a valid rfc1950 bitstream but not marked as lowest window size. However, if you write the header with isal_write_zlib_header(), it does match the fields set in the struct isal_gzip_header including the window bits in hdr->info. It should be fairly easy to change the IGZIP_ZLIB path to have the zlib header match the actual window size.

[rhpvorderman]

Thanks @gbtucker for your comments. I changed the code paths in a PR but there seems some issues with regards to flushing after this code change. I will look into this at a later time. (Currently recovering from a concussion and only able to work very short shifts).

[rhpvorderman]

Is a new ISA-L release on the schedule? Just asking so I know when I can fix this bug in the python bindings. I am by no means trying to rush anything.

[gbtucker]

Sorry @rhpvorderman, I though I had answered this. We hadn't planned a new release until next year.

[rhpvorderman]

@gbtucker no problem, thanks for the update!
I will make sure the python bindings are in tip-top shape by next year. All bugs that I know of have now been removed and performance is satisfactory (minimal or no overhead compared to the zlib-interface in CPython when operating on small data). After ISA-L 2.31 is released I will release the first 1.0.0 version of the bindings as I now consider them safe for production.