Re: [Gen-art] Genart last call review of draft-ietf-masque-connect-udp-12

[Lars Eggert <[e-mail]>]

Christer, thank you for your review. I have entered a No Objection ballot for this document.

Lars

> On 2022-5-28, at 20:53, Christer Holmberg via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Christer Holmberg
> Review result: Almost Ready
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-masque-connect-udp-12
> Reviewer: Christer Holmberg
> Review Date: 2022-05-28
> IETF LC End Date: 2022-06-08
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: The document is well written, and easy to read. However, I do have
> some comments (mainly editorial) that I would like the authors to address.
> 
> Major issues: N/A
> 
> Minor issues: N/A
> 
> Nits/editorial comments:
> 
> 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.
> 
> 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")
> 
> 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.
> 
> 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.
> 
> 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.
> 
> Section 2:
> ----------
> 
> Q2_1:
> 
> I suggest placing the example URIs after the requirements and procedures for
> creating the URL.
> 
> 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".
> 
> 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".
> 
> 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.
> 
> 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".
> 
> 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
> 
> Section 4:
> ----------
> 
> The text says:
> 
>  "This protocol..."
> 
> What protocol? :) As this is the first sentence of the Section, please be
> specific.
> 
> 
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art