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.i...@gmail.com> wrote: > Hi Christer, > > Thank you for your detailed review. I've written up the following PR to > address your comments: > https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/pull/173 > I also added additional comments inline below. > > Thanks, > David > > > On Sat, May 28, 2022 at 10:53 AM Christer Holmberg via Datatracker < > nore...@ietf.org> 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 mechanisms > which is suitable to casual readers. The "tunnel" terminology is more > pedantic > and best suited for readers expert in HTTP terminology. This is why we > differentiate 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 to > understand 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 issue > you have to report it, but you don't have to explicitly check for issues". > What you propose > would 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. 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 only > applies to the second bullet. But perhaps we can do better, would you have > a 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 unwieldy > and 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. Response >> > > I 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". >
_______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
