iframe-proxy

bdraco · 2026-07-26T21:01:23Z

What do these changes do?

The llhttp callbacks in the C parser are declared except -1, which promises Cython that a -1 return always has an exception pending. The callbacks actually return -1 as a plain abort status after catching the error and stashing it for later delivery. That is fine when llhttp calls them, but feed_data also calls cb_on_body directly to drain pending decompressed data; when the decompressor raises during that drain, Cython takes the error path with no exception set and crashes in __Pyx_AddTraceback on Python 3.12 and newer. This happens in the wild with brotlicffi 1.2, see python-hyper/brotlicffi#225 and cython/cython#7857.

The callbacks are now declared except? -1 so Cython checks PyErr_Occurred() before taking the error path, and the drain call handles the failure explicitly; the error reaches the user through the payload as ContentEncodingError like other payload errors.

Are there changes in behavior for the user?

A decompression failure while draining pending data now raises ContentEncodingError instead of crashing the interpreter.

Is it a substantial burden for the maintainers to support this?

No, the exception declarations now match what the callbacks actually do.

Related issue number

Fixes #13203.

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES/ folder
- name it <issue_or_pr_num>.<type>.rst (e.g. 588.bugfix.rst)
- if you don't have an issue number, change it to the pull request
  number after creating the PR
  - .bugfix: A bug fix for something the maintainers deemed an
    improper undesired behavior that got corrected to match
    pre-agreed expectations.
  - .feature: A new behavior, public APIs. That sort of stuff.
  - .deprecation: A declaration of future API removals and breaking
    changes in behavior.
  - .breaking: When something public is removed in a breaking way.
    Could be deprecated in an earlier release.
  - .doc: Notable updates to the documentation structure or build
    process.
  - .packaging: Notes for downstreams about unobvious side effects
    and tooling. Changes in the test invocation considerations and
    runtime assumptions.
  - .contrib: Stuff that affects the contributor experience. e.g.
    Running tests, building the docs, setting up the development
    environment.
  - .misc: Changes that are hard to assign to any of the above
    categories.
- Make sure to use full sentences with correct case and punctuation,
  for example:
```
Fixed issue with non-ascii contents in doctest text files
-- by :user:`contributor-gh-handle`.
```
  Use the past tense or the present tense a non-imperative mood,
  referring to what's changed compared to the last released version
  of this project.

Test output

The new test reproduces the crash on the unfixed extension:

tests/test_http_parser.py Fatal Python error: Segmentation fault

With the fix, on Python 3.14.2 with the C extension built:

tests/test_http_parser.py::test_decompress_error_while_draining_pending_data[py-parser] PASSED
tests/test_http_parser.py::test_decompress_error_while_draining_pending_data[c-parser] PASSED
835 passed, 2 skipped, 6 deselected, 3 xfailed in 1.89s

Pure Python mode:

AIOHTTP_NO_EXTENSIONS=1: 1 passed

The regenerated call site in _http_parser.c now guards the error path:

if (unlikely(__pyx_t_5 == ((int)-1) && PyErr_Occurred())) __PYX_ERR(0, 637, __pyx_L1_error)

Drafted with Claude Code (Fable 5); reviewed by bdraco.

The llhttp callbacks are declared except -1 but return -1 as a plain abort status with the exception already caught and stashed. That is fine when llhttp calls them, but feed_data also calls cb_on_body directly to drain pending decompressed data, and Cython takes the error path on -1 without checking PyErr_Occurred, crashing in __Pyx_AddTraceback on Python 3.12+. Declare the callbacks except? -1 so -1 without a pending exception is a legal value and handle the drain failure explicitly.

aiolibsbot · 2026-07-26T21:08:30Z

codecov · 2026-07-26T21:12:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.98%. Comparing base (4ae5d49) to head (8587f8c).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #13249   +/-   ##
=======================================
  Coverage   98.98%   98.98%           
=======================================
  Files         131      131           
  Lines       48993    49021   +28     
  Branches     2550     2551    +1     
=======================================
+ Hits        48496    48524   +28     
  Misses        373      373           
  Partials      124      124

