HTTP/1.1
draft-ietf-httpbis-messaging-19

Thanks for this revision to a crucial standards.  Nits only:

** Section 6.1.  Editorial.  s/ A sender MUST NOT apply chunked more than once/ A sender MUST NOT apply chunked encoding more than once/

** Section 11.1. Hopefully s/sprintf/snprintf/

Thanks to the editors and contributors of this document for a great job. Specially for the security consideration section, it is very well written and anyone implementing this document should pay extra attentions to that section.

I have following comment and question -

*  I consider this as editorial fix hence not holding a discuss but I would like to see this addressed. This document uses terminologies defined in section 3 of https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-semantics-16#section-3, for example - user agent, client. However, it does not refer to the the semantics doc. I think it must refer to the section 3 of semantic document.

* Section 2.2 :  it says -

          "When a server listening only for HTTP request messages, or processing
   what appears from the start-line to be an HTTP request message,
   receives a sequence of octets that does not match the HTTP-message
   grammar aside from the robustness exceptions listed above, the server
   SHOULD respond with a 400 (Bad Request) response and close the
   connection."
         Is there a reason why it is not a MUST but SHOULD? In my small scale implementation experiments I implemented it as a MUST. I believe if a 400 is send followed by a close connection then it is a "save yourself" action for the server and a MUST thing to do.  

* from the ID nits : there is an unused reference to RFC7231.

Thanks for addressing my concern about absolute-form target URIs at
origin servers (by confirming in -semantics that scheme-specific
requirements must be met for properly directed requests)!

I read over the diff from -16 to -17 and had only one new (nit-level)
comment:

Section 9.4

   Furthermore, using multiple connections can cause undesirable side
   effects in congested networks.  Using larger number of multiple
   connections can also cause side effects in otherwise uncongested
   networks, because their aggregate and initially synchronized sending
   behavior can cause congestion that would not have been present if
   fewer parallel connections had been used.

nit: "Using larger number of multiple connections" doesn't seem right,
and possibly in more ways than just the singular/plural mismatch.

However, I'll also retain my previous ballot comments, as on further
inspection I'm not sure to what extent they were already covered in
github.

=====BEGIN PREVIOUS BALLOT COMMENT=====
Thanks for another extremely well written document!

The only general comment I have is that [Semantics] did such a good job
of portraying the trailer section as a generic concept that I was
surprised to see it presented as specific to the chunked
transfer-encoding in this document.  It seems to me (naively, of
course), that when the content can accurately be delimited, whether by
Content-Length or the chunked transfer-encoding, a trailer section could
be read after the request or response and clearly distinguished from the
start of a new request or response.  I recognize that we have a
significant deployed base to be mindful of backwards compatibility with,
and so do not propose to recklessly add trailer sections everywhere.  It
might be worth some more prominent acknowledgment that in HTTP/1.1 the
trailers section is limited to the chunked transfer-encoding, and
discussion of why trailers are not usable in other HTTP/1.1 scenarios,
though.

I did make a github PR with a handful of editorial suggestions, at
https://github.com/httpwg/http-core/pull/872 .

2.2

   The presence of such whitespace in a request might be an attempt to
   trick a server into ignoring that field line or processing the line
   after it as a new request, either of which might result in a security
   vulnerability if other implementations within the request chain
   interpret the same message differently.  [...]

Given the previous procedure that gives as a permitted behavior to
"consume the line without further processing", it seems like an attempt
to get the server to ignore the field line would have succeeded if this
procedure is followed?  I suppose the important difference is that the
field line is completely suppressed from any version of the message
transmitted downstream, thus avoiding the opportunity for a different
interpretation.  Regardless, though, it seems like the text of the guidance
as written (not quoted above) reads like it is setting us up for
vulnerabilities in the presence of non-compliant (or HTTP/1.0?)
implementations in the request chain.  We might want to put in a bit
more explanation of how the stated procedure avoids the vulnerability.

Section 3.2

   Recipients of an invalid request-line SHOULD respond with either a
   400 (Bad Request) error or a 301 (Moved Permanently) redirect with
   the request-target properly encoded.  [...]

(I assume 301 rather than 308 was an intentional choice for maximum
compatibility with old/broken clients.)

Section 3.3

   Supplying a default name for authority within the context of a
   secured connection is inherently unsafe if there is any chance that
   the user agent's intended authority might differ from the selected
   default.  A server that can uniquely identify an authority from the
   request context MAY use that identity as a default without this risk.

Is the contents of the TLS SNI extension sufficient request context to
uniquely identify an intended authority?

Section 5.1

                                                    The field line value
   does not include any leading or trailing whitespace: OWS occurring
   before the first non-whitespace octet of the field line value or
   after the last non-whitespace octet of the field line value ought to
   be excluded by parsers when extracting the field line value from a
   field line.

