You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Declarative HTTP actions should fail consistently when a response exceeds the host's body-size policy, before text conversion or workflow assignment.
Description & Review Guide
What are the major changes? Add a configurable, positive max_response_body_bytes with a 10 MiB default. Read raw response streams and bound both received bytes and each gzip/deflate decoding stage. Apply the same policy before redirect or authentication responses are buffered. Validate wrapped/raw deflate formats without guessing from their prefix, finalize ordinary empty encoded responses, and preserve selected-client encoding policy. Document the supported encodings and configuration. Constrain HTTPX to the validated 0.27/0.28 series and add focused compatibility CI for both.
What is the impact of these changes? Oversized bodies raise httpx.StreamError; unsupported, incomplete, or trailing encoded data raises httpx.DecodingError. Identity, gzip, and deflate are supported, with up to four encoding stages. Response streams are closed on success, failure, and cancellation; caller-owned clients remain open. Existing client authentication, redirect settings, cookies, headers, and text encodings are retained. HTTPX must satisfy >=0.27,<0.29. Supplied response hooks and transports must not prebuffer bodies.
What do you want reviewers to focus on? Decoding bounds, intermediate response handling, HTTPX content-cache integration, and client configuration/lifetime preservation.
Focused checks ran from python using Python 3.13 and an x64 .NET 10 runtime for PowerFx:
.\.venv\Scripts\python.exe -m pytest packages\declarative\tests\test_default_http_request_handler.py -k response_body_above_default_limit -q — before the implementation change: all three cases failed because the expected exception was not raised; after the change: all three passed.
.\.venv\Scripts\python.exe -m pytest packages\declarative\tests\test_default_http_request_handler.py packages\declarative\tests\test_http_request_executor.py packages\declarative\tests\test_http_request_yaml_integration.py -m 'not integration' -n 2 -q -o addopts='' — 168 passed, none skipped. Stable parameter IDs and deterministic compressed fixtures also preserve pytest-xdist collection. Coverage includes platform-bound decoder arguments for large configured limits and bodyless-response boundaries, including 205, 1xx, and successful versus unsuccessful CONNECT responses.
.\.venv\httpx-compat\Scripts\python.exe -m pytest packages\declarative\tests\test_default_http_request_handler.py -m 'not integration' -q -o addopts='' — 118 passed with HTTPX 0.27.0 and 118 passed with 0.28.1 in an isolated environment, none skipped.
python -m uv pip check --python .venv\httpx-compat\Scripts\python.exe — passed for both HTTPX versions.
This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.
The reason will be displayed to describe this comment to others. Learn more.
MAF Automated Review — Iteration 1
Result: Findings reported Scope: full PR (1 commit(s)): 40d937589862 Model:gpt-5.6-sol
Overview
The implementation adds per-response wire and decoding-stage limits, closes streams unconditionally, and exercises redirects, authentication retries, malformed encodings, and cancellation. Two compatibility/correctness gaps remain: selected-client Accept-Encoding policy is overwritten, and an empty truncated encoded response can bypass the promised completion check.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.
max_response_body_bytes accepts every positive integer, but this max_length is passed to zlib as a Py_ssize_t. For example, a legal value of 10**100 makes any gzip response fail with OverflowError: Python int too large to convert to C ssize_t instead of enforcing the configured policy. Please either reject limits above the platform-supported maximum during construction or clamp the per-call zlib bound safely, and cover that boundary.
Addressed the large-limit point in #9358 (review) in ca54a60: the native zlib argument is clamped to sys.maxsize without changing the configured cumulative limit. All six platform/large-limit gzip and deflate regressions failed with OverflowError before the fix and now pass. The full focused HTTP set passes all 160 cases under pytest-xdist; lint, formatting, and source/test Pyright checks pass.
205 Reset Content is also a bodyless response. With Content-Encoding: gzip and no bytes, this branch leaves the decoder enabled, so finish() raises DecodingError for a valid 205 response. Include 205 here and add it to the parameterized bodyless-response test.
Document 205 Reset Content as bodyless
python/packages/declarative/README.md:58
The bodyless-response list omits 205 Reset Content, whose responses must not contain content. Update this documentation together with the implementation and regression test so valid empty 205 responses are not described or handled as incomplete encodings.
This bodyless-response coverage omits status 205, which is why the implementation's incomplete bodyless-status check is not detected. Add a 205 case so an empty encoded 205 Reset Content remains accepted.
The bodyless-response exemption is incomplete: HTTP 1xx responses, 205 Reset Content, and successful CONNECT responses also cannot carry response content. If any of these includes a representation-level Content-Encoding header, the current code finalizes an empty decoder and raises DecodingError instead of returning the bodyless response. Extend this condition (and the bodyless-response cases in the test) to cover all HTTP bodyless semantics.
Addressed the remaining bodyless-response points in #9358 (review) in 6665733 (205 was already handled in 5926ef8). The exemption now also includes 1xx and successful CONNECT responses; the README and regression cases cover these boundaries. All five added bodyless cases failed with DecodingError before the fix and now pass. Unsuccessful CONNECT responses still validate encoded content. All 168 focused HTTP tests pass under pytest-xdist; Ruff and source/test Pyright checks pass.
This introduces a hard dependency on HTTPX internals (httpx._client.ACCEPT_ENCODING, plus the later response._content write), while this package declares httpx>=0.27,<1. A compatible 0.x release may move either private implementation detail and make the declarative package fail at import time or stop caching correctly. Please avoid private HTTPX symbols, or constrain the dependency to the explicitly validated HTTPX series and add compatibility coverage for that constraint.
Addressed the compatibility point in #9358 (review) in 99633f4: declarative now requires HTTPX >=0.27,<0.29, with matching lock metadata and documented rationale. Two isolated CI jobs cover 0.27.0 and 0.28.1. Local runs passed all 118 handler tests on each version, with dependency consistency checks passing; the neighboring HTTP suite still passes all 168 cases. Ruff and source/test Pyright pass, and uv lock --check --offline confirms unchanged resolved versions and registry URLs.
This iteration preserves selected-client encoding policy, validates ordinary empty encoded responses, and replaces ambiguous deflate-prefix detection with bounded dual-mode validation. Per-stage size checks, unconditional response cleanup, focused boundary tests, and HTTPX 0.27/0.28 compatibility coverage prevent a publishable Critical, High, or Medium concern in the authoritative incremental range.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
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
breaking changeUsage: [PRs], Target: all PRs that introduce changes that are not backward compatibledocumentationUsage: [Issues, PRs], Target: documentation in the code base and learn docspythonUsage: [Issues, PRs], Target: Python
3 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.
Motivation & Context
Declarative HTTP actions should fail consistently when a response exceeds the host's body-size policy, before text conversion or workflow assignment.
Description & Review Guide
max_response_body_byteswith a 10 MiB default. Read raw response streams and bound both received bytes and each gzip/deflate decoding stage. Apply the same policy before redirect or authentication responses are buffered. Validate wrapped/raw deflate formats without guessing from their prefix, finalize ordinary empty encoded responses, and preserve selected-client encoding policy. Document the supported encodings and configuration. Constrain HTTPX to the validated 0.27/0.28 series and add focused compatibility CI for both.httpx.StreamError; unsupported, incomplete, or trailing encoded data raiseshttpx.DecodingError. Identity, gzip, and deflate are supported, with up to four encoding stages. Response streams are closed on success, failure, and cancellation; caller-owned clients remain open. Existing client authentication, redirect settings, cookies, headers, and text encodings are retained. HTTPX must satisfy>=0.27,<0.29. Supplied response hooks and transports must not prebuffer bodies.Focused checks ran from
pythonusing Python 3.13 and an x64 .NET 10 runtime for PowerFx:.\.venv\Scripts\python.exe -m pytest packages\declarative\tests\test_default_http_request_handler.py -k response_body_above_default_limit -q— before the implementation change: all three cases failed because the expected exception was not raised; after the change: all three passed..\.venv\Scripts\python.exe -m pytest packages\declarative\tests\test_default_http_request_handler.py packages\declarative\tests\test_http_request_executor.py packages\declarative\tests\test_http_request_yaml_integration.py -m 'not integration' -n 2 -q -o addopts=''— 168 passed, none skipped. Stable parameter IDs and deterministic compressed fixtures also preserve pytest-xdist collection. Coverage includes platform-bound decoder arguments for large configured limits and bodyless-response boundaries, including 205, 1xx, and successful versus unsuccessful CONNECT responses..\.venv\httpx-compat\Scripts\python.exe -m pytest packages\declarative\tests\test_default_http_request_handler.py -m 'not integration' -q -o addopts=''— 118 passed with HTTPX 0.27.0 and 118 passed with 0.28.1 in an isolated environment, none skipped.python -m uv pip check --python .venv\httpx-compat\Scripts\python.exe— passed for both HTTPX versions.python -m uv lock --check --offline— passed; resolved versions and registry URLs remain unchanged..\.venv\Scripts\python.exe -m ruff check packages\declarative\agent_framework_declarative\_workflows\_http_handler.py packages\declarative\tests\test_default_http_request_handler.py— passed..\.venv\Scripts\python.exe -m ruff format --check packages\declarative\agent_framework_declarative\_workflows\_http_handler.py packages\declarative\tests\test_default_http_request_handler.py— passed..\.venv\Scripts\python.exe -m pyright --pythonpath .venv\Scripts\python.exe --project packages\declarative\pyproject.toml packages\declarative\agent_framework_declarative\_workflows\_http_handler.py— zero errors or warnings..\.venv\Scripts\python.exe -m pyright --pythonpath .venv\Scripts\python.exe --project pyrightconfig.tests.json packages\declarative\tests\test_default_http_request_handler.py— zero errors or warnings.The full workspace test suite and other test type checkers were not run locally.
Related Issue
N/A - maintainer-directed bugfix; submission without a public tracking issue was explicitly approved. No matching competing open PR was found.
AI Assistance
AI assistance details: Copilot assisted with implementation, tests, documentation, and this PR description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.