bpo-45509: Check gzip headers for corrupted fields

[rhpvorderman]

Check whether the COMMENT, NAME and HCRC fields are correct.

EDIT: Additionally, slightly increase performance for header checking for headers produced by gzip.compress or zlib.compress.

https://bugs.python.org/issue45509

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[serhiy-storchaka]

Try to use a bytearray instead of io.BytesIO. Would it be faster? In what cases?

[rhpvorderman]

Performance improvement was necessary (see bug report). So I made it better the following way:

• When the data is read from fp a bytes object is created. Instead of passing it anonymously to the functions that performs the check it is saved as a local variable.
• The FHCRC flag is checked and the result is saved as fhcrc, so the check runs only once.
• When fhcrc is truthy, a header_parts list is created.
• Every time fp is read and fhcrc is truthy the already created bytes objects are appended to the header_parts list. List appending has a very low overhead. It simply expands an array of PyObject pointers. Pointers that we already have as the named bytes objects.
• When the header should be checked, a b"".join(header_parts) is called to recreate the header.

This means for the most common use cases (no flags or only FNAME set) that only a few extra truthy checks are performed. This comes at a very low cost of 3% (see the bug report for results).

This is worth it since it enables correct truncation checking for FNAME and FCOMMENT as well as enabling checking of the header CRC when FHCRC is set.

[rhpvorderman]

@serhiy-storchaka Thank you for taking a look and thank you for your insights. I have updated the PR accordingly. It is much better now in terms of performance.

[rhpvorderman]

I made a small tweek that shaves off another 2 microseconds

• Use if not magic instead of if magic == b''
• Use fhcrc = bool(flag & FHCRC) instead of fhcrc = flag & FHCRC

[rhpvorderman]

With further performance tweaks, reading the gzip header is now either faster or neutral compared to the most common flags (no flags, or only FNAME set). See bpo-45509 for more performance details.

The performance tweaks are:

• Check if flags == 0. If so return last_mtime immediately and skip any further processing.
• Check if flags == FNAME. If so read until the NULL byte terminating the name and return last_mtime. Skip any further processing.
• In all other cases. Use a slower code path were all parts of the header are saved for header crc checking. (Even if FHCRC is not set!). A single code path for all other cases is used here to keep the code simple(r). Since these cases will be rare, it makes no sense complicating the code for an optimal code path where parts of the header are only saved if FHCRC is set.

[rhpvorderman]

@serhiy-storchaka The PR is ready for re-review when you have the time. The performance issue has been addressed.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.