I have in general tried to refrain from commenting on the extensive use
of the phrase "ought to" in this group of documents, but this particular
scenario seems like a strong candidate for a BCP 14 keyword.

Section 9.8


   TLS provides a facility for secure connection closure.  When a valid
   closure alert is received, an implementation can be assured that no
   further data will be received on that connection.  TLS
   implementations MUST initiate an exchange of closure alerts before
   closing a connection.  A TLS implementation MAY, after sending a
   closure alert, close the connection without waiting for the peer to
   send its closure alert, generating an "incomplete close".  [...]

This is written as if it's imposing normative requirements on generic
TLS implementations (not placing restrictions on what TLS
implementations are suitable for HTTPS).  Fortunately, these "MUST
initiate" and "MAY close without waiting" requirements seem to already
be present in RFC 8446...

                                                              This
   SHOULD only be done when the application knows (typically through
   detecting HTTP message boundaries) that it has sent or received all
   the message data that it cares about.

...whereas this SHOULD does not have an obvious analogue in RFC 8446,
and thus it would make sense to retain the BCP 14 keyword for.

NITS

Section 4

                           The rest of the response message is to be
   interpreted in light of the semantics defined for that status code.
   See Section 15 of [Semantics] for information about the semantics of
   status codes, including the classes of status code (indicated by the
   first digit), the status codes defined by this specification,

In some sense it seems that the referenced status codes are defined by
[Semantics], not "this specification".  I was initially going to propose
(in my PR) a change to "defined for", but that seems incorrect and I
don't have a better proposal handy.

Section 5.2

   A sender MUST NOT generate a message that includes line folding
   (i.e., that has any field line value that contains a match to the
   obs-fold rule) unless [...]

Since we don't include the obs-fold production as a component of any
other production, and field-value excludes CRLF, it seems that any such
field line value would already be in violation of the ABNF and thus
forbidden.  I don't really want to advocate for including obs-fold in
the field-value production in -semantics, though, so maybe accepting
this nit is the least bad choice here.

Section 9.2

   A client that has more than one outstanding request on a connection
   MUST maintain a list of outstanding requests in the order sent and
   MUST associate each received response message on that connection to
   the highest ordered request that has not yet received a final (non-
   1xx) response.

"Highest ordered" implies some numerical rank-list of ordering, but we
don't seem to clearly indicate whether older or newer requests receive
higher numerical indices.  It seems simples to just say "oldest" (or
"newest", if that was the intent) rather than applying numerical
ranking.
=====END PREVIOUS BALLOT COMMENT=====

Section 9.1. , paragraph 2, comment:
>    protocols.  Each connection applies to only one transport link.

I'm not sure I understand what is meant by "transport link" and how connections
would "apply" to one.

Section 9.4. , paragraph 4, comment:
>    consumes server resources.  Furthermore, using multiple connections
>    can cause undesirable side effects in congested networks.

Using larger number of multiple connections can even cause side effects in
otherwise uncongested networks, because their aggregate and initially
synchronized sending behavior can *cause* congestion that would not have been
present if fewer parallel connections had been used.

Section 13.1. , paragraph 14, comment:
>    [Welch]    Welch, T. A., "A Technique for High-Performance Data
>               Compression", IEEE Computer 17(6), June 1984.

This can become an informative reference, so to not create a DOWNREF, since it's
only used in the description of an IANA codepoint.

Section 13.2. , paragraph 12, comment:
>    [RFC7230]  Fielding, R., Ed. and J. F. Reschke, Ed., "Hypertext
>               Transfer Protocol (HTTP/1.1): Message Syntax and Routing",
>               RFC 7230, DOI 10.17487/RFC7230, June 2014,
>               <https://www.rfc-editor.org/info/rfc7230>.

I think it is common practice to normatively cite an RFC that is being
obsoleted.

-------------------------------------------------------------------------------
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 9.3. , paragraph 3, nit:
-    Connection header field (Section 7.6.1 of [Semantics]), if any:
+    based on the protocol version and Connection header field in the

Section 11.2. , paragraph 2, nit:
-    Request smuggling ([Linhart]) is a technique that exploits
-                      -         -
+    Request smuggling [Linhart] is a technique that exploits

Section 12.3. , paragraph 3, nit:
-    |            | ([RFC1951]) inside the "zlib" | 7.2       |
-                   -         -
-    |            | data format ([RFC1950])       |           |
-                               -         ^
+    |            | [RFC1951] inside the "zlib"   | 7.2       |
+                                               ++
+    |            | data format [RFC1950]         |           |
+                                        ^^

