Repository navigation
gzip.compress(..., mtime=0) in cpython 3.11+ unexpectedly sets OS byte in gzip header #112346
Description
Bug report
description
Using gzip.compress() with mtime=0 in 3.8<=cpython<=3.10, the OS byte, i.e. the 10th byte in the GZIP header, is set to 255 "unknown" (also see e.g. #83302):
Line 599 in dc0adb4
| return struct.pack("<BBBBLBB", 0x1f, 0x8b, 8, 0, int(mtime), xfl, 255) |
However, in cpython 3.11 and 3.12, the OS byte is suddenly set to a "known" value, e.g. 3 ("Unix") on Ubuntu.
This is not mentioned in the changelog for Python 3.11.
This may lead to problems in the context of reproducible builds. In our case, hash checking fails after decompressing and re-compressing a gzipped archive.
how to reproduce
Here's an example, where byte 10 is \xff in python 3.10 and \x03 in python 3.11:
~ $ python
Python 3.10.12 (main, Jun 11 2023, 05:26:28) [GCC 11.4.0] on linux
>>> import gzip
>>> gzip.compress(b'', mtime=0)
b'\x1f\x8b\x08\x00\x00\x00\x00\x00\x02\xff\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00'
~ $ pyenv shell 3.11
~ $ python
Python 3.11.6 (main, Nov 23 2023, 17:30:16) [GCC 11.4.0] on linux
>>> import gzip
>>> gzip.compress(b'', mtime=0)
b'\x1f\x8b\x08\x00\x00\x00\x00\x00\x02\x03\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00'cause
I guess this is caused by python 3.11 delegating the gzip.compress() call to zlib if mtime=0, as mentioned in the docs:
Changed in version 3.11: Speed is improved by compressing all data at once instead of in a streamed fashion. Calls with mtime set to 0 are delegated to zlib.compress() for better speed.
and source:
Lines 609 to 612 in 89ddea4
| if mtime == 0: | |
| # Use zlib as it creates the header with 0 mtime by default. | |
| # This is faster and with less overhead. | |
| return zlib.compress(data, level=compresslevel, wbits=31) |
Apparently zlib does set the OS byte.
CPython versions tested on:
3.8, 3.9, 3.10, 3.11, 3.12
Operating systems tested on:
Linux, macOS, Windows
Linked PRs
- gh-112346: Bugfix: Remove faster codepath from gzip.compress as it introduces behavioral inconsistencies #114116
- gh-112346: Document the OS byte in
gzip.compressoutput change in 3.11 #120480 - gh-112346: Always set OS byte to 255, simpler gzip.compress function. #120486
- [3.13] gh-112346: Always set OS byte to 255, simpler gzip.compress function. (GH-120486) #120563
- [3.13] gh-112346: Document the OS byte in
gzip.compressoutput change in 3.11 (GH-120480) #120612 - [3.12] gh-112346: Document the OS byte in
gzip.compressoutput change in 3.11 (GH-120480) #120613 - [3.11] gh-112346: Document the OS byte in
gzip.compressoutput change in 3.11 (GH-120480) #120614
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
dennisvang commentedon Nov 23, 2023
In itself I think it may be a good thing that the
OSbyte is properly set.The problem is just that the change is not explicitly documented, as far as I know.
add test for gzip header OS flag
rhpvorderman commentedon Jan 16, 2024
Hi @dennisvang . This is my fault, as I delegated gzip.compress(mtime=0) to zlib.compress, incorrectly assuming this was the same. The reason is that zlib.compress is faster. But if it leads to behavioral changes, that is not acceptable.
I believe this can easily be remedied by removing the codepath.
rhpvorderman commentedon Jan 16, 2024
I have made a PR. Just now and put Bugfix in the name. Now I hope it will get attention.
dennisvang commentedon Jan 16, 2024
@rhpvorderman Thanks for picking this up.
I wonder, if this is the only side-effect, and if the performance gain from using
zlib.compressis worth it, perhaps you could just keep delegating tozliband change byte 10 back to\xffafterwards?rhpvorderman commentedon Jan 16, 2024
Well, as mentioned in the PR, keeping two separate code paths caused issues before. It is best to keep one codepath. There is a mention in the documentation about zlib.compress so users who need the performance can use it themselves.
dennisvang commentedon Jan 16, 2024
@rhpvorderman You're right, that makes sense.
rhpvorderman commentedon Apr 15, 2024
ping
This bug and fix have been lingering for a while.
serhiy-storchaka commentedon Jun 12, 2024
For reference, this feature was added in bpo-43613 (gh-87779). It included more optimizations, the only issue with delegating the whole compression to zlib, when mtime is 0.
The fix looks correct and it still preserves some speed up. An alternate solution could be to call
zlib.compress()(even if mtime is not 0) and then patch the result for mtime and the OS byte, but I do not know how reliable is it and whether method is faster.Merge branch 'main' into pythongh-112346
gpshead commentedon Jun 12, 2024
Is this really a big deal? We won't be able to backport this to 3.11 as that is in security fix only mode.
zlib cannot be presumed to produce canonical output. There are many different zlib implementations.
Decompressing gzipped data that you did not produce and recompressing it without using identical software is already not guaranteed to produce the same compressed output.
From a reproducible build perspective I suggest always patching irrelevant fields such as gzip header mtime and OS fields to constant values as part of the build.
dennisvang commentedon Jun 13, 2024
@gpshead Probably not for most people.
As commented above, I was only hoping for a small note to be added to the changelog (or documentation).
Just in case someone does rely on the OS byte in the gzip header, in whatever context.
Very true
I just mentioned the reproducible build example to provide some context, although it was an edge case involving files created on the same machine, with exact same zlib implementation, but a different python version.
10 remaining items
pythongh-112346: Document the OS byte in `gzip.compress` output chang…
pythongh-112346: Document the OS byte in `gzip.compress` output chang…
[3.12] gh-112346: Document the OS byte in `gzip.compress` output chan…
[3.13] gh-112346: Document the OS byte in `gzip.compress` output chan…
[3.11] gh-112346: Document the OS byte in `gzip.compress` output chan…
pythongh-112346: Always set OS byte to 255, simpler gzip.compress fun…
pythongh-112346: Document the OS byte in `gzip.compress` output chang…
pythongh-112346: Always set OS byte to 255, simpler gzip.compress fun…
pythongh-112346: Document the OS byte in `gzip.compress` output chang…
pythongh-112346: Always set OS byte to 255, simpler gzip.compress fun…
pythongh-112346: Document the OS byte in `gzip.compress` output chang…
effigies commentedon Feb 3, 2025
I think this can be closed?
Fix failing reproducibility tests in master.