search

quicwg / base-drafts

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

Recommend careful behavior around push streams #4931

Closed MikeBishop closed 2 years ago

MikeBishop commented 2 years ago

Note that all of these recommendations are SHOULD NOT -- an implementation under memory pressure might choose to discard data aggressively, even at the risk of degrading the connection state. Closes #4930.

LPardue commented 2 years ago

Thanks Mike. The change looks fine to me, I'll let others debate the finer details.

Assuming this is the preferred direction, I'd ask that we hold the PR and not merge it until all the process stuff is in hand.

MikeBishop commented 2 years ago

@martinthomson, can you expand on why you think the latter changes aren't good? Noting that subpar things might happen if you abort reading seems like the RFC2119 definition of SHOULD NOT abort reading. Certainly we can explain the SHOULD NOT more than we currently do.

martinthomson commented 2 years ago

Fact is: clients will want to discard responses when the thing that asked for that response goes away. You are saying that they SHOULD NOT do that. But they are going to anyway. I would rather we just say that if you have push enabled and you do that, you might be relying more heavily on your push ID timeouts to ensure that you properly handle missing push information.

MikeBishop commented 2 years ago

I'm not saying anything about request streams; I agree that clients will want to discard requests they don't need any more and will potentially be discarding PUSH_PROMISE frames along with them. The timeout in that case is fine.

What I'm specifically recommending in the text is that endpoints read far enough into a unidirectional stream (1-9 bytes) to discover the push ID and mark it consumed. If they don't, the server might eventually become unable to push; no timeout will recover from that.