Flag	Coverage Δ
Autobahn	`22.12% <5.00%> (-0.01%)`	⬇️
CI-GHA	`98.90% <100.00%> (+<0.01%)`	⬆️
OS-Linux	`98.67% <100.00%> (+<0.01%)`	⬆️
OS-Windows	`97.03% <100.00%> (+<0.01%)`	⬆️
OS-macOS	`97.93% <100.00%> (-0.01%)`	⬇️
Py-3.10	`98.11% <100.00%> (-0.01%)`	⬇️
Py-3.11	`98.38% <100.00%> (+<0.01%)`	⬆️
Py-3.12	`98.46% <100.00%> (+<0.01%)`	⬆️
Py-3.13	`98.44% <100.00%> (+<0.01%)`	⬆️
Py-3.14	`98.46% <100.00%> (+<0.01%)`	⬆️
Py-3.14t	`97.56% <100.00%> (+0.01%)`	⬆️
Py-pypy-3.11	`97.39% <100.00%> (+<0.01%)`	⬆️
VM-macos	`97.93% <100.00%> (-0.01%)`	⬇️
VM-ubuntu	`98.67% <100.00%> (+<0.01%)`	⬆️
VM-windows	`97.03% <100.00%> (+<0.01%)`	⬆️
cython-coverage	`37.97% <100.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

codspeed · 2026-07-26T21:25:45Z

Merging this PR will not alter performance

✅ 83 untouched benchmarks
⏩ 83 skipped benchmarks¹

_{Comparing fix-parser-drain-segfault (8587f8c) with master (25561d9)}

83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

aiolibsbot · 2026-07-26T21:30:11Z

PR Review — Fix segfault when decompression fails while draining pending data

Correct, minimal fix for a real 3.12+ segfault. Merge-ready.

Root cause is accurately diagnosed: except -1 promised Cython that a -1 return always carries a pending exception, but the callbacks return -1 as a plain abort after stashing the error. On the direct cb_on_body(b"", 0) drain call, Cython took the error path with no exception set and crashed in __Pyx_AddTraceback on 3.12+. Switching to except? -1 makes Cython guard on PyErr_Occurred() first — verified this generates the && PyErr_Occurred() check shown in the test output.
Applying except? -1 to every callback (not just cb_on_body) is consistent and required: each callback catches internally and stashes the error, and the function-pointer typedefs (llhttp_data_cb/llhttp_cb) had to be changed in lockstep so exception specs stay compatible.
The explicit if result == -1: return EMPTY_FEED_DATA_RESULT is necessary alongside the declaration change — without it, execution would fall through to llhttp_execute on a parser already in the error state.
_more_data_available = False in the cb_on_body error path correctly prevents re-entering the drain (and re-hitting the broken decompressor) on the next feed_data.
Verified both parsers deliver the failure via the payload: the py-parser catches at http_parser.py:540 and only re-raises InvalidHeader/TransferEncodingError, so ContentEncodingError surfaces through payload.exception(). This means the prior review's suggestion API changed to create_server #1 (try/except/else asymmetry) is resolved — the author's simplified direct assert is actually a stronger test that asserts no raise for both parametrizations.
Test detail is careful: the 0x78 first byte avoids the deflate-header sniff swapping the decompressor (which would reset the broken decoder's call counter), and the 1 MiB first output forces the high-water pause so pending data remains to drain.
Changelog fragment, PR-number symlink, and :user: attribution follow repo conventions.
Only note: the drain error path does not clear self._payload, unlike the llhttp error path — non-blocking symmetry suggestion below.

🟢 Suggestions

1. Drain error path leaves self._payload set, unlike the llhttp error path
aiohttp/_http_parser.pyx:637-639

The new early-return correctly bails out when cb_on_body reports an error during the pending-data drain. One asymmetry with the normal error path is worth a look:

When llhttp_execute reports a payload error, feed_data sets self._payload = None (line 692) before returning.
This new drain path returns EMPTY_FEED_DATA_RESULT without clearing self._payload.

Because self._payload stays set, a later feed_eof() will enter the if self._payload is not None: branch (line 594) and, for this Content-Length response, raise ContentLengthError — a second, arguably redundant error after the ContentEncodingError already delivered to the payload.

This is not a crash and is strictly better than the previous segfault, so it does not block merge. But for symmetry and to avoid a confusing secondary error on connection close, consider clearing self._payload (and/or the relevant EOF state) on this error path too. Unverified in an end-to-end connection-close scenario — flagging from static reading of feed_eof.

if result == -1:
    # Exception already delivered to the payload in cb_on_body.
    return EMPTY_FEED_DATA_RESULT

Checklist

Exception declarations match actual callback behavior (except? -1)
Segfault path fixed; error delivered to user as ContentEncodingError
No re-entry into broken drain after error (_more_data_available reset)
Regression test reproduces crash and passes on both parsers
Changelog fragment + PR symlink + attribution correct
No state left inconsistent on error path — suggestion ＃1

Automated review by Kōan (Claude) HEAD=8587f8c 3 min 9s

bdraco · 2026-07-26T22:28:23Z

Drain error path leaves self._payload set, unlike the llhttp error path
aiohttp/_http_parser.pyx:637-639

decline that. Too risky for this and should be kept narrow

patchback · 2026-07-27T17:51:33Z

Backport to 3.14: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.14/3c6cf44da3e5c984655c58784818ce5bf3f4d05c/pr-13249

Backported as #13272

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2026-07-27T17:51:45Z

…ails while draining pending data (#13273) **This is a backport of PR #13249 as merged into master (3c6cf44).** Co-authored-by: J. Nick Koston <nick@koston.org>

…ails while draining pending data (#13272) **This is a backport of PR #13249 as merged into master (3c6cf44).** Co-authored-by: J. Nick Koston <nick@koston.org>

pyba-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jul 26, 2026

Add PR number changelog symlink

6089a5c

bdraco added backport-3.14 Trigger automatic backporting to the 3.14 release branch by Patchback robot backport-3.15 Trigger automatic backporting to the 3.15 release branch by Patchback robot labels Jul 26, 2026

Assert drain decompression error is delivered via the payload

8587f8c

bdraco marked this pull request as ready for review July 26, 2026 22:28

bdraco requested review from asvetlov and webknjaz as code owners July 26, 2026 22:28

Dreamsorcerer merged commit 3c6cf44 into master Jul 27, 2026
53 checks passed

Dreamsorcerer deleted the fix-parser-drain-segfault branch July 27, 2026 17:51

This was referenced Jul 27, 2026

[PR #13249/3c6cf44d backport][3.14] Fix segfault when decompression fails while draining pending data #13272

Merged

[PR #13249/3c6cf44d backport][3.15] Fix segfault when decompression fails while draining pending data #13273

Merged

Odacchi mentioned this pull request Aug 7, 2026

ContentLengthError raised despite all bytes received: paused parser drops on_message_complete when server closes early (3.14 regression) #13348

Closed

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

Fix segfault when decompression fails while draining pending data - #13249

Fix segfault when decompression fails while draining pending data#13249
Dreamsorcerer merged 3 commits into
masterfrom
fix-parser-drain-segfault

bdraco commented Jul 26, 2026

Uh oh!

aiolibsbot commented Jul 26, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 26, 2026 •

edited

Loading

Uh oh!

codspeed Bot commented Jul 26, 2026 •

edited

Loading

Uh oh!

aiolibsbot commented Jul 26, 2026

Uh oh!

bdraco commented Jul 26, 2026

Uh oh!

Uh oh!

patchback Bot commented Jul 27, 2026 •

edited

Loading

Uh oh!

patchback Bot commented Jul 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

bdraco commented Jul 26, 2026

What do these changes do?

Are there changes in behavior for the user?

Is it a substantial burden for the maintainers to support this?

Related issue number

Checklist

Uh oh!

aiolibsbot commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed Bot commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

aiolibsbot commented Jul 26, 2026

PR Review — Fix segfault when decompression fails while draining pending data

🟢 Suggestions

Checklist

Uh oh!

bdraco commented Jul 26, 2026

Uh oh!

Uh oh!

patchback Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.14: 💚 backport PR created

Uh oh!

patchback Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.15: 💚 backport PR created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aiolibsbot commented Jul 26, 2026 •

edited

Loading

codecov Bot commented Jul 26, 2026 •

edited

Loading

codspeed Bot commented Jul 26, 2026 •

edited

Loading

patchback Bot commented Jul 27, 2026 •

edited

Loading

patchback Bot commented Jul 27, 2026 •

edited

Loading