Subject: [rtcweb] Ben Campbell's Yes on draft-ietf-rtcweb-jsep-24: (with COMMENT)

From: Ben Campbell <[email-masked]>
Date: Mon, 11 Dec 2017 19:49:05 -0800

Ben Campbell has entered the following ballot position for
draft-ietf-rtcweb-jsep-24: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-rtcweb-jsep/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I'm balloting "yes", but I have some minor comments and nits:

Substantive:

- General:
I still find the edges around where SDP is and isn't required a bit vague.
Section 3.3 says that the JSEP implementation internal representation is SDP.
While I have trouble imagining implementing this otherwise, do we really mean
to mandate the internals? Is there an assumption that said internal
representation is _serialized_ SDP vs some abstract internal representation?

Section 3.1 says that the signaling model is not specified. Is there an
implicit assumption that, however things are signaled, the signaling process
moves around serialized SDP? Or is it envisioned that one could write an
application without serialized SDP occurring anywhere above the API?

-5, throughout: There are a number of normative keywords used in constructions
like "... MUST foo, as described in RFCXXX". That construction is ambiguous
about whether it defines a normative requirement to do something, and the doing
of that something is described in the RFC, or if describes the referenced RFC
as defining the MUST.

In general, it's best to avoid using 2119/8174 keywords to talk about external
requirements, except as a direct quote. I think this is especially true here
given the interdependencies between drafts in cluster 238. If in the future we
update a dependency to change a normative requirement, we risk creating
conflicts, and we make it unclear which text is authoritative.

-5.1.1: I think the operative requirement is "MUST support" rather than "MUST
indicate support". (While indication may also be required, it seems a
consequence of "MUST support".)

-5.1.2: " Any profile in the offer matching one of the following MUST be
accepted:" Isn't the real requirement that any of the following must be
interpreted the same, or otherwise in some specified way? I assume there may be
completely unrelated reasons to reject an m-section, but the "MUST be accepted"
language seems to overrule that.

-5.4: "The SDP returned from createOffer or createAnswer MUST NOT be changed
   before passing it to setLocalDescription."
Is that a requirement on the JSEP implementation, or the javascript
application? If the latter, is it appropriate to put normative requirements on
the application? It seems like it would be better to normatively state the JSEP
implementations's behavior when the application does something incorrect. 
(Which seems to be done further down the page.)

-5.7: " The effect of rollback MUST be the same regardless of whether
setLocalDescription or setRemoteDescription is called." Does that mean if I
call setLocalDescription, then rollback with a call to setRemoteDescription,
_both_ are rolled back?

-5.8.3: The MUSTs in this section seem to be putting requirements on the SDP
being parsed. Shouldn't they instead describe the parser's behavior based on
that SDP input? The correctness of the SDP being parsed seems beyond the
control of the implementation.

-8, second paragraph: When describing how the javascript is untrusted, it may
be worth mentioning that it may have been downloaded from an untrusted source.

Editorial and Nits:

-1.1: Please expand ICE and MCU on the first mention.
s/config/configuration
Sentence starting with "Through its abstraction of signaling...": Should that
say "Though it abstracts signaling..."

-2: Please consider using the boilerplate from RFC 8174. There are a number of
instances of lowercase versions of normative keywords.

-3.2, paragraph 2: The MUST seems like a statement of fact.

-3.4.1, 2nd paragraph: Please expand or explain "mid" on first mention. (It is
explained 3.5.2.1)

-3.5.1, last paragraph: Inconsistent capitalization: "MID"

-3.5.2.1: Please expand (or define) "ufrag" on first mention.

-3.6.1, first paragraph: is "intersect" the correct verb here? Missing
conjunction in "hardware decoder capababilities, local policy".

-3.6.2, 2nd paragraph: s/"may be producing"/"may produce"

