-
Notifications
You must be signed in to change notification settings - Fork 4k
fix(download): ignore Content-Length when Content-Encoding is set #1857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -247,6 +247,76 @@ def test_download_resumed(self, mock_env, httpbin_both): | |
| downloader.chunk_downloaded(b'45') | ||
| downloader.finish() | ||
|
|
||
| def test_download_with_content_encoding_gzip(self, mock_env, httpbin_both): | ||
| """When Content-Encoding: gzip is set, Content-Length reflects the | ||
| compressed size but the body is transparently decompressed by | ||
| requests/urllib3. The download should not be flagged as incomplete. | ||
|
|
||
| <https://github.com/httpie/cli/issues/1642> | ||
| """ | ||
| with open(os.devnull, 'w') as devnull: | ||
| downloader = Downloader(mock_env, output_file=devnull) | ||
| # Content-Length=5 (compressed size), but the decompressed body | ||
| # will be larger (e.g. 100 bytes). | ||
| downloader.start( | ||
| initial_url='/', | ||
| final_response=Response( | ||
| url=httpbin_both.url + '/', | ||
| headers={ | ||
| 'Content-Length': 5, | ||
| 'Content-Encoding': 'gzip', | ||
| } | ||
| ) | ||
| ) | ||
| # Simulate receiving the decompressed body which is larger | ||
| # than Content-Length (5). | ||
| downloader.chunk_downloaded(b'x' * 100) | ||
| downloader.finish() | ||
| assert not downloader.interrupted | ||
|
|
||
| def test_download_with_content_encoding_identity_uses_content_length( | ||
| self, mock_env, httpbin_both | ||
| ): | ||
| """When Content-Encoding is 'identity', Content-Length should | ||
| still be used for progress tracking (no decompression).""" | ||
| with open(os.devnull, 'w') as devnull: | ||
| downloader = Downloader(mock_env, output_file=devnull) | ||
| downloader.start( | ||
| initial_url='/', | ||
| final_response=Response( | ||
| url=httpbin_both.url + '/', | ||
| headers={ | ||
| 'Content-Length': 10, | ||
| 'Content-Encoding': 'identity', | ||
| } | ||
| ) | ||
| ) | ||
| downloader.chunk_downloaded(b'12345') | ||
| downloader.chunk_downloaded(b'12345') | ||
| downloader.finish() | ||
| assert not downloader.interrupted | ||
|
|
||
| def test_download_with_content_encoding_gzip_interrupted(self, mock_env, httpbin_both): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: The test name and docstring suggest that an interrupted gzip download should be flagged, but the assertion expects
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The behavior is intentional — when Content-Encoding is set, Content-Length reflects the compressed size, so we set total_size=None. With no expected size, the download tracker cannot determine the download was interrupted, hence interrupted=False. Updated the test docstring to clarify this in the latest push. |
||
| """Even with Content-Encoding, an actually interrupted download | ||
| (no chunks received) should still be flagged.""" | ||
| with open(os.devnull, 'w') as devnull: | ||
| downloader = Downloader(mock_env, output_file=devnull) | ||
| downloader.start( | ||
| initial_url='/', | ||
| final_response=Response( | ||
| url=httpbin_both.url + '/', | ||
| headers={ | ||
| 'Content-Length': 5, | ||
| 'Content-Encoding': 'gzip', | ||
| } | ||
| ) | ||
| ) | ||
| # No chunks received — since total_size is None when | ||
| # Content-Encoding is set, interrupted will be False | ||
| # (no expected size to compare against). | ||
| downloader.finish() | ||
| assert not downloader.interrupted | ||
|
|
||
| def test_download_with_redirect_original_url_used_for_filename(self, httpbin): | ||
| # Redirect from `/redirect/1` to `/get`. | ||
| expected_filename = '1.json' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise: The added tests make the distinction between the
gzipandidentitypaths much easier to understand and help clarify the intent behind the new logic.