Subject: [quicwg/base-drafts] CANCEL_PUSH frame payload ambiguity (#948)

From: Lucas Pardue <[email-masked]>
Date: Tue, 21 Nov 2017 11:32:52 +0000 (UTC)

HTTP/QUIC says:

> The CANCEL_PUSH frame carries a Push ID encoded as a variable-length integer. The Push ID identifies the server push that is being cancelled (see Section 5.2.6).
_..snip.._
A server MUST treat a CANCEL_PUSH frame payload does not contain exactly one variable-length integer as a connection error of type HTTP_MALFORMED_CANCEL_PUSH.

The first sentence implies that a single Push ID is carried in the payload. The last sentence (filling in the missing-word typo) could imply that multiple Push IDs can be carried in the payload but that servers must reject such frames.

It might help to be more explicit and include a figure for the payload. This provides consistency with the PRIORITY and PUSH_PROMISE sections which also refer to the Push ID. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/948

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Nov 2017 21:49:11 +0000 (UTC)

The last sentence indicates that the length of the frame could be mismatched (too long or too short) with there being exactly one variable-length integer, and if that happens then all bets are off on the interpretation of the frame.  I'm not sure how that would be more explicit, but feel free to propose text.

We generally don't put figures for frames that contain a single field, though some of the PRs have added that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/948#issuecomment-346172370

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 22 Nov 2017 12:12:15 +0000 (UTC)

> The last sentence indicates that the length of the frame could be mismatched (too long or too short) with there being exactly one variable-length integer, and if that happens then all bets are off…

I see where you are coming from. I'd be happy to propose a PR(s) but don't want to go off in a wrong direction. Please let me know what you think after reading the following.

> A server MUST treat a CANCEL_PUSH frame payload does not contain exactly one variable-length integer as a connection error of type HTTP_MALFORMED_CANCEL_PUSH.

Is this specific to the server, or applicable to either endpoint that receives a frame mismatch? Unless I misunderstand something special with varints (elsewhere the spec mentions truncation), the condition seems overly specific. I.e. The format is defined as exactly one varint, so any deviation from that is malformed and leads to an error with a suitable code.

I noticed that HTTP_MALFORMED_CANCEL_PUSH frame error is undefined. Probably just slipped through the net. Looking a bit deeper, I see that the HTTP/QUIC pattern is to define a named error code for each frame type, in contrast QUIC Transport defines a generic frame error code (FRAME_FORMAT_ERROR 0x7) and then a specific frame error codespace (FRAME_ERROR 0x1XX, where the last octet expresses the frame type). I wonder if such an approach might also be suitable for HTTP/QUIC e.g. HTTP_FRAME_ERROR.

I have no problem not diagramming single field frames. I admit to not noticing that convention until you pointed it out but it makes sense.

I think what confused me slightly is the general document layout regarding server push and push ID. The first instances of the Push ID term are in Section 4.3 and 4.4 which reference 5.2.6. Unfortunately, 5.2.6 entangles the general description of what is Push ID, with how it's used in the PUSH_PROMISE frame. Section 4.4 includes guidance on using Push ID without defining what it is.

So I wonder if it might help to extract out a common definition of Push ID (varint, used in X,Y,Z frames etc) into section 4.4. Then specific sections can specialise their usage of Push ID as required.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/948#issuecomment-346332867