-4.1.2: Please expand LS on first mention. (I can guess from context. Please
don't make me :-)  )

- 4.1.6: s/"codec/RTP/RTCP"/"codec, RTC, or RTCP"
"for each SDP line, the generation of the SDP will follow the process defined
for generating an initial offer from the document that specifies the given SDP
line." While I worked out what that means, I found "document" to be ambiguous.
At first I interpreted it as the "SDP document" rather than "the RFC". (Note
this occurs several times in subsequent sections.)

-4.1.8, third paragraph: ""pranswer" indicates that a description should be
parsed as an
   answer, but not a final answer"
I think it would be more clear to say "... parsed as a provisional answer,
rather than a final answer".

-5.2.1: Paragraph starting with: " Each m= section, provided it is not marked
as bundle-only, MUST generate..." The m= section is not the real actor here, is
it?

---

From: Justin Uberti <[email-masked]>
Date: Thu, 21 Dec 2017 18:26:42 -0800

Thanks for the detailed comments, will send a full response shortly.

On Mon, Dec 11, 2017 at 7:49 PM, Ben Campbell <[email-masked]> wrote:

> _______________________________________________
> rtcweb mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtcweb
>

---

From: Justin Uberti <[email-masked]>
Date: Thu, 21 Dec 2017 18:31:40 -0800

> https://datatracker.ietf.org/doc/draft-ietf-rtcweb-jsep/

I suppose this could be restated to say that we can *think* of
SessionDescriptions
as SDP containers, but the internal representation could be
anything, as long as it can roundtrip to SDP. Would that be preferable?

> application without serialized SDP occurring anywhere above the API?

The API does move around session descriptions, and, as mentioned above, they
can be thought of as SDP containers. However, the application signaling
protocol need not involve SDP at all, as mentioned:

     JSEP does not specify a particular signaling model or state
     machine, other than the generic need to exchange session
     descriptions in the fashion described by
     <xref target="RFC3264"></xref> (offer/answer)

IOW, the app signaling model is not specified, but the API interaction
model does involve the use of SDP containers.

> conflicts, and we make it unclear which text is authoritative.

Generally, this is pointing the reader at a particular normative section,
   and saying that yeah, you MUST follow that section as written. Perhaps
   the use of 2119 language here isn't ideal, but I don't see this as
   confusing. Do you have a suggestion about how this should be handled
instead?

> consequence of "MUST support".)

Session descriptions can't really support these specs, but they can
       indicate support by including the appropriate lines. I tend to think
       this is correct as written.

> language seems to overrule that.

Yes, perhaps we could be clearer and say
       "MUST be considered valid" (in reference to the profile).

> (Which seems to be done further down the page.)

This section is, as you mention, application guidance. Perhaps RFC2119
     language doesn't make sense here, but the guidance here seems like a
good
     summary for how the application should behave.

> Well, there is only a pending local at this point, and it is rolled back
by setRD.

> control of the implementation.

Yes, the correctness of the SDP is beyond the control of the impl, but,
the handling of broken SDP is spelled out in the beginning of this section:

        If any line is not well-formed, or cannot be parsed as
        described, the parser MUST stop with an error and reject the
        session description, even if the value is to be discarded. This
        ensures that implementations do not accidentally misinterpret
        ambiguous SDP.

Saying that a specific line MUST have X and Y is then a shorthand for
repeatedly saying that if the line cannot be parsed properly, the impl MUST
stop with an error.

> be worth mentioning that it may have been downloaded from an untrusted

yes, we could simply say that it is untrustworthy as a result of being
downloaded from an untrusted source

> Editorial and Nits:

Good catches here. Will fix these except for a few exceptions noted below.

> -3.6.2, 2nd paragraph: s/"may be producing"/"may produce"

"may be producing" seems OK to me. Did you have a specific concern?

> rather than a final answer".

Parsing has only 2 modes, offers and answers; the provisionality of the
       answer only affects how it is handled once parsed. I think this
should
       stay as-is.