Hypertext Transfer Protocol Version 2 (HTTP/2)
RFC 7540

Note: This ballot was opened for revision 16 and is now closed.

(Alia Atlas) Yes

(Richard Barnes) (was Discuss) Yes

Comment (2015-01-21 for -16)
No email
send info
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."

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

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.)

Section 3.4: "the sever MUST send"

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.

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 "/".

Section 8.1.2.3: "_:authority_"
Remove the underscores?
 
Section 10.3: "if they are translater verbatim"

Alissa Cooper Yes

Comment (2015-01-20 for -16)
No email
send info
Thanks for all of your work on this. I'm not thrilled with the decisions around TLS and opportunistic security, but I understand how we got here.

= Section 3.1 =
It might be worth keeping this sentence from the text that is marked for deletion:

"Examples and text throughout the rest of this document use "h2" as a
   matter of editorial convenience only."

= Section 3.2 =
"A server that supports HTTP/2 can accept the upgrade with a 101
   (Switching Protocols) response."

Is there a reason why this behavior is not normatively described? Is there some other response that clients should expect that has the same semantic?

= Section 5.1 =
"WINDOW_UPDATE or RST_STREAM frames can be received in this state
      for a short period after a DATA or HEADERS frame containing an
      END_STREAM flag is sent. ...
      endpoints MAY choose to treat frames that arrive a significant
      time after sending END_STREAM as a connection error
      (Section 5.4.1) of type PROTOCOL_ERROR."

Can some ballpark guidance be given about how long "a short period" and "a significant time" are expected to be? Or how an implementation might calibrate these values?

s/Frame of unknown types/Frames of unknown types/

= Section 5.2.2 =
"Deployments that do not require this capability can advertise a flow
   control window of the maximum size, incrementing the available space
   when new data is received."

I found this a little vague. Does this mean deployments can advertise a flow control window of size 2^31-1 and then locally increment available memory as new data is received? Would be good to state here both what the maximum size is and what is meant by "available space."

= Section 6.8 =
s/the remote can know/the remote peer can know/

= Section 8.2 =

s/that is not cacheable, unsafe or that/that is not cacheable, safe or that/

= Section 10.7 =
"Intermediaries SHOULD retain padding for DATA frames, but MAY drop
   padding for HEADERS and PUSH_PROMISE frames.  A valid reason for an
   intermediary to change the amount of padding of frames is to improve
   the protections that padding provides."

The different recommendations for different frame types here are a bit puzzling. Why are intermediaries expected to be able to improve padding-based protection for HEADERS and PUSH_PROMISE frames more so than for DATA frames?

= Section 10.8 =

Is there anything more you could say about possible mitigations for the fingerprinting issue? Or there might not be, absent some coordination between endpoints, which would likewise be useful to note.

(Barry Leiba) Yes

(Kathleen Moriarty) (was No Objection) Yes

Comment (2015-01-21 for -16)
No email
send info
Nice work on this draft!  It is very well written and easy to read.  There was also a lot of thought put into tough security questions like compression and the use of cipher suites in working group sessions, which is very much appreciated.  I have a few non-blocking comments that are mostly editorial.

Editorial consideration: Section 9.2.2  

Thanks for adjusting the language in this section and in Appendix A to avoid confusion, making the WG consensus clear that the blacklist usage is a "SHOULD NOT", changing the word prohibited to "blacklist" works for me.

Followup from last IETF meting and my previous comment:
Section 9.2.2 - I'll note first that there is clear WG consensus for using blacklists.  I'm not expecting a change here, but wanted to note that I think you may run into supportability/cost issues in the future with this approach.

In the current text, any cipher suite that's new (not yet blacklisted) can be supported, which includes regional cipher suites.  Since we are in the midst of a push for crypto and seeing the response from those who monitor, including governments, this opens up room for requirements to be set country by country outside of the protocol.

(Martin Stiemerling) Yes

Comment (2015-01-21 for -16)
No email
send info
An excellent document. 

Alissa spotted already what I would have remarked about  the "short period" in Section 5.1.

(Jari Arkko) No Objection

(Benoît Claise) No Objection

Comment (2015-01-22 for -16)
No email
send info
Thank you for a very well-written writeup, which helped the review.

Same remark as Richard regarding figure 1

Spencer Dawkins (was Discuss) No Objection

Comment (2015-01-23 for -16)
No email
send info
Thanks for working through my Discuss and comments! Clearing now ...

(Adrian Farrel) No Objection

(Stephen Farrell) (was Discuss) No Objection

Comment (2015-01-26 for -16)
No email
send info
The first point below was a DISCUSS but is now a comment
as Martin pointed out http/1.1 has the same issue so it's not
new here really.

