poll_read data is buffered without bounds

[jvimal]

h3/h3/src/stream.rs

Line 433 in 84316da

buf: BufList::new(),

The poll_read() on BufRecvStream (wrapping the underlying QUIC provider's stream) is buffering chunks in-memory in a BufList that appears to grows without bounds:

h3/h3/src/stream.rs

Lines 448 to 450 in 84316da

	if let Some(mut data) = data {
	self.buf.push_bytes(&mut data);
	Poll::Ready(Ok(false))

In some tests that I ran over loopback to send and receive data as fast as possible, if the client does not consume from BufRecvStream quickly (for any reason) I observed that the client memory grows without bounds. This should ideally not be the case -- a slow consumer should back-pressure the sender. This problem does not exist if I use the underlying QUIC provider's stream directly.

I don't have a handy reproducer that I can share at the moment, but I can work on one if it'll be useful.

[Ruben2424]

Thanks for reporting this.

Looking at the code, it could happen, when the first chunk of memory read from the quic stream holds multiple http3 frames. Then for each frame it tries to read from the buffer again adding a potential received memory chunk to the buffer for each read. This only happens when the server sends faster then the reader reads.

IMO the best way to solve this is to get rid of buffer BufList.
The purpose of BufList, is to buffer data when a read chunk of memory holds more than one frame. IIRC, another purpose is to hold the data until a complete frame is arrived.
Removing the BufList would require to refactor the Frame parsing to a kind of stateful parser in case a read chunk holds only a part of the data. Then only 1 memory chunk would need to be buffered.
The most Frames hold only protocol data so it should be possible without extra buffering. The exception:

• Data: The data in Data Frames goes directly to the user, so no buffering required.
• Header: Header data needs to be buffered, but the specification already introduces a limit here: See MAX_FIELD_SECTION_SIZE settings parameter. Note, the MAX_FIELD_SECTION_SIZE setting is not applied correctly at the moment. See Start decoding headers once enought data is received #294. I am working on a solution for this in WIP Start decoding headers once enought data is received and add dynamic table #296