aiortc / aioquic

QUIC and HTTP/3 implementation in Python
BSD 3-Clause "New" or "Revised" License
1.69k stars 237 forks source link

FINs are swallowed if data is fully ACKed #438

Closed mhils closed 10 months ago

mhils commented 10 months ago

aioquic currently swallows FINs if the following two conditions are both met:

  1. The stream reader is finished (so this typically affects servers)
  2. The remaining stream data is acked while the FIN is queued for sending.

Here is a minimal repro showcasing the issue:

from aioquic.quic.packet_builder import QuicDeliveryState
from aioquic.quic.stream import QuicStreamSender

s = QuicStreamSender(0, True)
s.write(b"foo")
print(f"{s.get_frame(4096, 4096)=}")
print(f"{s.is_finished=}")

s.write(b"", end_stream=True)
s.on_data_delivery(QuicDeliveryState.ACKED, 0, 3)
print(f"{s.is_finished=}")  # returns true...
print(f"{s.get_frame(4096, 4096)=}")  # but still produces a packet!

This behavior makes QuicConnection._write_application discard the stream prematurely under some circumstances:

https://github.com/aiortc/aioquic/blob/ac18ff69991b676576b139c2c4b17d4890d63c82/src/aioquic/quic/connection.py#L2831-L2835

I'll follow up with a PR momentarily. This made me go slightly crazy in https://github.com/mitmproxy/mitmproxy/pull/6557 because it only appeared as a Heisenbug every 60mins of CI time, but here we are. I'm happy to start 2024 off on a good note. 😅