Repository navigation
Conversation
….request Add urllib.request.HTTPGzipHandler, an opt-in handler that requests gzip-compressed responses and transparently decodes them. The handler is not one of the default build_opener() handlers, so urlopen()'s behaviour is unchanged unless it is added explicitly. When the request has no Accept-Encoding header, the handler adds "Accept-Encoding: gzip"; if the response then carries "Content-Encoding: gzip", the body is decoded incrementally as it is read and the Content-Encoding and Content-Length headers, which describe the compressed body, are removed. A request that already carries an Accept-Encoding header is left untouched, so a caller that means to handle the compressed response itself still receives it. Only gzip is handled, and the zlib module is required.
Documentation build overview
432 files changed ·
|
HTTPGzipHandler() raises ImportError without zlib, so on builds that lack it (such as the MSan CI job) twelve of the tests errored instead of being skipped.
| https_response = http_response | ||
|
|
||
|
|
||
| class _GzipReader(io.BufferedIOBase): |
There was a problem hiding this comment.
Instead of creating a new _GzipReader class, can we not use https://docs.python.org/3/library/gzip.html gzip.GzipFile(fileobj=response, mode="rb")
This is a critical change that is required before we can merge this PR.
orsenthil
left a comment
orsenthil
left a comment
There was a problem hiding this comment.
Let us please use gzip directly instead of hand building it.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Replace the hand-written _GzipReader with gzip.GzipFile, as requested in review. Unlike _GzipReader, GzipFile bounds how much a sized read decompresses. GzipFile does not close the file object it wraps, so addclosehook closes the response. gzip is imported inside the handler, so importing urllib.request does not import it. Junk after the last gzip member now raises BadGzipFile, as it does with gzip.open(), so the test that expected it to be ignored is removed.
|
@orsenthil, done in aaaa85a. This also fixes a memory bug in my reader: it decompressed each 128 KiB it read in one go, so a I have made the requested changes; please review again |
|
Thanks for making the requested changes! @orsenthil: please review the changes made to this pull request. |
This adds
urllib.request.HTTPGzipHandler, an opt-in handler that sendsAccept-Encoding: gzipand transparently decodes aContent-Encoding: gzipresponse body. It's the long-standing request in this issue (originally bpo-1508475).Opt-in: the handler isn't in
build_opener()'s defaults, sourlopen()is unchanged unless you ask for it withbuild_opener(HTTPGzipHandler). Martin Panter (2014) asked what happens when someone downloads a.tar.gz, and suspected Range requests would break; opt-in keeps both out of the default path. Senthil Kumaran preferred on-by-default in 2012, and on the issue has since called opt-in "better and conservative".No double-decode: the handler decodes only when it added
Accept-Encodingitself. A request with a caller-suppliedAccept-Encodinggets the response untouched, which answers the double-decode objection from Serhiy Storchaka (2012) and Martin Panter (2014). After decoding,Content-EncodingandContent-Lengthare removed, since they describe the compressed body.Decoding: the response is wrapped in
gzip.GzipFile(fileobj=response, mode="rb"), which decompresses as the caller reads and never more than one buffer ahead of a sized read.addclosehookcloses the response, because GzipFile doesn't close the file it wraps. Concatenated members are decoded in full. As withgzip.open(), junk after the gzip data raisesgzip.BadGzipFileand a truncated stream raisesEOFError.Scope: gzip only.
deflateis left out because some servers send it without the zlib wrapper (RFC 9110, section 8.4.1.2); it can be a follow-up. Constructing the handler withoutzlibraisesImportError.test_urllib2,test_urllibandtest_urllib2_localnetpass.