fix: add max_length support to Gzip/Brotli decoders for urllib3 v2.6+ compatibility#492
Closed
benijaichenco wants to merge 2 commits intogoogleapis:mainfrom
Closed
fix: add max_length support to Gzip/Brotli decoders for urllib3 v2.6+ compatibility#492benijaichenco wants to merge 2 commits intogoogleapis:mainfrom
benijaichenco wants to merge 2 commits intogoogleapis:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
product-auto-label
bot
added
the
api: storage
benijaichenco
force-pushed
the
fix-urllib3-max-length-compat
branch
from
3d7a3b2 to
0b38722
Compare
chandra-siri
requested changes
Dec 9, 2025
|
thanks @benijaichenco for contributing :) |
Author
|
All set @chandra-siri :) |
BrennaEpp
approved these changes
Dec 10, 2025
cojenco
approved these changes
Dec 10, 2025
|
Thank you for contributing! We are missing test coverage for lines 692 and 743, do you think that is something you could add @benijaichenco? |
|
Also the linter is failing
|
chandra-siri
requested changes
Dec 11, 2025
Comment on lines
+689
to
+692
| @property | ||
| def has_unconsumed_tail(self) -> bool: | ||
| # Checks for buffered, unread bytes | ||
| return self._decoder.has_unconsumed_tail |
There was a problem hiding this comment.
this property is not required for _GzipDecoder . It's needed only for _BrotliDecoder.
Please remove these lines.
|
It'd also be good to add an explicit test against the latest |
|
any progress on this? (I'm mostly just wanting remove our monkeypatch code) |
4 tasks
Contributor
|
Closing. Superceded by #495. |
chalmerlowe
added a commit
that referenced
this pull request
Feb 26, 2026
…495) ## Summary Fixes #491. Supersedes #492 (stale). `urllib3 >= 2.6.0` now passes `max_length` to `decoder.decompress()` and accesses `decoder.has_unconsumed_tail`. Our custom `_GzipDecoder` and `_BrotliDecoder` wrappers didn't accept these, causing `TypeError` at runtime. This PR: - Updates `_GzipDecoder.decompress` and `_BrotliDecoder.decompress` to accept and forward `max_length`, with a `TypeError` fallback for older urllib3 - Adds `has_unconsumed_tail` property to `_BrotliDecoder` (the proxy class that needs it explicitly) - Applies the same fix to the async `_GzipDecoder` in `google/_async_resumable_media/` - Adds test coverage for the new `max_length` forwarding, the fallback path, and `has_unconsumed_tail` ## Feedback from #492 addressed - **`max_length=-1` default** (chandra-siri) — uses `-1` to match [urllib3's default](https://github.com/urllib3/urllib3/blob/bfe8e198a13800e3ee8ef8124a8928acb170c843/src/urllib3/response.py#L55), not `None` - **`has_unconsumed_tail` only on `_BrotliDecoder`** (chandra-siri) — `_GzipDecoder` inherits this from the parent `urllib3.response.GzipDecoder`, so no override is needed - **Lint failure** (BrennaEpp) — no trailing whitespace - **Missing test coverage for fallback paths** (BrennaEpp) — added tests for `max_length` forwarding, `TypeError` fallback, and `has_unconsumed_tail` (with `AttributeError` fallback for older urllib3) ## Test plan - [x] `pytest tests/unit/requests/test_download.py` — all pass - [x] `pytest tests_async/unit/requests/test_download.py` — all pass - [x] Full unit suite (514 tests) — all pass - [x] No lint issues (flake8 clean) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Chalmer Lowe <chalmerlowe@google.com>
Contributor
|
Closing. Superceded by #495. |
chalmerlowe
added a commit
that referenced
this pull request
Feb 26, 2026
…495) ## Summary Fixes #491. Supersedes #492 (stale). `urllib3 >= 2.6.0` now passes `max_length` to `decoder.decompress()` and accesses `decoder.has_unconsumed_tail`. Our custom `_GzipDecoder` and `_BrotliDecoder` wrappers didn't accept these, causing `TypeError` at runtime. This PR: - Updates `_GzipDecoder.decompress` and `_BrotliDecoder.decompress` to accept and forward `max_length`, with a `TypeError` fallback for older urllib3 - Adds `has_unconsumed_tail` property to `_BrotliDecoder` (the proxy class that needs it explicitly) - Applies the same fix to the async `_GzipDecoder` in `google/_async_resumable_media/` - Adds test coverage for the new `max_length` forwarding, the fallback path, and `has_unconsumed_tail` ## Feedback from #492 addressed - **`max_length=-1` default** (chandra-siri) — uses `-1` to match [urllib3's default](https://github.com/urllib3/urllib3/blob/bfe8e198a13800e3ee8ef8124a8928acb170c843/src/urllib3/response.py#L55), not `None` - **`has_unconsumed_tail` only on `_BrotliDecoder`** (chandra-siri) — `_GzipDecoder` inherits this from the parent `urllib3.response.GzipDecoder`, so no override is needed - **Lint failure** (BrennaEpp) — no trailing whitespace - **Missing test coverage for fallback paths** (BrennaEpp) — added tests for `max_length` forwarding, `TypeError` fallback, and `has_unconsumed_tail` (with `AttributeError` fallback for older urllib3) ## Test plan - [x] `pytest tests/unit/requests/test_download.py` — all pass - [x] `pytest tests_async/unit/requests/test_download.py` — all pass - [x] Full unit suite (514 tests) — all pass - [x] No lint issues (flake8 clean) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Chalmer Lowe <chalmerlowe@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #491
Adds the required
max_length=Noneparameter to decoder methods to maintain compatibility withurllib3 >= 2.6.0.