Skip to content

Parse chunk size strictly in Response.parse - #1780

Closed
Bountybette wants to merge 1 commit into
python-websockets:mainfrom
Bountybette:strict-chunk-size
Closed

Bountybette wants to merge 1 commit into
python-websockets:mainfrom
Bountybette:strict-chunk-size

Conversation

@Bountybette

Copy link
Copy Markdown

Checklist

  • I personally discussed the issue with a maintainer before opening this pull request. I understood and followed their requirements.
  • I reviewed the design choices and implementation details of the changes. I can explain them without relying on an AI coding agent.
  • I wrote a short, specific pull request description that makes it easy for maintainers to understand the problem and the solution.
  • I am aware that maintainers have AI coding agents. Merely throwing an AI at an issue wastes their time. It comes across as a lack of respect.

Problem

Follow-up to 0437fbc, which made Content-Length strict. read_body still parses the chunk-size line with int(raw_chunk_size, 16), so +5, 0x5, 0_5 and padded values are accepted, and -1 reaches StreamReader.read_exact and fails its assert n >= 0 with a bare AssertionError (under python -O, read_exact(-1) instead returns everything buffered except the last byte). A server sending a malformed chunked error response triggers this. It is a strictness issue, not a vulnerability: the body is only read for display and the connection is not reused.

Solution

Validate the chunk size against 1*HEXDIG (RFC 9112 section 7.1) before converting it and raise ValueError, like the Content-Length fix. Trailing BWS before ; is stripped so valid chunk extensions such as 6 ;foo=bar keep working. An alternative was rejecting negatives only after int(), but that would still accept 0x and _ forms.

Also raise ValueError instead of AssertionError for negative values.
@aaugustin

Copy link
Copy Markdown
Member

Checklist?

@aaugustin

Copy link
Copy Markdown
Member

As it stands this PR comes across as "AI found something that doesn't happen in real life and creates unnecessary work for maintainers".

@aaugustin

Copy link
Copy Markdown
Member

Fixed in bc17da4 with less code + also covering the parsing of the HTTP status code.

@aaugustin aaugustin closed this Oct 16, 2026
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.

2 participants