Repository navigation
start new dev branch; add audit file - #1987
Merged
Merged
Conversation
…rupt input
Add tests that show the permessage-* compression backends do not handle
the decompression budget, or a corrupt compressed frame, the same way.
Tests only; no fix yet.
New tests:
- test_websocket_compress.py, DecompressBudgetUniformTests (all codecs):
- test_budget_zero_rejects: budget 0 must raise PayloadExceededError,
also for a 1-octet message;
- test_one_octet_over_budget_rejects;
- test_over_budget_across_frames_rejects (remaining budget per frame);
- test_bomb_inflation_bounded_by_budget: rejecting a 32 MiB zero-run
against a 64 KiB budget must allocate < 1 MiB (tracemalloc peak,
black-box; skipped on PyPy, which has no tracemalloc).
- test_websocket_compress_corrupt.py (new, backend-neutral): a corrupt
compressed frame must fail the connection without a decoder exception
escaping the protocol, and deliver nothing (message + streaming mode).
- test_aio_websocket.py: test_corrupt_compressed_frame_closes_connection
- on asyncio, a corrupt compressed frame must close the connection.
- test_websocket_max_message_size.py: split _build_compressed_body_frame()
out of _build_compressed_frame(), so a test can send a raw body.
Verified locally on asgard1, CPython 3.14 and PyPy 3.11, pytest (asyncio)
and trial (Twisted). Failing as intended:
- test_bomb_inflation_bounded_by_budget[brotli] (CPython only):
32 MiB traced peak against the 1 MiB limit;
- test_budget_zero_rejects[deflate, size=1]: returns 1 octet;
- CorruptCompressedFrameTests[deflate, bzip2, brotli], both modes:
zlib.error / OSError / brotli.error escape the protocol;
- test_corrupt_compressed_frame_closes_connection (asyncio): the
connection is left open.
All other new tests pass; existing tests unchanged and passing. The
snappy cases are skipped in CI (python-snappy is not installed there);
with python-snappy 0.7.3 locally, they fail too: bomb (32 MiB peak) and
corrupt frame (accepted, empty message delivered).
Expected CI result: RED - the test jobs fail (pytest/asyncio and
trial/Twisted, CPython and PyPy) on the tests listed above. Lint,
typing, build and wheel jobs are unaffected and should stay green.
Related: crossbario#1986
Note: This work was completed with AI assistance (Claude Code).
…ut handling All four permessage-* compression extensions now handle the decompression budget, and input they cannot decompress, the same way. Budget (raise PayloadExceededError, and stop near the budget instead of inflating a whole frame first): - brotli: Decompressor.process(output_buffer_limit=budget + 1); over budget if more came back, or if the decoder holds back output (can_accept_more_data() is False). brotlicffi stops exactly at the limit, CPython's brotli at its output-buffer granularity (e.g. ~2 MiB for 1 MiB). - snappy: python-snappy's StreamDecompressor has no output limit, so it is fed one framing chunk at a time (a chunk inflates to at most 64 KiB; the decoder rejects larger ones). Pending input is a bytearray, so input that trickles in is appended, not copied anew per call. - deflate: the 1 octet let through once the budget is used up is now rejected too. - bzip2: unchanged (already exact). Corrupt input (new autobahn.exception.PayloadCorruptError, a RuntimeError like PayloadExceededError, raised "from" the library error): - deflate: zlib.error (also from end_decompress_message); - bzip2: OSError / EOFError, and a stream not ended at message end; - brotli: brotli.error / brotlicffi.error, and a stream not finished at message end; - snappy: OSError / snappy.UncompressError, an invalid chunk header (rejected at once: stream identifier length != 6, oversized data chunk, reserved type 0x02-0x7f), and input left over at message end. snappy no longer accepts garbage silently. - protocol.py: PayloadCorruptError from decompress_message_data() or end_decompress_message() fails the connection with 1007 (invalid frame payload data), via _invalid_payload(). - autobahn.exception.__all__ now also lists Disconnected. asyncio: an exception escaping _dataReceived() in the receive loop (_consume) is logged and the connection dropped (transport.abort()), as on Twisted. Before, the loop stopped and left the connection open, unserviced. Docs: the PerMessageCompress contract docstring, the protocol.py comment and the changelog (in the not yet renamed 26.9.1 section, which becomes 26.10.1) describe the new behaviour. Dependencies: compress extra floors brotli>=1.2.0 / brotlicffi>=1.2.0, the first versions with output_buffer_limit (checked in the 1.1.0 / 1.1.0.0 and 1.2.0 / 1.2.0.0 sdists). dev extra: python-snappy>=0.7.3 (test-only), so the snappy cases run in CI. CI: new main.yml job "unit-tests": trial + pytest on cpy314, cpy313, cpy312, cpy311 and pypy311 (fail-fast off), then `just check-compressors`, which fails unless all four extensions are available. Until now the unit tests ran in CI on cpy314 only (quality-checks) - correction to the red commit db22cea, whose message expected red on PyPy too: PyPy unit tests did not run in CI. Tests: - test_websocket_compress.py: corrupt input -> PayloadCorruptError (all codecs); truncated message -> PayloadCorruptError (bzip2, brotli, snappy; not detectable for deflate, which ends a message with a sync flush); a message split into two frames at 9 offsets round-trips; bomb test bound allows one copy of the input (snappy buffers it: 1.5 MiB on the wire for the 32 MiB zero-run, so 1.71 MiB peak). - test_websocket_compress_corrupt.py: also asserts close code 1007 (failByDrop=False). - test_aio_websocket.py: any exception in data processing drops the connection. - test_websocket_max_message_size.py: LIMIT 128 -> 1024. Its precondition (compressed frame < LIMIT) failed for snappy (222 octets), which never showed because python-snappy was not installed in CI. - test_event_loop_1965.py (pre-existing failure, also on master): the child runs with -W error::DeprecationWarning, and on Python < 3.13 passlib imports the stdlib "crypt" module, which warns. Ignore that one warning. Never seen before, since CI ran the unit tests on 3.14 only. Verified locally on asgard1, full matrix, each run in its own network namespace, python-snappy 0.7.3 + brotli 1.2.0 / brotlicffi 1.2.0.2: CPython 3.11, 3.12, 3.13, 3.14 and PyPy 3.11 x Twisted/trial and asyncio/pytest x NVX on/off = 20 runs, all green: - trial: 340-341 passed per CPython run, 339-340 on PyPy; - pytest: 310-311 passed per CPython run, 309-310 on PyPy; - skips: only the existing ones, plus the bomb test on PyPy (no tracemalloc). Reporter PoC: peak RSS 76 -> 82 MiB (was 583) on CPython, unchanged on PyPy. Corrupt frame over a real socket: asyncio closes the connection (it stayed open), Twisted fails it with 1007. Expected CI result: GREEN - all jobs, including the new unit-tests job on all five Pythons. Related: crossbario#1986 Note: This work was completed with AI assistance (Claude Code).
… with context takeover
Add MultiMessageRoundtripTests: one compressor / decompressor pair carries
five messages (compressible, random, empty, > 64 KiB, tiny), for every
codec, with and without context takeover, with and without a decompression
budget. All other compression tests build a fresh pair per message, which
hid this. _make_compressor_pair() takes a no_context_takeover argument
(both directions; bzip2 has none).
Defect shown: with context takeover - which the default brotli offer
negotiates - PerMessageBrotli reuses the Compressor after
end_compress_message() called finish() on it, and the second message
fails ("brotli: encoder failed" on CPython, "Error encountered compressing
data." with brotlicffi on PyPy). The decompressor is reused after its
stream finished as well. deflate, bzip2 and snappy pass in both modes.
Local (asgard1): the new test fails only for permessage-brotli with
no_context_takeover=False (both budgets), on cpy314 and pypy311.
Expected CI result: RED - unit-tests (all five Pythons) and the
quality-checks unit test runs fail in
MultiMessageRoundtripTests.test_messages_roundtrip_on_one_pair; all other
jobs green.
Note: This work was completed with AI assistance (Claude Code).
…essage end_compress_message() finished the brotli Compressor, and with context takeover (negotiated by the default offer) start_compress_message() then reused it for the next message, which fails. The Decompressor was reused after its stream had finished as well. Fixed as compress_bzip2 does it: end_compress_message() / end_decompress_message() drop the finished object, so the next message gets a new one. Each message is therefore one complete brotli stream, as before for the first message and as with no context takeover, so the wire format is unchanged. The context takeover parameters are still negotiated, but brotli takes over no context across messages. Local (asgard1), with all four codecs installed: full matrix green, 20/20 (cpy311-314 + pypy311 x trial/pytest x NVX on/off): trial 341-342 passed per CPython run, 340-341 on PyPy; pytest 311-312 / 310-311. ruff check and ty pass. Expected CI result: GREEN - all jobs, including MultiMessageRoundtripTests on all five Pythons. Note: This work was completed with AI assistance (Claude Code).
…sions per extension) Add docs/websocket/compression.rst to the WebSocket programming guide. It lists, for each of the four per-message compression extensions, which Python package it uses on CPython and on PyPy and which compression library (and version) that package uses or bundles - checked in the sdists: - deflate / bzip2: stdlib zlib / bz2, as built into the interpreter - brotli: brotli 1.2.0 (CPython) and brotlicffi 1.2.0.0 / 1.2.0.2 (PyPy) all bundle libbrotli 1.2.0 (google/brotli) - snappy: python-snappy 0.7.3 is pure Python on cramjam (Rust/PyO3, cpyext on PyPy), whose snappy is the Rust snap crate 1.1.1 - not google/snappy; python-snappy declares no cramjam floor plus context takeover per extension (bzip2 and brotli: none), decompression limits and close codes 1009 / 1007. The old page under docs/work/ is excluded from the build and predates permessage-brotli; left as is. Also reword the PayloadCorruptError docstring: "permessage-*" opened RST emphasis and caused two Sphinx warnings in the generated API docs. Local (asgard1): just docs cpy314 builds, no warnings from the new page or autobahn.exception (remaining warnings are pre-existing). ruff check passes. Expected CI result: GREEN - all jobs (docs and docstring only). Note: This work was completed with AI assistance (Claude Code).
The section on decompression limits named PayloadCorruptError for invalid data, but only the close code (1009) for a message over the limit. Name PayloadExceededError there too, and say it is the same for all four extensions on both Twisted and asyncio, as for invalid data. Expected CI result: GREEN - all jobs (docs only). Note: This work was completed with AI assistance (Claude Code).
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
1 participant
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 #1986