- From: Lucas Pardue <lucaspardue.24.7@gmail.com>
- Date: Mon, 15 Nov 2021 23:21:45 +0000
- To: Francesca Palombini <francesca.palombini@ericsson.com>
- Cc: "draft-ietf-httpbis-priority@ietf.org" <draft-ietf-httpbis-priority@ietf.org>, "ietf-http-wg@w3.org" <ietf-http-wg@w3.org>, "draft-ietf-httpbis-http2bis@ietf.org" <draft-ietf-httpbis-http2bis@ietf.org>
- Message-ID: <CALGR9obJkdF15bY8ZueOMncQ=NwidanwqOj-pOcD4tAy7jHFfw@mail.gmail.com>
Hi Francesca, Thank you for the review! On Fri, Nov 12, 2021 at 11:34 PM Francesca Palombini < francesca.palombini@ericsson.com> wrote: > Thank you for this document. Here is my review: please handle these > comments together with any future Last Call ones. I have opened an issue > with the text below in the github: > https://github.com/httpwg/http-extensions/issues/1783 . The first one > should be fixed (and is easy to fix), the rest are more for my > understanding – and apologies in advance if I have missed previous > decisions. > > > > Francesca > > > > 1. ----- > > > > FP: For some reason the IDnit check didn't pick it up, but the boilerplate > about BCP 14 (first paragraph of Section 1.1) is outdated and should be > updated to: > > > > The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", > > "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and > > "OPTIONAL" in this document are to be interpreted as described in > > BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all > > capitals, as shown here. > Thanks. See https://github.com/httpwg/http-extensions/pull/1784 > > 2. ----- > > > > However, in order to maintain wire compatibility, HTTP/2 priority > > signals are still mandatory to handle (see Section 5.3.2 of [HTTP2]). > > > > FP: I have re-read this Section of HTTP2 several times, but I still can't > be sure that "mandatory to handle" is accurate. In particular, paragraph 1 > states that "some of the mandatory handling is retained" (side note: that > leaves me confused to which parts are retained - maybe one of the HTTP2 > authors in CC can help me here): > > > > This update to HTTP/2 deprecates the priority signaling defined in > > RFC 7540 [RFC7540]. The bulk of the text related to priority signals > > is not included in this document. The description of frame fields > > and some of the mandatory handling is retained to ensure that > > implementations of this document remain interoperable with > > implementations that use the priority signaling described in RFC > > 7540. > > > > And the last paragraph states: > > > > priority of requests in the absence of any priority signals. Servers > > MAY interpret the complete absence of signals as an indication that > > the client has not implemented the feature. The defaults described > > > > Which to me implies that clients are not required to implement them. But > maybe this is me misinterpreting "handling" as processing from either > clients or servers, while maybe it is intended to be mandatory to be > supported from servers only? > > > > Just to clarify, this might not be an actionable comment on this document, > but just want to make sure that [HTTP2] and this document are consistent. > My understanding is that RFC7540bis is trying to say that we can't remove the priority pieces of HTTP/2 HEADERS frames, or the entire PRIORITY frame. Implementations of HTTP/2 will be expected to parse those elements of the protocol and ensure correctness but ignoring the information in valid frames is allowed. My working knowledge has always been that clients don't process priority signals but now I take a look at RFC7540(bis) and can't seem to find a requirement for that. The Priorities draft is not trying to change HTTP/2, we can probably wordsmith things to make that more clear. I'm happy to take a cue from RFC7540bis authors if they have a suggestion. > > > 3. ----- > > > > SETTINGS_NO_RFC7540_PRIORITIES parameter with value of 1, it SHOULD > > stop sending the HTTP/2 priority signals. If the value was 0 or if > > the settings parameter was absent, it SHOULD stop sending > > PRIORITY_UPDATE frames (Section 7.1), but MAY continue sending the > > > > FP: For both SHOULDs above: it might be good to give a hint of when it is > acceptable to deviate from the RECOMMENDations here. Otherwise, since there > are a number of SHOULD throughout the document (I noted those in Section > 4.1 as well), it might be good to address them in one sentence explaining > that those recommendations have exceptions. (I am pointing this out as I > expect it will otherwise come up later in IESG evaluation) > There's not really strong proactive reasons for deviating from this behaviour, our SHOULDs reflect that implementations might have their own reasons or constraints that prevent the guidance from being MUST level. I'll see if there is any text we can do to better elucidate the reason for these SHOULDs, rather than reasons to deviate. > > > 4. ----- > > > > Section 16. IANA Considerations > > > > FP: Is there a reason not to request the first available values in the > IANA registries: 0x7 for SETTINGS_NO_RFC7540_PRIORITIES and 0xb for > PRIORITY_UPDATE instead of 0x9 and 0x10 ? > I can't recall there being initial strong reasons for the chosen HTTP/2 values but changing them now would affect any implementations. I think at this stage the values we have picked are likely already burned. Cheers, Lucas
Received on Monday, 15 November 2021 23:22:10 UTC