Skip to content

Python: [BREAKING] fix(declarative): bound HTTP response bodies - #9358

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 7 commits into
mainfrom
fix/http-response-body-limits
Oct 14, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 7 commits into
mainfrom
fix/http-response-body-limits

Conversation

@baywet

@baywet Vincent Biret (baywet) commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

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

  • 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.
  • 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

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Copilot assisted with implementation, tests, documentation, and this PR description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • 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.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9015c274-1215-4163-94db-643befd21f8d
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:58
@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible labels Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Raw-deflate prefix ambiguity and zero-byte encoded responses can produce incorrect decoding behavior.

2 open findings
What changed in this PR

Adds bounded HTTP response handling for declarative workflows, including compressed and intermediate responses.

Changes:

  • Adds configurable 10 MiB wire and decoded-stage limits.
  • Supports bounded gzip/deflate decoding and stream cleanup.
  • Documents behavior and adds extensive tests.
File Description
python/​packages/​declarative/​AGENTS.md Records handler constraints.
python/​packages/​declarative/​README.md Documents limits and encodings.
python/​packages/​declarative/​.../​_http_handler.py Implements bounded response decoding.
python/​packages/​declarative/​tests/​test_default_http_request_handler.py Tests limits, decoding, redirects, authentication, and cleanup.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@github-code-quality

github-code-quality Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: Python

Python / code-coverage/python

The overall line coverage in commit 99633f4 in the fix/http-response-bo... branch remains at 92%, unchanged from commit ec47b90 in the main branch.

Show a line coverage summary of the most impacted files.
File main ec47b90 fix/http-response-bo... 99633f4 +/-
packages/declar...http_handler.py 100% 98% -2%

Updated October 13, 2026 20:51 UTC

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9015c274-1215-4163-94db-643befd21f8d

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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.

Affected areas: python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Extremely large valid limits currently cause an unexpected zlib OverflowError.

0 open findings

2 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject or safely clamp oversized max_response_body_bytes values

python/​packages/​declarative/​agent_framework_declarative/​_workflows/​_http_handler.py:419

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.

🧠 Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9015c274-1215-4163-94db-643befd21f8d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9015c274-1215-4163-94db-643befd21f8d
@baywet

Copy link
Copy Markdown
Member Author

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Bodyless 205 responses are incorrectly treated as incomplete encoded responses.

0 open findings

Previously missed (3)

In code that hasn't changed since last review

Medium severity Handle 205 Reset Content as a bodyless response

python/​packages/​declarative/​agent_framework_declarative/​_workflows/​_http_handler.py:373

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.

Low severity 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.

Low severity Test empty encoded 205 Reset Content responses

python/​packages/​declarative/​tests/​test_default_http_request_handler.py:988

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.

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The bodyless-response logic omits HTTP 1xx, 205, and successful CONNECT responses.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle all bodyless responses before decoding Content-Encoding

python/​packages/​declarative/​agent_framework_declarative/​_workflows/​_http_handler.py:374

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.

🧠 Review effort: Balanced

@baywet

Copy link
Copy Markdown
Member Author

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The security-sensitive decoding and HTTPX authentication/redirect integration warrant final human validation.

0 open findings

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The implementation relies on private HTTPX internals despite allowing future HTTPX 0.x releases.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Medium severity Avoid relying on private HTTPX internals

python/​packages/​declarative/​agent_framework_declarative/​_workflows/​_http_handler.py:33

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.

🧠 Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9015c274-1215-4163-94db-643befd21f8d
@baywet

Copy link
Copy Markdown
Member Author

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Security-sensitive streaming, decoding, authentication, and redirect behavior relies on private HTTPX internals and warrants final human review.

0 open findings

🧠 Review effort: Balanced

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAF Automated Review — Iteration 2

Result: No findings
Scope: 6 net-new commit(s): 328f5821f3a5, 4beaa9645374, ca54a602166c, 5926ef896530, 666573324ace, 99633f4652c7
Model: gpt-5.6-sol

Overview

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.

Merged via the queue into main with commit 76543b4 Oct 14, 2026
57 checks passed
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) deleted the fix/http-response-body-limits branch October 14, 2026 09:12

This branch was successfully deployed

1 active deployment
github-app-auth — 99633f46 Deployed Oct 13, 2026 by baywet via team_check #6728
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants