search

quicwg / base-drafts

Internet-Drafts that make up the base QUIC specification
https://quicwg.org
1.62k stars 203 forks source link

STOP_SENDING to QPACK streams #4984

Closed tatsuhiro-t closed 2 years ago

tatsuhiro-t commented 2 years ago

https://datatracker.ietf.org/doc/html/draft-ietf-quic-qpack-21#section-4.2

These streams MUST NOT be closed. Closure of either unidirectional stream type MUST be treated as a connection error of type H3_CLOSED_CRITICAL_STREAM.

I'd like to clarify that whether this effectively says that receiver is not allowed to send STOP_SENDING. STOP_SENDING is elicit RESET_STREAM that closes a stream.

An endpoint that receives a STOP_SENDING frame MUST send a RESET_STREAM frame if the stream is in the "Ready" or "Send" state.

STOP_SENDING itself does not close a stream, but it is an action to request a closure. I found in interop runner test, one implementation sends STOP_SENDING to QPACK streams. My implementation sends RESET_STREAM when it is received as mandated by spec, and upon stream closure, the connection is closed because QPACK streams are closed, then the test fails.

HTTP draft explicitly says that requesting closure for the control stream is prohibited:

The sender MUST NOT close the control stream, and the receiver MUST NOT request that the sender close the control stream.

It would be nice to add the similar statement to QPACK streams.

martinthomson commented 2 years ago

It's super-late in the process, but maybe the editors can see a way to squeeze an editorial change in.

LPardue commented 2 years ago

Thanks Tatsuhiro. I agree, IMO the intention was always for consistent treatment of open control streams and open qpack streams when it comes to not closing them.

Finding a way to address this during AUTH48 would be nice.

MikeBishop commented 2 years ago

I agree, this was the intention -- closing the stream is treated as an error, and being asked to commit an error is itself an error. One could squint and say it's covered by the existing text -- upon receipt of the STOP_SENDING, the send is required to close the stream; closure of the stream is already required to be treated as an error.

It's technically a normative change, but there's no effective change in behavior.

afrind commented 2 years ago

If it's a normative change, is it still OK to change in auth48 just with a WG consensus call or is more process required?

mirjak commented 2 years ago

The AD has to decide what to do and approve this change to the RFC editor.

LPardue commented 2 years ago

Since the fix includes a gentle normative change, marking this as a design issue and we'll put out a consensus call to the list.

MikeBishop commented 2 years ago

Consensus was declared, so #4985 was merged.