gzip module has no custom exception

[srid]

BPO	6584
Nosy	@terryjreedy, @pitrou, @ezio-melotti, @merwok, @mmaker, @serhiy-storchaka, @ZackerySpytz
PRs	bpo-6584: Add a BadGzipFile exception to the gzip module #13022
Files	6584_1.patch 6584_2.patch 6584_3.patch 6584_4.patch 6584_5.patch issue6584_6.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-13.07:53:07.837>
created_at = <Date 2009-07-27.06:10:48.819>
labels = ['3.8', 'type-feature', 'library']
title = 'gzip module has no custom exception'
updated_at = <Date 2019-05-13.07:53:07.837>
user = 'https://bugs.python.org/srid'

bugs.python.org fields:

activity = <Date 2019-05-13.07:53:07.837>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-13.07:53:07.837>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2009-07-27.06:10:48.819>
creator = 'srid'
dependencies = []
files = ['16565', '16566', '21209', '22956', '22958', '27943']
hgrepos = []
issue_num = 6584
keywords = ['patch']
message_count = 20.0
messages = ['90976', '91151', '101185', '101187', '130842', '130844', '130883', '130961', '138536', '138544', '138732', '142517', '174685', '175285', '175286', '175289', '176385', '176891', '341145', '342288']
nosy_count = 10.0
nosy_names = ['terry.reedy', 'pitrou', 'dstanek', 'ezio.melotti', 'eric.araujo', 'gruszczy', 'srid', 'maker', 'serhiy.storchaka', 'ZackerySpytz']
pr_nums = ['13022']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue6584'
versions = ['Python 3.8']

[srid]

Much like zipfile.BadZipfile, we need a base custom exception for the
gzip module. At least, catch gzip-related exceptions and throw a
tarfile.TarError when used *via* tarfile.*.

See the following example (the exception escaped the "try... except
tarfile.TarError: .. " block!):

---

[...]
File "/home/sridharr/as/pypm/src/pypm/common/compression.py", line
199, in _ensure_read_write_access
for tarinfo in tarfileobj.getmembers():
File "/opt/ActivePython-2.6/lib/python2.6/tarfile.py", line 1791, in
getmembers
self._load() # all members, we first have to
File "/opt/ActivePython-2.6/lib/python2.6/tarfile.py", line 2352, in
_load
tarinfo = self.next()
File "/opt/ActivePython-2.6/lib/python2.6/tarfile.py", line 2307, in
next
self.fileobj.seek(self.offset)
File "/opt/ActivePython-2.6/lib/python2.6/gzip.py", line 382, in seek
self.read(1024)
File "/opt/ActivePython-2.6/lib/python2.6/gzip.py", line 219, in read
self._read(readsize)
File "/opt/ActivePython-2.6/lib/python2.6/gzip.py", line 284, in _read
self._read_eof()
File "/opt/ActivePython-2.6/lib/python2.6/gzip.py", line 304, in
_read_eof
hex(self.crc)))
IOError: CRC check failed 0x115929f0 != 0x9f074a38L

[terryjreedy]

Unless something in the docs claims that there is/should be such a
thing, this is a feature request, not a bug ('behavior') report, and
only applicable to future x.y versions.

[gruszczy]

I have created a small patch, that introduces BadGzipFile exception. It is a subclass of IOError, so it would be backward compatible and will be still caught by old code, but this way is distinct from IOError.

[gruszczy]

Modified patch with test, that catches both BadGzipFile and IOError exceptions.

[gruszczy]

Bump! How about commiting this patch? Or maybe there is something missing? I'll be happy to fix it.

[pitrou]

Since the patch makes BadGzipFile a subclass of IOError, it doesn't look unreasonable.
Some nits:

• a gzipped file is not an "archive"
• the unit tests should use either the "with" statement, or try/finally blocks to properly close the file even when the test fails
• you should probably explicitly test that BadGzipFile is a subclass of IOError

[gruszczy]

I'll be very happy to fix this after Friday. Thanks for your comments.

[gruszczy]

I had some time today, so I managed to fix the patch. I hope now everything is ok.

[gruszczy]

Bump! Antoine, do you think the patch is acceptable and can be committed now?

[terryjreedy]

Your changes appear to address all three of Antoine's 'nits'.

[merwok]

Ezio has found a few other things to improve (follow the “review” link to the right of the patch link).

[mmaker]

The attached patch follows Ezio's hints.

[ezio-melotti]

The new exception should also be documented, and a versionadded and Doc/whatsnew/3.4.rst entry added.

[mmaker]

done!

[serhiy-storchaka]

Not all invalid gzip files raise BadGzipFile. Some of them raises ZlibError.

[mmaker]

Well, I specified the word files everywhere for that reason. Looking at Doc/library/zlib.rst I see:

For reading and writing .gz files see the gzip module.

Also, I specified 'the gzip module' on the whatsnew section.
Is there anything more specific I could have done?

[serhiy-storchaka]

I added some comments on Rietveld.

[serhiy-storchaka]

read32() should raise BadGzipFile when less than 4 bytes read. Also you should introduce read8() function which raises BadGzipFile when less than 1 byte read.

See also bpo-4844 and bpo-14315 for zipfile.

[ZackerySpytz]

I'd like to see this issue move forward, so I've created a PR.

[serhiy-storchaka]

New changeset cf599f6 by Serhiy Storchaka (Zackery Spytz) in branch 'master':
bpo-6584: Add a BadGzipFile exception to the gzip module. (GH-13022)
cf599f6