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

[Eric Rescorla <[e-mail]>]

On Tue, Jan 2, 2018 at 3:36 PM, Ben Campbell <[e-mail]> wrote:

> >
> >
> >> On Dec 21, 2017, at 8:31 PM, Justin Uberti <[e-mail]> wrote:
> >>
>
> […]
>
>
> >> ----------------------------------------------------------------------
> >> 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?
>
> I think that would be more accurate.
>
> >
> >> 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.
>
> The clarification you proposed to my previous comment helps here, too.
>
> >
> >> -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?
>
> It’s a technicality, but it can create confusion about which text is
> authoritative. That’s not an issue as long as both are in agreement, but
> future updates to either this or that spec can easily mess up that
> agreement.
>
> My personal preference is to reserve 2119/8174 keywords for the text that
> is authoritative for the requirement, and to have other text that refers to
> that requirement do it descriptively. For example, “RFC XXXX mandates foo”.
>

I guess this is a matter of opinion. I think that it's useful to list the
rules here using
2119 language. I think the text makes clear that in this construction it's
the
RFC that we are pointing to that's authoritative.


>
>
>
> >
> >> -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).
>
> That would help, thanks.
>
> >
> >> -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.
>
> My personal preference would be to avoid use of 2119 keywords for things
> the JSEP implementation does not control.
>
> >
> >> -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.
>
> Okay.
>
> >
> >> -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.
>
> Again, I don’t think the use of 2119/8174 keywords for things beyond the
> control of the implementation fits the sense of those documents. I suggest
> either stating these without 2119/8174 keywords, or adding something to the
> boilerplate to the effect of “When used in to describe requirements for SDP
> inputs, these terms indicate requirements for that SDP to be considered
> well-formed”.
>

I'm comfortable with the current language....

-Ekr


> >
> >
> >> -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:
> >>
> >>
>
> […]
>
> >> -3.6.2, 2nd paragraph: s/"may be producing"/"may produce"
> >>
> > "may be producing" seems OK to me. Did you have a specific concern?
> >
>
> Just a grammar nit. “may be producing” uses the progressive aspect. That
> indicates temporary action ongoing at some point of time. Is that the
> intent?
>
> >> -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.
>
> The existing text already talks about parsing as a final answer. If
> parsing is the same either way, perhaps it should say “treated as” rather
> than “parsed as”?
>
> >
> >
> >> -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
> >>
> >
>