Re: AD review of draft-ietf-httpbis-http2bis-05

[Francesca Palombini <[e-mail]>]

Hi Martin! Thanks to you and Cory for considering my comments, I know these were mostly straightforward, and sorry it was tedious, I do hope they bring some additional value, however small, to the document.

I went through the PRs and I answered to some of the points below inline. The rest, as you say, did not need more discussion. I also checked off in https://github.com/httpwg/http2-spec/issues/974 the points which I think are dealt with.

As you can see I left 1. and 7. unchecked just so I remember which comments I have further questions on.

Thanks again, and feel free to merge all of the PRs (except 977 - let's confirm this is what we want first).

Francesca


From: Martin Thomson <[e-mail]>
Date: Friday, 12 November 2021 at 15:02
To: [e-mail] <[e-mail]>
Subject: Re: AD review of draft-ietf-httpbis-http2bis-05
>
>Recall that this is a bis document.  We have a narrow scope for changes and so not everything here is new.
>
>With that in mind, we should try to fix the issues you have identified.  I've opened a few pull requests.  I've trimmed your text where I think that it isn't necessary to discuss anything.  Most are straightforward, so while this was tedious, it's now done.  Thanks for reading through so carefully.
>
>On Thu, Nov 11, 2021, at 08:23, Francesca Palombini wrote:
>> 1. ----- https://github.com/httpwg/http2-spec/pull/977

FP: Thank you. However I just wanted to note here that, although my preference in general would be to take the most updated version of a specification, the TCPbis doc currently sits with a big number of DISCUSS comments since end of September. I think the wg should consider this when deciding which reference to go with. I'd like it to be a wg decision since I am pretty sure the question "should this document reference TCPbis?" will come up in IESG evaluation (hence why I brought it up) and I'd like us to be prepared for it. If the wg agrees to go forward with referencing TCPbis I can coordinate with Martin Duke and authors to see what's the expected timeline.

>> 2. ----- https://github.com/httpwg/http2-spec/pull/978
>> 3. ----- https://github.com/httpwg/http2-spec/pull/979
>> 4. ----- https://github.com/httpwg/http2-spec/pull/980
>I chose to cite the section that defines frames rather than the IANA registry as this document only defines a subset of the frames in the registry.
>

FP: That's fine, thank you.

>> 5. ----- https://github.com/httpwg/http2-spec/pull/981
>You are right to point out that our flow control text is a little soft.  I've tightened it up a little in this PR.
>

FP: Thank you! Looks good.

>> 6. ----- https://github.com/httpwg/http2-spec/pull/982
>As above, I've cited the section, not the registry.
>
>> 7. ----- https://github.com/httpwg/http2-spec/pull/983
>I've added a reference to the priority deprecation text to the first of these newly deprecated fields.

FP: Great, thank you. I had some questions about what part of the handling for priorities is still mandatory to support, following my reading of draft-ietf-httpbis-priority (see point 2 of https://mailarchive.ietf.org/arch/msg/httpbisa/mLM0RujKL6ZXY4eQtbQo2Xomf7c/ ). Maybe you could help clarifying it for me? I read this section as well as 5.3, and although the fields are defined, since they are deprecated there is no processing defined, except for errors. What am I missing?

>
>> 8. ----- Cory got this one.
>> 9. ----- Cory got this one.
>> 10. ----- no action necessary
>>       A value of 0 for SETTINGS_MAX_CONCURRENT_STREAMS SHOULD NOT be
>>       treated as special by endpoints.  A zero value does prevent the
>>
>> FP: When is it ok that the 0 value is treated as special?
>
>A client that receives this from a server - and does not receive another SETTINGS in a reasonable time - cannot make new requests.  It might decide that it cannot continue to use the connection in that case.
>

FP: That makes sense, thanks for clarifying.

>> 11. ----- Cory got this one.
>Note that ignoring a value (if allowed or required) is applying it.
>

FP: Ah, that could be interpreted like that.. it is tricky though to say the client can rely on the values having been applied, and that could mean all its settings have been ignored. Not insisting, just clarifying my interpretation :) If no one else sees this as misinterpretable, I am fine as is (and thanks Cory for polishing the text).

>> 12. ----- Cory got this one.
>> 13. ----- Unless someone else picks this up, I'm going to pass on adding more references to the definition of error codes.

FP: Ok.

>> 14. -----
>>
>>    set after receiving the HEADERS frame that opens a request or after
>>    receiving a final (non-informational) status code MUST treat the
>>
>> FP: Where is a "non-informational status code" defined?
>
>This is a term of art in HTTP and necessary knowledge.  I don't believe that a citation is necessary.
>

FP: What I was looking for was a reference to Section 15.2 of [HTTP], but I am fine either way.

>> 15. ----- https://github.com/httpwg/http2-spec/pull/984
>Yeah, I think we fix simplify by removing this second, contradictory sentence.  Good catch.  (Edits made 5 years apart don't always guarantee consistency.)
>

FP: Glad this comment was useful! :)

>> 16. ----- https://github.com/httpwg/http2-spec/pull/985
>> 17. ----- Cory got this one.
>> 18. ----- https://github.com/httpwg/http2-spec/pull/986
>> 19. ----- Cory got this one.
>> 20. -----
>> FP: Can a reference be added to the section where the TCP FIN bit is defined?
>
>TCP is really terrible about this.  FIN is so fundamental a concept it just ends up spread all over.  Anything I might cite is not useful.  So not really.
>

FP: Ok, that's fine. Just checking :)

>> 21. ----- I don't plan to do anything about this.  These are pictures, not code.

FP: Ok, sure. But we all know how important examples are.

>> 22. ----- Cory got this one.
>> 23. ----- Cory got this one.
>> 24. ----- https://github.com/httpwg/http2-spec/pull/987
>> 25. ---- https://github.com/httpwg/http2-spec/pull/988