quicwg / base-drafts

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

Rejecting duplicated push streams requirement is unenforceable #4828

Closed guoye-zhang closed 3 years ago

guoye-zhang commented 3 years ago

Each Push ID MUST only be used once in a push stream header. If a push stream header includes a Push ID that was used in another push stream header, the client MUST treat this as a connection error of type H3_ID_ERROR

For clients that have sent CANCEL_PUSH, they don't know if server's push stream will still arrive or not. They have to remember Push IDs forever in order to differentiate between cancelled push and completed push.

gloinul commented 3 years ago

I would appreciate some quick discussion if this issue is important to address before I approve or not.

MikeBishop commented 3 years ago

We could make this a MAY. The other formulation we've used is "MUST ... if the client detects", making the response mandatory if the condition is detected, but not specifying the required level of effort to detect it.

MikeBishop commented 3 years ago

I don't know that this issue is critical to address, but I agree that with certain (reasonable) implementation strategies this particular MUST becomes unenforceable / unachievable.

ianswett commented 3 years ago

+1 for "MUST if the client detects" if we're going to make a change.

LPardue commented 3 years ago

I think "MUST if detects" is suitable here - it points at the core problem. Simply s/MUST/MAY doesn't help people appreciate that might not be able to detect the condition. I'd also suggest that "MUST if detects" is an editorial change.

LPardue commented 3 years ago

@guoye-zhang can you confirm if #4829 addresses your issue?

guoye-zhang commented 3 years ago

Looks good!

LPardue commented 3 years ago

Thanks. In that case I believe we have addressed this issue with an editorial change. @gloinul please confirm if we can merge the related PR and incorporate the change in during the RFC editor process.

gloinul commented 3 years ago

Yes, the change looks good. I could actually include it as an RFC-edtior note in my approval message.