W3C home > Mailing lists > Public > ietf-http-wg@w3.org > January to March 2015

Re: Richard Barnes' Discuss on draft-ietf-httpbis-http2-16: (with DISCUSS and COMMENT)

From: Martin Thomson <martin.thomson@gmail.com>
Date: Mon, 26 Jan 2015 15:55:46 -0800
Message-ID: <CABkgnnV2PKM4+9ecFrTMh5va7v0uwE2Dea81USadjUxmmrh-fQ@mail.gmail.com>
To: Richard Barnes <rlb@ipv.sx>
Cc: The IESG <iesg@ietf.org>, Mark Nottingham <mnot@mnot.net>, "httpbis-chairs@tools.ietf.org" <httpbis-chairs@tools.ietf.org>, HTTP Working Group <ietf-http-wg@w3.org>, "draft-ietf-httpbis-http2.all@tools.ietf.org" <draft-ietf-httpbis-http2.all@tools.ietf.org>
Thanks for the review Richard,

https://github.com/http2/http2-spec/pull/709

On 21 January 2015 at 22:14, Richard Barnes <rlb@ipv.sx> wrote:
> (1) Section 3.2: "A server MUST ignore a "h2" token in an Upgrade header
> field."
> What is the reasoning behind this exclusion?  This seems to unnecessarily
> rule out the use of TLS, especially given that the server can opt out by
> choosing "h2c".

I think that Mark explained this adequately.

Your suggestion is one of the different startup variants that was
considered and rejected.  The conclusion being that there needs to be
*exactly one* way start any given transport, regardless of scheme.
(We can discuss why the prior knowledge thing remains, if you like.)

Having the startup path vary based on scheme like that seems to invite
the sort of layering confusion you want to avoid though.

> (2) Figure 1 seems really confusing.  If the reader notices the phrase "9
> octets of the frame header", he'll probably come to the right conclusion,
> but it also seems likely that some readers will infer from the layout
> that the header is 12 octets long, with the fields aligned to word
> boundaries.  Just eliminating the header with the bit positions would
> help a lot.  Likewise for the figures in Section 6.

Fixed throughout.

> (3) Section 9.1.1: "For "http" resources..."
> This seems to imply that requests for "http" resources can only be
> carried over bare TCP, which seems wrong given the presence of the
> ":scheme" pseudo-header.  Proposed text:
[...]
> NEW: "For TCP connections without TLS, this depends on the host having
> resolved to the same IP address."

Sold.  No harm in anticipating future needs.

> (4) Section 9.1.1: "For "https" resources..."
> The salient requirement here is that the certificate provided by the
> server MUST pass any checks that the client would have done if it were
> initiating the connection afresh.  In addition to the name check here,
> that would include things like HPKP.  Suggested text:
>
> OLD: "For "https" resources, connection reuse additionally depends on
> having a certificate that is valid for the host in the URI."
> NEW: "For "https" resources, connection reuse additionally depends on
> having a certificate that is valid for the host in the URI.  The
> certificate presented by the server MUST satisfy any checks that the
> client would perform when forming a new TLS connection for the host in
> the URI (e.g. HPKP checks [HPKP])."

Yes, though I dropped the HPKP reference.  I considered adding a 2818
or 6125 reference, but decided to avoid both and also avoid implying
too narrow a scope for the checks.

> (5) Section 10.4: "Pushed responses for which an origin server is not
> authoritative (see Section 10.1) are never cached or used."
> This seems like a rather important point, for which I can't find any
> normative text.  It seems like in Section 8.2.1, the client should be
> REQUIRED to verify that the ":authority" field in the PUSH_PROMISE
> contains a value for which the client would have been willing to re-use
> the connection (as specified in Section 9.1).

MUST-ied.  As in
- <xref target="authority"/>) are never cached or used.
+ <xref target="authority"/>) MUST NOT be used or cached.

> Section 2.1:
> The definitions of "client" and "server" here are a bit lean.  For
> example, one might read them and conclude that the client and server
> roles are independent of who sends requests and responses.  It would be
> good to clarify these roles, updating the definition in RFC 7230: "An
> HTTP "client" is a program that establishes a connection to a server for
> the purpose of sending one or more HTTP requests.  An HTTP "server" is a
> program that accepts connections in order to service HTTP requests by
> sending HTTP responses."

Check the PR here.

> Section 2.1: "across a virtual channel"
> What is a virtual channel?  Can this phrase just be deleted?

Baleeted.

> Section 3.1: "CREF1: RFC Editor's Note:"
> It seems like it could be useful to leave in a variant of this note,
> describing the variant identifiers used by pre-RFC versions of the
> protocol.  (And thus removing the RFC 2119 language.)

I think that I might just rely on tools.ietf.org to provide that
little bit of history.  If that's OK with you.

I mean, technically, we should be registering all these identifiers.

> Section 4.1: "R: A reserved 1-bit field."
> I was mystified by the purpose of this field until I realized that it's
> only there because stream IDs are 31 bits (to make room for the Exclusive
> flag, I guess).  Might help this read more smoothly to note something to
> that effect here.

The exclusive thing was a relatively recent addition.  An explanation
would be retconning.

The actual and original reason is what I shall term "MSB-phobia": the
fear associated with 32- or 64-bit numbers that use the most
significant bit.  Languages without unsigned variants of the fixed
sized integers, combined with an unwillingness to deal with the
occasional rollover into negative values (or a fear that doing so
might trigger a bug), causes some people to avoid using the bit
entirely.

In any case, we've avoided providing such explanations in the past,
preferring to stick with facts.

> Section 5.1:
> When I initially read Figure 2, I thought that "H/", "ES/", etc.
> designated types of frames (or frames + flags).  If, as they appear, they
> indicate alternatives, it would be clearer to add a space before the
> "/".

Done.

> Section 8.1.2.3: "_:authority_"
> Remove the underscores?

Stephen noticed it too.

Did you know that <spanx> defaults to <spanx style="emph">.  Well, you
do now.  And you will never get those brain cells back either.
Received on Monday, 26 January 2015 23:56:15 UTC

This archive was generated by hypermail 2.4.0 : Friday, 17 January 2020 17:14:42 UTC