Just a quick one, I'm not sure if it's me reading it wrong
or a bug... happy to clear and let you fix once we've
figured it out anyway.

8.2.2 says that clients MUST validate that a proxy is
configured for the correspondsing request. Since you say
MUST, don't you need to say what exacftly is meant there?
If that is somewhere here, I'm not seeing it. I realise that
some of that may be controversial, but if so, I think saying
that would be better than leaving it dangling (Note that
I've also a comment below about 10.1 not saying enough, if
the fix for this were to be a new paragraph or two about
when pushed stuff is ok, then you might handle my comment on
10.1 here, and that might be better, not sure.)
(Putting this one first as I'd really like an answer
but will not be blocking on it no matter how much I
dislike the answer:-)

--- OLD COMMENTS below


- 6.1, and elsewhere, I just want to check that the 256
octet limit on padding isn't too restrictive a design. Is it
still possible to say arrange that every response from a
server has the same size? (Say from a server that is only
used for serving images and where there could be a 2KB
variation in file size or something.) I think that can work
via HEADERS and CONTINUATIONs but wanted to check the limits
of flexibility here. I'm similarly interested in the limits
within which a client can add padding to a request, but I
think the same trick works if it works at all.

- 3.1: a question on the text to be removed about
draft-specific identifiers - did the WG consider how to
handle drafts produced from now until the RFC issues?  I've
not yet looked to see if there are normative dependencies
that might lengthen that process, but if there might be it
could be useful to discuss in the WG if it turns out there
are a bunch of discusses that might take a while and a few
versions to sort out. If the IESG eval is clean enough and
there are no delaying normative refs then that's probably
not needed.

- 3.2, the last para before 3.2.1 is hard to read, maybe it
was added late but it seems to use concepts not yet
explained at that point (e.g. half-closed) Maybe a forward
ref to Figure 2 would be enough to fix.

- 5.3.4, "can be moved" in 1st sentence - I don't get why
it's ok to not say MUST or SHOULD there, iff the change is
as a result of a peer's action. Shouldn't a 404 for a
dependecy have a predictable impact on the overall page
load?

- 6.10, odd that there's no padding here

- 8.1, a bit of abnf here might have helped, ah well

- 8.1.2.3, I assume the underscors in "_:authority_" are a
typo, if not they are possibly confusing

- end of p57, does the last para mean that a header field
value can be split betwee a HEADERS frame and the subsequent
CONTINUATION frame (with possible padding in between when
looked at on-the-wire)? If so, then I think you might want
to be clearer about that since I could see it leading to
interop issues.  

- p64, DNS-ID is what (dNSName?)? And "Common Name" might be
better as commonName, but both probably need a reference.
(I don't care about nitty capitalisations, but wouldn't some
coders need to reference?)

- 9.1, which TLS library allows a client to know that it'll
shortly be time to re-key? Doing so is doable, as e.g. NTP
has a mechanism like that for pre-announcing leap seconds
I'm told. Buti I'm not sure anyone actually does it at all
today.

- 9.2.1 - the renegotiation text is confusing. First para on
that starts with "MUST be disable" next one says "MAY use"
for client cert confid. I think adding something like "Other
than as noted below" before the start of  the 1st renego
para (the 3rd para of 9.2.1) might fix this, but could be
some more editing would be better.

- 9.2.2 - Isn't it sad that there are so many undesirable
TLS ciphersuites. Sorry about that;-)

- 10.1 - I think this could be a lot clearer about which
pushed things are to be thrown away and I think being
clearer about that might avoid some future problems. 

- 10.5.1 has a few typos, no harm being nice to the RFC
editor and fixing those if you're pushing out a -17

- 10.7 - what general purpose padding does TLS1.2 provide?
I'm sad that this section is so negative - does that
indicate that people haven't really tried this out so much?
While it is clear there can be failed attempts at using
padding to thwart traffic analysis I think having this
mechanism defined so we can in future learn how to better
mitigate traffic analysis is a good thing and we ought not
be so down on that.

- For the record, I'm also sad that this isn't all and only
over TLS with the option of opportunistic security for http:
schemed URIs but I accept that the wg debated that and
decided for this.

(Brian Haberman) No Objection

(Joel Jaeggli) No Objection

Comment (2015-01-21 for -16)
No email
send info
This has been a long time in coming, thank you.

(Ted Lemon) No Objection

(Pete Resnick) No Objection

Comment (2015-01-21 for -16)
No email
send info
3.2 says:

   A server MUST ignore a "h2" token in an Upgrade header field.
   Presence of a token with "h2" implies HTTP/2 over TLS, which is
   instead negotiated as described in Section 3.3.

And 3.3 says:

   HTTP/2 over TLS uses the "h2" application token.  The "h2c" token
   MUST NOT be sent by a client or selected by a server.

