Subject: review: http2-10

From: Michael Köller <[email-masked]>
Date: Thu, 3 Apr 2014 21:36:12 +0200

Hi,

Julian asked me to do a review on the current draft of the HTTP/2 . 
My apologies, if my comments may have been discussed on the mailing list already. I tried to search the mailing list for answers, deleted some of the remarks I had noted down but still may have missed some threads.

4.3),  paragraph 7
> Header blocks MUST be transmitted as a contiguous sequence of frames, with no interleaved frames of any other type or from any other stream.
Me not being very familiar with the current state of http2, I was puzzled by the question: Why no interleaved frames?
I read mostly to the end and ahead into the referenced specs to deduce the reason: Header compression! 
I would like to see an informative sentence spent here about the stateful nature of header compression.

6.4) RST_STREAM
Is there a reason that this frame type does not carry an optional debug data field, like GOAWAY does? I can image same situations and use cases on both.
Speaking of debug data: Maybe the spec should advise about a reasonable default data format? Like a default encoding for text based data?

6.8), paragraph 4
> After sending a GOAWAY frame, the sender can discard frames for new streams. However, any frames that alter connection state cannot be completely ignored. For instance, HEADERS, PUSH_PROMISE and CONTINUATION frames MUST be minimally processed to ensure a consistent compression state (see Section 4.3)
The term "compression state" is undefined at this point. It results implicitly from the referenced header compression spec. That's another indicator that section 4.3 should tell something about the stateful nature of header compression.

Best regards,
Michael Köller

---

From: Martin Thomson <[email-masked]>
Date: Thu, 3 Apr 2014 13:21:12 -0700

Thanks for calling out the shortcoming with respect to compression
statefulness.  I hope that this
(https://github.com/http2/http2-spec/commit/9939558e7c12bba8dc383ec294b059c98b8f8738)
helps clarify.

On 3 April 2014 12:36, Michael Köller <[email-masked]> wrote:
> Is there a reason that this frame type does not carry an optional debug data
> field, like GOAWAY does? I can image same situations and use cases on both.
> Speaking of debug data: Maybe the spec should advise about a reasonable
> default data format? Like a default encoding for text based data?

We discussed this possibility and decided not to add the debugging
data as a result of a combination of factors:

* Anything that requires debugging is probably worth elevating to
GOAWAY.  Any time RST_STREAM is used, the sender can send GOAWAY
instead.

 (Note that the working group has explicitly decided to avoid being
Postel-principle-compliant with this protocol: being lenient on errors
leads to people taking liberties, which leads to a giant mess.)

* There were DoS-related concerned with having unbounded data in
RST_STREAM, whereas the opportunity for exploitation on GOAWAY is
less.

* We only barely accepted the proposal in the first place.

---

From: Michael Köller <[email-masked]>
Date: Fri, 4 Apr 2014 06:17:19 +0200

> (https://github.com/http2/http2-spec/commit/9939558e7c12bba8dc383ec294b059c98b8f8738)

Looks good.

Regards,
Michael