search

private-octopus / picoquic

Minimal implementation of the QUIC protocol
MIT License
527 stars 156 forks source link

`max_datagram_frame_size` not set in combination with `H3 Datagram` setting #1552

Closed MPK1 closed 9 months ago

MPK1 commented 9 months ago

In HTTP/3 interoperation with aioquic, I noticed that picoquic does not set the max_datagram_frame_size transport parameter during the handshake, causing the whole transmission to fail (see here). Right after the handshake, picoquic sends the HTTP/3 setting to enable H3 datagrams. This causes aioquic to crash, as they check if the mentioned transport parameter is set (see here).

Even though, as far as I understand, the H3 datagrams could also work without the QUIC Datagram Extension, picoquic should probably still set this transport parameter as it supports the QUIC Datagram Extension. According to RFC9221 Section 3, the transport parameter should be set to indicate support for datagram frames.

huitema commented 9 months ago

In my tests, I see that the picoquic server is setting the max_datagram_frame_size parameter to 1536 bytes... but only if the client sets it to a non zero value. Is aoiquic also setting up that parameter? (Also, aioquic should probably not crash...)

huitema commented 9 months ago

@MPK1, I think that PR #1553 fixed the problem. Can you confirm?

MPK1 commented 9 months ago

@huitema thank you for the fast reply! You're right, aioquic should definitely not close the connection in this case. Aioquic tends to behave a bit strange sometimes anyways, I'm just doing some H3 interoperability and this was one of the few remaining issues with lots of implementations :)

I my tests, the picoquic client, as well as the aioquic client, does not set the max_datagram_frame_size transport parameter, resulting in the respective other to also not send it. Your PR seems good to me, I will review it tomorrow in more detail. I'm not that deep into RFC9297, so I'm not sure about the capsules and how it is possible to use H3 datagrams without QUIC datagrams.

Best regards!

MPK1 commented 9 months ago

@huitema I just found out that these changes caused the SETTINGS frame to be sent without a stream type. After that, I noticed that you were faster in finding and fixing this issue 😁 And in case you don't already know: nginx was actually the only other QUIC library that closed the connection (even though it shouldnt as I understand the RFC) because the control stream wasn't the first stream opened by the client.