-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ben Kakuk's COMMENT on Messaging #915
Comments
s/larger number of multiple connections/larger numbers of connections/ (#915)
tune prose in text that references [HTTP] for status code information (#915)
I'm not a TLS expert. That said, if this is correct, would be lowercasing the two keywords be sufficient? |
|
71e70d2 is for section 9.8 |
|
Change for 9.8 LGTM (as what might pass for a TLS expert). |
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.
Editors: #921
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.
Editors: The HTTP/1.1 message format does not allow adding trailers as a section following the message body. Trailers is allowed in chunked because the content itself is encoded into a format that concludes with an optional trailer section (within the message body). There is no possibility of generalizing this further for HTTP/1.x.
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.
Editors: #926
(I assume 301 rather than 308 was an intentional choice for maximum
compatibility with old/broken clients.)
Editors: This requirement existed long before 308 was minted. In any case, 301 would be preferred since we don't want to assume too much about why a non-GET request method is being used with an invalid request-line (usually meaning that the client failed to encode spaces in the hypertext reference).
Is the contents of the TLS SNI extension sufficient request context to
uniquely identify an intended authority?
Editors: Maybe. Using a certificate that is specific to that authority (one that can't be shared by multiple origins and thus was definitely chosen by the client) would be sufficient. We don't need to define this further.
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.
Editors: #927
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...
Editors: Rewritten by 71e70d2
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.
Editors: None of that reads well (it was imported from RFC2818). This is better rewritten as a factual statement of when it knows, as committed above.
NITS
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.
Editors: #922
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.
Editors: I believe this is more than a nit. Opened #923
"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.
Editors: #928
The text was updated successfully, but these errors were encountered: