Skip to content

python: document output_buffer_limit's block-granularity behavior#1464

Open
MukundaKatta wants to merge 1 commit into
google:masterfrom
MukundaKatta:codex/output-buffer-limit-docstring
Open

python: document output_buffer_limit's block-granularity behavior#1464
MukundaKatta wants to merge 1 commit into
google:masterfrom
MukundaKatta:codex/output-buffer-limit-docstring

Conversation

@MukundaKatta
Copy link
Copy Markdown

Refs #1460.

The docstring for Decompressor::process in python/_brotli.c describes output_buffer_limit as an absolute cap:

The maximum size of the output buffer. If set, the output buffer will not grow once its size equal or exceeding that value.

In practice the check is only applied inside the BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT branch of the decompression loop, so the returned output can exceed the requested limit by up to one decoder block. Repro on x86_64 from #1460:

limit bytes returned
1 32752
1024 32752
32752 32752
32753 98272

Rewrite the docstring so it describes what the code actually does — a best-effort, block-granularity cap — instead of implying a byte-precise hard limit. This matches the existing test assertions in python/tests/decompressor_test.py, which check against MIN_OUTPUT_BUFFER_SIZE (32 KiB) rather than the caller-supplied limit.

Testing

  • Docstring-only change. No runtime behaviour change.
  • Existing python/tests/decompressor_test.py::test_decompress_with_limit and test_changing_limit already assert len(decompressed_chunk) <= MIN_OUTPUT_BUFFER_SIZE (32 KiB), consistent with the new docstring wording; they continue to pass.
  • I intentionally left the test assertions as they are — per-block cap matches what the code does, and tightening the test to the caller-supplied limit would invert the behaviour the docstring now documents.

Out of scope

The issue also suggests considering whether output_buffer_limit should be enforced at byte granularity. That would be a behavioural change in the Buffer_Grow path (the unconditional initial Buffer_Grow() call allocates MIN_OUTPUT_BUFFER_SIZE bytes before the loop even sees the limit), and likely a separate discussion; this PR only aligns the documented contract with the shipped behaviour.

The python _brotli.c docstring for Decompressor::process describes
output_buffer_limit as an absolute cap: the buffer 'will not grow once
its size is equal to or exceeds that value.' In practice, the cap is
checked inside the BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT branch only,
so the returned output can exceed the requested limit by up to one
decoder block (observed on x86_64: a 1-byte limit yields a 32,752-byte
payload, a 32,753-byte limit yields 98,272 bytes).

Rewrite the docstring so it describes what the code actually does --
a best-effort, block-granularity cap -- instead of implying a
byte-precise hard limit. No runtime behavior change; test assertions
already check against MIN_OUTPUT_BUFFER_SIZE rather than the caller
limit, so they still pass unchanged.

Refs google#1460.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant