quiche icon indicating copy to clipboard operation
quiche copied to clipboard

Correctly handle write_epoch() when an application close was done and…

Open normanmaurer opened this issue 4 years ago • 3 comments

… no streams are left to process

Motivation:

If all streams are closed and an application close is done we need ensure we still return an non error result from write_epoch as otherwise we will never send the application close to the remote peer

Modifications:

Change if branch to also respect app_error and so application close

Result:

Correctly send APPLICATION_CLOSE in all cases

normanmaurer avatar Dec 11 '20 19:12 normanmaurer

If you give me a bit of a hand here I can also try to write a unit test for it :) .. I found it while doing some test within https://github.com/netty/netty-incubator-codec-quic

normanmaurer avatar Dec 11 '20 19:12 normanmaurer

This will cause application close frames to be sent in Initial and Handshake packets, which is a violation of §12.5 of the transport draft:

CONNECTION_CLOSE frames signaling application errors (type 0x1d) MUST only appear in the application data packet number space.

Also, we already do trigger sending a 1-RTT packet when an application error is set: https://github.com/cloudflare/quiche/blob/master/src/lib.rs#L3828 ...and we even have a test for this case: https://github.com/cloudflare/quiche/blob/master/src/lib.rs#L8479

But of course there could be a bug somewhere, so it would be helpful to understand your case better.

ghedo avatar Dec 14 '20 12:12 ghedo

Let me try to adjust the test case as I only have some code that shows the issue with my java integration

Am 14.12.2020 um 13:32 schrieb Alessandro Ghedini [email protected]:

 This will cause application close frames to be sent in Initial and Handshake packets, which is a violation of §12.5 of the transport draft:

CONNECTION_CLOSE frames signaling application errors (type 0x1d) MUST only appear in the application data packet number space.

Also, we already do trigger sending a 1-RTT packet when an application error is set: https://github.com/cloudflare/quiche/blob/master/src/lib.rs#L3828 ...and we even have a test for this case: https://github.com/cloudflare/quiche/blob/master/src/lib.rs#L8479

But of course there could be a bug somewhere, so it would be helpful to understand your case better.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

normanmaurer avatar Dec 14 '20 12:12 normanmaurer