Hi Christer,
Al and I had a productive conversation on one of the points that you had brought up,
so I wanted to mention that here, see inline below.
Thanks,
David
On Tue, May 31, 2022 at 3:53 PM David Schinazi <dschinazi.ietf@xxxxxxxxx> wrote:
Hi Christer,Thank you for your detailed review. I've written up the following PR to address your comments:I also added additional comments inline below.Thanks,DavidOn Sat, May 28, 2022 at 10:53 AM Christer Holmberg via Datatracker <noreply@xxxxxxxx> wrote:GENERAL:
--------
QG_1:
The document contains a mix of "SHALL", "MUST", "SHALL NOT" and "MUST NOT", but
I can't see any specifc pattern based on whether "SHALL (NOT)" or "MUST (NOT)"
is used.I tend to use SHALL for active requirements and MUST for reactive requirements.Since RFC 2119 defines those terms as strictly identical, this doesn't harm comprehension.ABSTRACT:
---------
QA_1:
The text says:
"This document describes how to proxy UDP in HTTP,"
To me "proxy UDP in HTTP" sounds a little weird. Isn't "tunneling UDP over
HTTP" more appropriate? That terminology is also used later in the document.
(The same comment applies to "proxy TCP in HTTP")The "proxy" terminology is the simple way of referring to these mechanismswhich is suitable to casual readers. The "tunnel" terminology is more pedanticand best suited for readers expert in HTTP terminology. This is why wedifferentiate using the two by saying "more specifically".Section 1:
----------
Q1_1:
The text says:
"While HTTP provides the CONNECT method (see Section 9.3.6 of [HTTP])
for creating a TCP [TCP] tunnel to a proxy, it lacks a method for
doing so for UDP [UDP] traffic."
I suggest to remove the "it lacks..." part, because the following paragraph
says that the document defines such method.I've added "prior to this specification" to get the best of both worlds.Q1_2:
The text says:
"This protocol supports all versions of HTTP"
I think the text should say which is the latest supported, as there is no
guarantee future versions of HTTP will be supported.Switched to "all existing versions of HTTP".Section 1.1:
------------
Q1_1_1:
The txt says:
"Note that, when the HTTP version in use does not support multiplexing
streams (such as HTTP/1.1), any reference to "stream" in this
document represents the entire connection."
I suggest to replace "Note that," with e.g., "In this document,", as in the
paragraph above.I personally prefer "note that". I'll let the RFC Editor have final say on such details.Section 2:
----------
Q2_1:
I suggest placing the example URIs after the requirements and procedures for
creating the URL.As a first introduction to a concept, I personally find examples easier tounderstand than this very long list of requirements.Q2_2:
The text says:
"If the client detects that any of the requirements above are not met
by a URI Template, the client MUST reject its configuration and fail
the request without sending it to the UDP proxy. While clients
SHOULD validate the requirements above, some clients MAY use a
general-purpose URI Template implementation that lacks this specific
validation."
It sounds strange to say "MUST reject" while saying "SHOULD validate but MAY
use a general-purpose URI Template implementation".
I suggest to say "SHOULD validate the URI Template and reject it if it does not
fulfil the requirements above".The MUST is prefixed by an if clause. Conceptually this means: "if you notice an issueyou have to report it, but you don't have to explicitly check for issues". What you proposewould be "you should check and might report findings" which is not as strong.
We've now changed the text to match both the order you were suggesting and the
normative requirements that I had intended:
Clients SHOULD validate the requirements above; however, clients MAY use a
general-purpose URI Template implementation that lacks this specific validation.
If a client detects that any of the requirements above are not met by a URI
Template, the client MUST reject its configuration and fail the request without
sending it to the UDP proxy.
general-purpose URI Template implementation that lacks this specific validation.
If a client detects that any of the requirements above are not met by a URI
Template, the client MUST reject its configuration and fail the request without
sending it to the UDP proxy.
Section 3:
----------
Q3_1:
(This comment might apply to other parts of the document too)
The text says:
"To initiate a UDP tunnel associated with a single HTTP stream,
clients issue a request containing the "connect-udp" upgrade token."
I suggest to refer to a single entity for the procedure, i.e., "client" instead
of "clients".Agreed. I've changed this and some other parts of the draft.Section 3.1:
------------
Q3_1_1:
The text says:
"Upon receiving a UDP proxying request:
* if the recipient is configured to use another HTTP proxy, it will
act as an intermediary: it forwards the request to another HTTP
server. Note that such intermediaries may need to reencode the
request if they forward it using a version of HTTP that is
different from the one used to receive it, as the request encoding
differs by version (see below).
* otherwise, the recipient will act as a UDP proxy: it extracts the
"target_host" and "target_port" variables from the URI it has
reconstructed from the request headers, and establishes a tunnel
by directly opening a UDP socket to the requested target."
It is unclear whether the rest of the Section text applies to both of these
bullets, or only one of them.The rest of the section specifically mentions the "UDP proxy" and that onlyapplies to the second bullet. But perhaps we can do better, would you havea textual suggestion here?Also, the "(see below)" part in the first bullet refers to the following
Sections, but it is unclear. If refering to another section, I suggest using
explicit Section references instead of "below".I tried spelling out the four related subsections and it became unwieldyand less clear than "below".Sections 3.2, 3.3, 3.4 and 3.5:
-------------------------------
I suggest to use sub-sections, with e.g., the following structure:
3.2. HTTP/1.1 Usage
3.2.1. Request
3.2.2. Response
3.3. HTTP/2 and HTTP/3 Usage
3.3.1. Request
3.3.2. ResponseI personally prefer the current structure.Section 4:
----------
The text says:
"This protocol..."
What protocol? :) As this is the first sentence of the Section, please be
specific.Agreed. Replaced "this protocol" with "The mechanism for proxying UDP in HTTP defined in this document".
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
