Re: [Masque] Zaheduzzaman Sarker's No Objection on draft-ietf-masque-connect-udp-14: (with COMMENT)

[David Schinazi <[e-mail]>]

Hi Zahed, thanks for your comments.
Responses inline.
David

On Thu, Jun 16, 2022 at 3:11 AM Zaheduzzaman Sarker via Datatracker <
[e-mail]> wrote:

> - Section 3: s/pct-encoding/percent-encoding and would be good to add a
> reference to pct-encoding (https://datatracker.ietf.org/doc/html/rfc3986)
>

Agreed, fixed via this commit:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/commit/e372382e788bde3c84d903390f8a577f72d8dff6


 - Section 3.1: it took a bit of time for me to understand if we are
> redefining
>  the "intermediaries" here again. We have already described what is an
>  intermediary in the context of this document. Hence, I would suggest that
> we
>  just write - if the recipient is an intermediary, it forwards ....
>

We're not redefining what an intermediary is, we're following the previous
definition.
However, an HTTP server can be both an intermediary and an origin at the
same time,
and handle some requests as an intermediary and some other requests as an
origin.
That's why we specifically phrased it as "act as an intermediary".

 - Section 3.1: what exactly "fail the request" means? is it only a
>  notification to some waiting processes/methods or sending the error
> message or
>  both? can we be more specific here?
>

Agreed. I've replaced the word failed with better terminology from RFC9110
in this commit:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/commit/c5661f0012520aff8d9fb56b7ebaebee3c18f8bf

 - Section 5 : reference to the QUIC DATAGRAM frame would be nicer.
>

Agreed. Added a reference via this commit:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/commit/946a2b65103f8f2132c8c98a0f9f8c86bceeeb1b

 - Section 5 : it says -
>
>                 "Therefore, endpoints MUST NOT send HTTP Datagrams with a
>                 Payload field longer than 65527 using Context ID zero. An
>                 endpoint that receives a DATAGRAM capsule using Context ID
> zero
>                 whose Payload field is longer than 65527 MUST abort the
> stream.
>                 "
>
>         I assume the HTTP Datagram and a DATAGRAM capsule is referring to
> the
>         same thing, but this is confusing. Can use HTTP DATAGRAM in the
> second
>         sentence as well? The DATAGRAM capsule kind of appeared in this
> section
>         all on a sudden.
>
> In this section to me it seems HTTP DATAGRAM and DATAGRAM capsule is
> interchangeably used, if I am correct then I think it is worth mentioning
> this
> for the sake of better understanding.
>

I agree that this was imprecise, I've fixed it in this commit:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/commit/33d4ebc7b8ed1796fa67146b83f70862ac289efc

- Section 5 : it says
>
>         "If a UDP proxy knows it can only send out UDP packets of a certain
>         length due to its underlying link MTU, it SHOULD discard incoming
>         DATAGRAM capsules using Context ID zero whose Payload field is
> longer
>         than that limit without buffering the capsule contents."
>
>      It is not clear why "SHOULD" is used here? what are the other option
> the
>      UDP proxy has in this case?
>

The SHOULD applies to the buffering part of the sentence, I've clarified it
in this commit:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/commit/33d4ebc7b8ed1796fa67146b83f70862ac289efc

- Section 3.6: if this section should be removed then [STRUCT-FIELD] must
> not
> be in the normative section, actually should be removed from the
> references.
> Either at this stage of this document we can remove this section or we can
> add
> a note to remove normative reference when this section is removed.
>

Good point. I've replaced the normative reference with a simple link in
this commit:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/commit/c5ad04c7b98b8bf77c05129124b2e363c6f6c883