Why isn't the presence of an "h2" Upgrade token on a clear text connection, or an "h2c" application token on a TLS connection, grounds for slamming the connection? Seems like something nefarious might be going on in either case. Seems like "MUST NOT send, MUST be a connection error if received" seems like the right thing to do.

4.2 says:

   An endpoint MUST send a FRAME_SIZE_ERROR error if a frame exceeds the
   size defined in SETTINGS_MAX_FRAME_SIZE, any limit defined for the
   frame type, or it is too small to contain mandatory frame data.

But later 5.1 says:

      Receiving any frames other than [blah blah blah]
      on a stream in this state MUST be treated as a connection error
      (Section 5.4.1) of type PROTOCOL_ERROR.

The MUSTs in there appear contradictory. If I get a frame with the wrong type for my current state that is *also* a bogus size, is there a requirement that I do PROTOCOL_ERROR, or FRAME_SIZE_ERROR, or is the choice of error code not really a requirement at all? I suspect that the "MUST be treated as a connection error" is the key and *not* the particular error code. I would re-word to simply say, "The FRAME_SIZE_ERROR error is sent when a frame exceeds..." in 4.2. In 5.1 and elsewhere, you could say something like:

      Receiving any frames other than [blah blah blah]
      on a stream in this state MUST be treated as a connection error
      (Section 5.4.1). Error type PROTOCOL_ERROR can be used for this
      condition.

I just think we'll regret when the protocol police come around saying, "But it said you MUST use such-and-so error code, and he didn't, so it's fine if my implementation does X", where X is a thoroughly idiotic thing.

4.3:

Earlier in the section, you say:

   A complete header block consists of either:

   o  a single HEADERS or PUSH_PROMISE frame, with the END_HEADERS flag
      set, or

   o  a HEADERS or PUSH_PROMISE frame with the END_HEADERS flag cleared
      and one or more CONTINUATION frames, where the last CONTINUATION
      frame has the END_HEADERS flag set.

So you've defined that the last frame (or the only frame if there's no continuation) has the END_HEADERS set. Cool. But then later you make a point to say:

   The last frame in
   a sequence of HEADERS or CONTINUATION frames MUST have the
   END_HEADERS flag set.  The last frame in a sequence of PUSH_PROMISE
   or CONTINUATION frames MUST have the END_HEADERS flag set.

I don't get the MUSTs. What is the situation that I need to look out for here? Is there a circumstance where an implementation might think it's OK to send the last frame without END_HEADERS set? Seems to me those two sentences can be deleted, but maybe I'm missing something.

Also unnecessarily MUSTy:

OLD
   An
   endpoint receiving HEADERS, PUSH_PROMISE or CONTINUATION frames MUST
   reassemble header blocks and perform decompression even if the frames
   are to be discarded.
NEW
   Hence, an endpoint receiving HEADERS, PUSH_PROMISE or CONTINUATION
   frames needs to reassemble header blocks and perform decompression
   even if the frames are to be discarded.
END

5.1:

   open:
      [...]

      From this state either endpoint can send a frame with an
      END_STREAM flag set, which causes the stream to transition into
      one of the "half closed" states: [...].

      Either endpoint can send a RST_STREAM frame from this state,
      causing it to transition immediately to "closed".

You should probably define the silly state of a RST_STREAM frame with and END_STREAM flag set on it. I presume that you immediately go to "closed", but if you implemented it in a goofy way, you may end up in "half closed".

A clarifying bit:

OLD
   half closed (local):
      A stream that is in the "half closed (local)" state cannot be used
      for sending frames.  Only WINDOW_UPDATE, PRIORITY and RST_STREAM
      frames can be sent in this state.
NEW
   half closed (local):
      A stream that is in the "half closed (local)" state cannot be used
      for sending frames other than WINDOW_UPDATE, PRIORITY and
      RST_STREAM.
END

Also:

      If an endpoint receives additional frames for a stream that is in
      this state, other than WINDOW_UPDATE, PRIORITY or RST_STREAM, it
      MUST respond with a stream error (Section 5.4.2) of type
      STREAM_CLOSED.

I think it would be good to introduce some language somewhere, maybe here (and in other places throughout section 6 referring to stream errors) or maybe in 5.4, that says that you MUST respond with a stream error *unless the frame in question would also constitute a connection error, in which case you MUST respond with a connection error*.

5.4.3: I'm not clear on how to implement a "MUST assume". What is it that the implementation MUST do?

8.2.2: I was actually a little surprised to see the SETTINGS_MAX_CONCURRENT_STREAMS suggestion. I would have thought that using window size would be much more obvious. Any reason you chose to discuss one instead of the other?

11.3: No advice or criteria to use for the expert reviewer?