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

[Justin Uberti <[e-mail]>]

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

> 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?
>

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?

>
> 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?
>

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.

>
> -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.
>

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?

>
> -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".)
>

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.

>
> -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.
>

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

>
> -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.)
>

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.

>
> -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?
>
> Well, there is only a pending local at this point, and it is rolled back
by setRD.


> -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.
>

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.

>
> -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.
>

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.

>
> -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"
>

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

>
> -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".
>

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.

>
> -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?
>
>
> _______________________________________________
> rtcweb mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/rtcweb
>