Repository navigation
Bound buffered frames by their declared length, before reading the payload - #366
seanmonstar wants to merge 1 commit into
Conversation
…yload
FrameDecoder accumulates into a BufList until a frame's whole declared payload
has arrived. Consuming each chunk is what releases the flow-control credit for
the next one, so credit is returned as fast as the decoder accumulates and no
transport window bounds the buffer. A peer declaring a large HEADERS is held in
memory in full before max_field_section_size can reject it, and a peer declaring
a large unknown frame is held in full before it can be ignored. The bound is the
sender's rate times how long the connection is allowed to live.
Frame::decode now decides from the type and the declared length alone:
* DATA is untouched — it carries a length and streams its body.
* A reserved HTTP/2 type is a connection error decided by the type, so its
payload is no longer waited for.
* An unknown type returns SkipUnknown with its length, and FrameDecoder
discards the payload as it arrives instead of storing it to drop it. RFC
9114 asks that unknown frames be ignored; ignoring one used to cost its
declared length. Refusing it instead would bound memory just as well and
would not be conformant.
* A frame that must be buffered to be parsed is refused with TooLarge if its
declared length exceeds the decoder's ceiling, before any of it is read, and
the accumulated state is released rather than left for the caller.
The ceiling defaults to 1 MiB — generous beside anything legitimate, since field
sections run to kilobytes and the control frames to tens of bytes, and finite,
which is the property that was missing. An endpoint that advertises a larger
max_field_section_size raises it to match: the ceiling is a memory bound, and
must never be tighter than what the endpoint has told peers it accepts, or a
field section would be refused from its encoded length before QPACK can report
the uncompressed size that RFC 9114 4.2.2 is written in terms of.
What the refusal means depends on where the frame arrived, so FrameStreamError
reports it rather than deciding: an oversized HEADERS on a request stream is
HeaderTooBig, a message-level rejection that leaves the connection alone, while
on the control stream — where every legitimate frame is tens of bytes and the
stream itself is critical — it is H3_EXCESSIVE_LOAD.
Two existing tests changed behaviour rather than shape: unknown_frame_type and
reserved_frame now observe SkipUnknown from the header instead of UnknownFrame
after the payload.
|
Thanks for working on this. I tested PR head d1b6b8d with the existing QUIC test helpers: the peer sends only a HEADERS frame prefix declaring a 2 MiB payload and keeps the stream open. With It looks like the default request path bypasses the nominal 1 MiB ceiling: Could the advertised default field-section limit be made finite, or could the encoded-frame limit otherwise stay finite while preserving the advertised-setting semantics? I can provide the small failing regression test if useful. |
FrameDecoder accumulates into a BufList until a frame's whole declared payload has arrived. Consuming each chunk is what releases the flow-control credit for the next one, so credit is returned as fast as the decoder accumulates and no transport window bounds the buffer. A peer declaring a large HEADERS is held in memory in full before max_field_section_size can reject it, and a peer declaring a large unknown frame is held in full before it can be ignored. The bound is the sender's rate times how long the connection is allowed to live.
Frame::decode now decides from the type and the declared length alone:
The ceiling defaults to 1 MiB — generous beside anything legitimate, since field sections run to kilobytes and the control frames to tens of bytes, and finite, which is the property that was missing. An endpoint that advertises a larger max_field_section_size raises it to match: the ceiling is a memory bound, and must never be tighter than what the endpoint has told peers it accepts, or a field section would be refused from its encoded length before QPACK can report the uncompressed size that RFC 9114 4.2.2 is written in terms of.
What the refusal means depends on where the frame arrived, so FrameStreamError reports it rather than deciding: an oversized HEADERS on a request stream is HeaderTooBig, a message-level rejection that leaves the connection alone, while on the control stream — where every legitimate frame is tens of bytes and the stream itself is critical — it is H3_EXCESSIVE_LOAD.
Two existing tests changed behaviour rather than shape: unknown_frame_type and reserved_frame now observe SkipUnknown from the header instead of UnknownFrame after the payload.
Patch was sent to me out of band. I've included original author.