Skip to main content

JavaScript Session Establishment Protocol (JSEP)
draft-uberti-rtcweb-rfc8829bis-05

Note: This ballot was opened for revision 03 and is now closed.

Murray Kucherawy
Yes
Erik Kline
No Objection
Comment (2022-10-02 for -03) Sent
# Internet AD comments for {draft-uberti-rtcweb-rfc8829bis-03}
CC @ekline

## Comments

### general

* Is there a reason this didn't get renamed with -ietf-?
  I see a working group adoption event in the history.
  Not important, just curious.

### S5.2.1, S5.3.1

* RFC 8840 Section 4.1.1 says that either 0.0.0.0 or :: may be used.  This
  section says that "c=" MUST use 0.0.0.0.  Can :: be used instead?

* Ditto for the Initial Answers discussion (and 8440 S4.1.3), pp. 53, 55.
John Scudder
No Objection
Comment (2022-09-28 for -03) Not sent
I really appreciate the clean diff vs 8829. Thanks for an easy review.
Paul Wouters
No Objection
Comment (2022-10-05 for -03) Sent
Some minor comments that are likely my misunderstanding of the protocol:

Section 3.5.2.1:

        Implementations MUST be prepared to receive objects with some fields
        missing, as mentioned above.

What does it mean to be "prepared"? Is there any guidance to give here ?

Section 5.2.1:

        The <sess-id> MUST be representable by a 64-bit signed integer,
        and the value MUST be less than 2^63-1.

Why is this a _signed_ integer and not an unsigned integer? What is the meaning of
2^63-1 in a 64 bit signed integer ?


        [...] SHOULD NOT be included.

        [...] MUST NOT be included.

Should these be extended to say "MUST be ignored when received" ? Or is there specific action that should take place that can be specified here?

NITS:

        opaque blobs; that is, the application will not need to read or
        change them.

maybe:

        opaque blobs; that is, the application will not need to parse or
        modify these.

("read" is a little ambiguous, it might need to memcpy (read and write) it,
but not grok it)


we could easily enable -> it could easily enable
Roman Danyliw
No Objection
Comment (2022-09-29 for -03) Not sent
Thank you to Yaron Sheffer for the SECDIR review.
Warren Kumari
No Objection
Comment (2022-10-05 for -03) Sent
As with many of the other ballots:
1: I don't really understand this protocol but
2: I do like the 'diff's :-)

I'd also like to thank Qin Wu for the [OpsDir review](https://datatracker.ietf.org/doc/review-uberti-rtcweb-rfc8829bis-02-opsdir-lc-wu-2022-03-27/), and also the authors for addressing them (https://github.com/rtcweb-wg/jsep/pull/1025) - from what I can tell (see #1 :-)), this improved the document.
Zaheduzzaman Sarker
No Objection
Comment (2022-10-05 for -03) Sent
Thanks for the updated specification and finding a way solve the issue with minimal changes.

One comment -

  - if not MUST, I was at least expecting a normative SHOULD to use the must-bundle policy instead of max-bundle in the following text-
    
     "As a result, "max-bundle" is considered deprecated, and new applications should use the "must-bundle" policy instead."
Éric Vyncke
No Objection
Comment (2022-10-03 for -03) Sent
# Éric Vyncke, INT AD, comments for draft-uberti-rtcweb-rfc8829bis-03
CC @evyncke

Thank you for the work put into this document. 

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education).

Like John Scudder, with such an easy diff vs. RFC 8829, I have only reviewed the diffs.

Special thanks to Sean Turner for the shepherd's detailed write-up including the WG consensus and the justification of the intended status, the unusual format is sensible for such a minimum -bis.

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS

### I-D name

Like Erik Kline @ekline, I really wonder (and regret) why the I-D name was not updated to become draft-ietf-*, the shepherd explanation does not really satisfy me as at least two ADs are commenting on this I-D name and if is generating more work for the IESG review (checking mailing list & status). Not a big deal but a real annoyance.

### Section 4.1.1

The change from "max-bundle" to "must-bundle" is unclear to me (but I am not an expert in this protocol) and, with an apparently significant change, should there be normative language to ensure transition from 8829 to the -bis ?

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
Alvaro Retana Former IESG member
No Objection
No Objection (for -03) Not sent

                            
Lars Eggert Former IESG member
No Objection
No Objection (2022-09-30 for -03) Sent
# GEN AD review of draft-uberti-rtcweb-rfc8829bis-03

CC @larseggert

Thanks to Joel Halpern for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/i_745aF4T6deyFdN6ocUBuddfg8).

## Comments

### Inclusive language

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term `traditional`; alternatives might be `classic`, `classical`, `common`,
   `conventional`, `customary`, `fixed`, `habitual`, `historic`,
   `long-established`, `popular`, `prescribed`, `regular`, `rooted`,
   `time-honored`, `universal`, `widely used`, `widespread`

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Outdated references

Reference `[RFC4566]` to `RFC4566`, which was obsoleted by `RFC8866` (this may
be on purpose).

Reference `[RFC5285]` to `RFC5285`, which was obsoleted by `RFC8285` (this may
be on purpose).

Reference `[RFC6347]` to `RFC6347`, which was obsoleted by `RFC9147` (this may
be on purpose).

Reference `[RFC5245]` to `RFC5245`, which was obsoleted by `RFC8445` and
`RFC8839` (this may be on purpose).

Reference `[RFC8843]` to `RFC8843`, which was obsoleted by `RFC9143` (this may
be on purpose).

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool
Robert Wilton Former IESG member
No Objection
No Objection (2022-10-03 for -03) Sent
Hi,

Thanks for this easy to review diff.

(1) p 23, sec 4.1.1.  Constructor

   [RFC8829] defined a policy known as "max-bundle", which, while
   defined identically to the "must-bundle" policy described above, was
   implemented by some implementations according to an earlier, pre-
   standard definition (in which, for example, no "m=" sections were
   marked as bundle-only).  As a result, "max-bundle" is considered
   deprecated, and new applications should use the "must-bundle" policy
   instead.

Disclaimer, I don't know this protocol.

I'm not sure why this isn't 'must use the "must-bundle" policy instead of "max-bundle."'

It is somewhat unclear to me how implementations move from RFC8829 to this RFC.  Specifically, it is unclear, for implementations that follow this RFC rather than RFC8829 (which is being obsoleted by this RFC), what, if any, handling of "max-bundle" is permitted.  My interpretation is that the "max-bundle" policy can still be specified, but its behaviour is not well-defined.  Further, is it appropriate for new implementations to also support "max-bundle" and treat it identically to "must-bundle", or should they reject that policy?

In summary, I think that some extra explanation here might be helpful.

Regards,
Rob