Fwd: Stuck in a train -- reading HTTP/2 draft.

See input from PHK below, forwarded with permission.

I’m going to open an editorial issue so that Martin can take this as input; if he (or anyone else) feels that the points that PHK raises need more discussion, please say so.

Regards,



Begin forwarded message:

> From: "Poul-Henning Kamp" <phk@phk.freebsd.dk>
> Subject: Stuck in a train -- reading HTTP/2 draft.
> Date: 16 June 2014 10:50:31 pm AEST
> To: Mark Nottingham <mnot@mnot.net>

[…]

> Page 12: Reserved bits
> ----------------------
> 
> Historically RFCs have defined reserved bits opposite what would
> have been most useful later on approximately 50% of the time.
> 
> I would split the difference:  A R1 single-bit field, which must
> be sent as zero and ignored on reception, and a R2 single-bit field,
> which must be sent a zero, and where receiving a one is an error.
> 
> Page 22ff: Priority
> -------------------
> 
> I think the PRIORITY stuff is going to be trouble and will need
> revisions later on to truly work.
> 
> It will also be the first thing implementors skip in order to get
> something working fast, and they may never come back and implement
> it, unless serious numbers show it's worth the complexity.
> 
> I would restrict treatment of PRIORITY in this main RFC to a outline
> format of the frame starting with a one byte version number, and
> where the frames can go etc.
> 
> Interpretation of the frames for version #0 I would eject to a
> separate optional RFC, to make the main RFC smaller, simpler and
> thus more appetizing.
> 
> In HEADERS the PRIORITY flag should just enable a 32bit field,
> the intepretation of which goes in the side-RFC.
> 
> 
> Page 28ff: Padding
> ------------------
> 
> If padding is intended for obscuration of traffic patterns, it should
> be available in all frames to avoid "housekeeping" frames from leaking
> information about the relationships of other frames.
> 
> For this reason I would make padding part of the framing format,
> rather than the individual message types.
> 
> I'm also sceptical of demanding that padding be zeros, historically
> that has been a popular recipe for staring through compressions and
> encryptions.  I would probably specify that it is up to the sender
> to decide what to pad with, with guidance to prefer PRN bits rather
> than predictable (ie: zeros) or meaningful data.
> 
> Page 28: Typographical
> ----------------------
> 
> It's a bad idea to end a sentence with "... %d." because of exactly
> what happens in draft 12:  It might bet pushed to the next page
> and confuse everybody.   s/1/one/ ?
> 
> Page 30  HEADERS/SEGMENTS
> -------
> 
> What does END_SEGMENT do on a HEADERS frame ?
> 
> It looks like Copy&Paste error.
> 
> If it's needed, no text explains what it does and it's missing on
> CONTINUATION.
> 
> It's bogus that a lot of text separates HEADERS from CONTINUATION.
> 
> It's also pretty non-obvious why we need CONTINUATION in the
> first place:  Why not simply send HEADERS until one of them has
> the END_HEADERS bit set ?
> 
> 
> Page 34: SETTINGS
> ------------------------
> 
> Are we absolutely certain that we will never need per-stream SETTINGS ?
> 
> At least I would specify SETTINGS with non-zero stream to be ignored
> so that we can open that later.
> 
> Page 34: SETTINGS_MAX_CONCURRENT_STREAMS
> ----------------------------------------
> 
> I would make the this field mandatory in the initial SETTINGS frames
> in order to make sure people think about the DoS potential.
> 
> I also seriously doubt that anybody with a relevant DoS risk is
> going to open with a default value of "infinite" until they have
> seen some kind of "proof of work" on part of the client.
> 
> I suspect such DoS mitigation will cause a lot of SETTINGS frames,
> but that is less prone to failure than trying to define a "slow-start"
> without any benefit of real-life DoS mitigation experience.
> 
> (You should *seriously* consider trying to arrange an INTEROP with
> the specific goal of investigating DoS resistance.)
> 
> Text should be added to explain what happens if N streams are
> open but (x < N) SETTINGS_MAX_CONCURRENT_STREAMS is sent (the
> already open streams survive, new streams must wait for them to
> finish ?)
> 
> What happens of SETTINGS_MAX_CONCURRENT_STREAMS gets set to zero
> and stays there ?  How long time does the client wait for before
> opening a new connection ?
> 
> Are clients allowed to open multiple connections to bypass the
> SETTINGS_MAX_CONCURRENT_STREAMS limit ?
> 
> Are proxies ?
> 
> 
> Page 36: SETTINGS synchronization
> ---------------------------------
> 
> This is utterly bogus.
> 
> There is no way a SETTINGS frame can overtake any other frame while
> in transit, and there is no way the receiver can refuse the SETTINGS.
> 
> There is also none of the settings which require synchronization between
> the ends or which the sender cannot prepare for before sending the
> SETTINGS frame.
> 
> Simply demand that SETTINGS be applied atomically when received, before
> processing any subsequent frames and we're done.
> 
> If the sender *really* wants to know that the SETTINGS has been
> registered in the far end, it can send a PING after the SETTINGS
> and wait for the PING+ACK.
> 
> As it is now, SETTINGS causes the loss of an RTT if implemented as
> specified.
> 
> Page 39: GOAWAY
> ---------------
> 
> I would give the sender the ability to add a small-ish integer
> to the Last-Stream-ID, as a way to avoid wasted work.
> 
> Doing that, I think an opening gambit of
> 
> 	SETTINGS(MAX_CONNCURRENT_STREAMS = n)
> 	GOAWAY(Last-Stream-ID = n)
> 
> could be a very efficient DoS mitigation strategy, which would give
> legitimate users a chance.  In particular if we allow GOWAY to
> update the Last-Stream-ID once the client has proven non-hostile.
> 
> In fact, it's not obvious to me why GOAWAY isn't simply a field
> in SETTINGS to begin with ?
> 
> Page 42:  Flow Control Window
> -----------------------------
> 
> Again:  Why is WINDOW_UPDATE a separate frame type ?  a field
> in SETTINGS would do just fine.
> 
> Doing that would also allow the connection FCW to be updated
> subsequently.
> 
> For obvious DoS reasons, it's a mistake to not apply flowcontrol
> to HEADERS.
> 
> I would allocate frame Type values so that the top bit indicates
> "subject to flow-control" to simplify implementation.  Similarly
> expending a bit to mark hop-to-hop frames would make life easier
> for high performance implementations.
> 
> I also think it is a mistake to exempt the 8 byte frame header from 
> the FCW:  You can fill a lot of pipe with 1 byte DATA frames when
> the overhead isn't counted.
> 
> The explanation about SETTINGS_INITIAL_WINDOW_SIZE causing negative
> window-sizes is somewhat clumsy because some unrealted stuff breaks
> the flow.  Maybe move "A SETTINGS frame cannot alter the connection
> flow control window." and "An endpoint MUST ... FLOW_CONTROL_ERROR"
> out of the way.
> 
> Page 49: BLOCKED
> ----------------
> 
> In general sending data to tell you cannot send data is a non-bright
> idea, that has been discared in almost all protocols for the last
> three decades.
> 
> If the goal is real-time tuning, information about how many bytes
> are stuck in queue should be communicated.
> 
> If the goal is offline statistics, it would be sufficient to report
> at the end of each stream how many times the window closed and
> the max number of bytes we saw queued on a closed window.
> 
> Page 50: Error messages
> -----------------------
> 
> The RFC should propose text-messages for presentation purposes
> bearing in mind that they will be seen and reported by grandmothers
> and brothers in-law.  For a good example how not to do this,
> see the gai_strerror(3) default implemenations.
> 
> Page 51: out of order flags
> ---------------------------
> 
> This is just asking for trouble...
> 
>   The last frame in the sequence bears an END_STREAM flag, though a
>   HEADERS frame bearing the END_STREAM flag can be followed by
>   CONTINUATION frames that carry any remaining portions of the header
>   block.
> 
> The last frame should have the END_STREAM flag, always, there's
> no excuse for "whoops" details like this.
> 
> Page 51: More details in example
> --------------------------------
> 
> The example in 8.1 should show where the END_HEADERS and END_STREAM bits
> are set.
> 
> Page 51:  To strict ordering ?
> ------------------------------
> 
>   Other frames (from any stream) MUST NOT occur between either HEADERS
>   frame and the following CONTINUATION frames (if present), nor between
>   CONTINUATION frames.
> 
> Isn't this needlessly strict ?  No harm would come from DATA frames
> or SETTING frames being stuffed in there.
> 
> All this trouble could be avoided by only submitting headers for
> decompression, as a unit, when the END_HEADERS have been received.
> 
> Page 51: Gibberish
> ------------------
> 
> This needs to be translated to human readable form:
> 
>   Otherwise, frames MAY be interspersed on the stream between these
>   frames, but those frames do not carry HTTP semantics.  In particular,
>   HEADERS frames (and any CONTINUATION frames that follow) other than
>   the first and optional last frames in this sequence do not carry HTTP
>   semantics.
> 
> 
> Page 51: Trailing headers
> -------------------------
> 
> Support for trailing headers should have a bit in SETTINGS.
> 
> How dow we know the DATA section is done if there are trailing headers ?
> 
> By Content-Length ?  (what if it doesn't match ?)
> 
> By appearance of the first (trailer-)HEADER ?
> 
> Trailing headers should be marked in a way that does not force a
> high-perf load-balancer to decode all headers to find out.  Setting
> a flag bit in the first block of HEADERS/CONTINUATION is free for
> the sender (they have to find out at that time anyway) and saves a
> lot of time for the receiver.
> 
> Page 55: reverse order
> ----------------------
> 
> It would make more sense if the ":" header fields were described
> before they appear in the examples on the preceeding pages.
> 
> page 57: query part
> -------------------
> 
> The query part of the URI should get its own ":query" psuedo-field
> for reasons of othogonality and to reduce the amount of data typical
> load-balancers have to examine.
> 
> Page 59: cookie compression needs example
> -----------------------------------------
> 
> Basic confusion-prevention.
> 
> Page 59: Clarification
> ----------------------
> 
> 	[...]the sum of the uncompressed DATA frame payload lengths [...]
> 
> Should probably (better) clarify that 'uncompressed' obny pertains to the
> per frame GZIP compression.
> 
> 
> I havn't looked at the PUSH stuff, I still consider it a fundamentally
> bad idea.
> 
> 
> -- 
> Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
> phk@FreeBSD.ORG         | TCP/IP since RFC 956
> FreeBSD committer       | BSD since 4.3-tahoe    
> Never attribute to malice what can adequately be explained by incompetence.

--
Mark Nottingham   http://www.mnot.net/

Received on Monday, 16 June 2014 21:30:40 UTC