Section 7.1.1. , paragraph 6, nit:
> to accept in the response, and whether or not the client is willing to prese
>                                ^^^^^^^^^^^^^^
Consider shortening this phrase to just "whether". It is correct though if you
mean "regardless of whether".

Section 9.5. , paragraph 2, nit:
> tack has received the client's acknowledgement of the packet(s) containing th
>                                ^^^^^^^^^^^^^^^
Do not mix variants of the same word ("acknowledgement" and "acknowledgment")
within a single text.

Section 9.8. , paragraph 2, nit:
> onse Splitting Response splitting (a.k.a, CRLF injection) is a common techni
>                                    ^^^^^
The abbreviation is missing a period after the last letter.

Section 10.2. , paragraph 17, nit:
>  such that it can be used over many different forms of encrypted connection,
>                                ^^^^^^^^^^^^^^
Consider using "many".

"Appendix B. ", paragraph 2, nit:
>  (which indicate that the client ought stop sending the header field), and t
>                                  ^^^^^^^^^^
Did you mean "ought to stop"?

"B.3. ", paragraph 2, nit:
> ferring to RFC 723x. * Remove acknowledgements specific to RFC 723x. * Move
>                               ^^^^^^^^^^^^^^^^
Do not mix variants of the same word ("acknowledgement" and "acknowledgment")
within a single text.

"B.4. ", paragraph 1, nit:
> pecific to RFC 723x. * Move "Acknowledgements" to the very end and make them
>                              ^^^^^^^^^^^^^^^^
Do not mix variants of the same word ("acknowledgement" and "acknowledgment")
within a single text.

"C.2.2. ", paragraph 4, nit:
> ction 12.4, update the APLN protocol id for HTTP/1.1 (<https://github.com/ht
>                                      ^^
This abbreviation for "identification" is spelled all-uppercase.

Uncited references: [RFC7231].

Obsolete reference to RFC2068, obsoleted by RFC2616 (this may be on purpose).

These URLs point to tools.ietf.org, which is being deprecated:
 * https://tools.ietf.org/html/draft-ietf-httpbis-semantics-16
 * https://tools.ietf.org/html/draft-ietf-httpbis-cache-16

These URLs in the document can probably be converted to HTTPS:
 * http://packetstormsecurity.com/papers/general/whitepaper_httpresponse.pdf

(3.3)  "Alternatively, it might be better to redirect the request to a safe resource that explains how to obtain a new client."

Did you mean "obtain a new user agent?"

(7.1) "recipients MUST anticipate potentially large decimal numerals"

s/decimal/hexidecimal (?) The chunk-size is in hex.

(10) This section could use an introductory paragraph. It is not at all clear from the text why someone would enclose message as data or in what context this would occur.

Thank you for cleaning up this spec.  I appreciate that the effort it takes to achieve this, but it is very helpful to the wider community in the long term.

Not surprisingly, I only have minor comments.

   Although HTTP makes use of some protocol elements similar to the
   Multipurpose Internet Mail Extensions (MIME) [RFC2045], see
   Appendix B for the differences between HTTP and MIME messages.

Nit: This doesn't parse easily to me, perhaps drop the Although?

  Various ad hoc limitations on request-line length are found in
  practice.  It is RECOMMENDED that all HTTP senders and recipients
  support, at a minimum, request-line lengths of 8000 octets.

I was wondering how this applies to constrained devices, but I assume that very constrained devices should probably be using something like CoAP, and otherwise most device should be able to cope with an 8k buffer.

Nit:
Various fields seem have a arbitrary amount of space before the equals: E.g., "method         = token".  I presume that this whitespace is not meaningful, but wondered if the spec would be clearer if it wasn't there.

   A sender MUST NOT
   apply chunked more than once to a message body

Nit: "apply chunked" doesn't read that well to me ... but I understand what it means.  If you decide to change this, then I would check the other places where "chunked" is used.

   
   A client MUST NOT process, cache, or forward such extra data as a
   separate response, since such behavior would be vulnerable to cache
   poisoning.

The intent in this paragraph seemed somewhat ambiguous.  E.g., is the requirement that the client must not process/cache the extra data, or that it must not process/cache the extra data as a separate response?  I assume the latter.

   All transfer-coding names are case-insensitive and ought to be
   registered within the HTTP Transfer Coding registry

Would a 2119 SHOULD be better than ought?

   A client MUST
   NOT send a request containing Transfer-Encoding unless it knows the
   server will handle HTTP/1.1 requests (or later minor revisions);

   and also

  A client MUST NOT use the chunked transfer encoding
   unless it knows the server will handle HTTP/1.1 (or later) requests;

Given that we are in 2021, and the HTTP/1.1 spec was published a couple of decades ago, I was wondering whether the MUST NOT is a bit strong?

Thanks,
Rob