The WebSocket Protocol as a Transport for the Session Initiation Protocol (SIP)
draft-ietf-sipcore-sip-websocket-10

I like this document, and think this extension to SIP will be useful.
Substantive comments; these are non-blocking, but please consider them
seriously, and feel free to chat with me about them:

-- General --

   _This section is non-normative._

I understand that the WebSockets document does this, but... is this really necessary?  The IETF manages to publish a large number of normative documents that contain non-normative sections, examples, and so on.  Most of them don't have this silliness in them.

This is non-blocking -- I can't justify a DISCUSS on this point -- but I *really* ask you not to do this.

-- Section 4.1 --
In the description of the handshake, you say that the Sec-WebSocket-Protocol header must "include" and "contain" the string "sip".  Does that mean that there might also be other things in the value of that header?  Or should these say "consist of"?

-- Section 5.2.4 --

I suggest wrapping this up by being explicit about the change to the 3261 text.  Like this:

ADD
   The sentence quoted above from [RFC3261] section 18 is thus
   amended as follows:

      All SIP elements MUST implement at least one of the following:
        *  Both UDP and TCP
        *  SIP WebSocket
      SIP elements MAY implement other protocols.
END

-- Section 6 --
Can you add any explanation of the differences among the various keepalives, and any advice about how one might choose which to use?  Is there any reason to use multiple of them?  If so, please explain; if not, please say so.

========
Other comments; no need to respond to these. Take them or modify them
as you please:

-- Section 4.2 --
I find this section very odd, though, I suppose, not objectionable.  I expected he third sentence to say something like, "Therefore, everything's A-OK, and we can do SIP over WebSockets.  But no, you simply say that the SIP clients and servers you're defining here are just like any other SIP clients and servers.  Is this really necessary to say?  Would anyone possibly imagine that, say, they could write a client that didn't do binary?

-- Section 7 --

   For many web applications the value of such a
   Cookie is provided by the web server once the user has authenticated
   themselves to the web server

Oof.  Can we avoid singular usage of "themselves"?  It should work fine to say, "once the user is [or has] authenticated to the web server," leaving off the reflexive pronoun.

-  Agreed with Barry's comment on Section 3 " _This section is non-normative._"

I see Section 3 as a useful introductory text, exactly like Section 1.
And ... Section 1 doesn't have "_This section is non-normative._ ". I wonder why if I follow your logic?
If the Section 3 content was included in section 1, would this required "_This section is non-normative._If "?
You see, " _This section is non-normative._" leads to so more questions than clarifications.
Like Barry (and others),  "I *really* ask you not to do this. "

- - As far as I can tell at https://www.iana.org/assignments/websocket/websocket.xml#subprotocol-name, SIP will be the first RFC-specified assignment.
From 6455

   10.  The request MAY include a header field with the name
        |Sec-WebSocket-Protocol|.  If present, this value indicates one
        or more comma-separated subprotocol the client wishes to speak,
        ordered by preference.  The elements that comprise this value
        MUST be non-empty strings with characters in the range U+0021 to
        U+007E not including separator characters as defined in
        [RFC2616] and MUST all be unique strings.  The ABNF for the
        value of this header field is 1#token, where the definitions of
        constructs and rules are as given in [RFC2616].

Should this document explain what happens if some other subprotocols are requested at the same time?
Disclaimer: I'm not an expert in Websocket, and this comment might just prove it. In other words, if this is a stupid comment, don't bother replying.

- Any reason why second paragraph of Section 4.1 is indented. I looks like a quotation
Same remark for the last paragraph of section 5.1

draft-ietf-sipcore-sip-websocket

5.2.1 vs 5.3  

wss = tls in 5.2.1 

sips + ws = port 443 in 5.3. wouldn't it be  wss?

   In the absence of DNS SRV resource records or an explicit port, the
   default port for a SIP URI using the "sip" scheme and the "ws"
   transport parameter is 80, and the default port for a SIP URI using
   the "sips" scheme and the "ws" transport parameter is 443.

Thanks for addressing all of my comments. One suggestion for the new text in 4.2:

      If there is at least one non-UTF-8 symbol in the whole SIP message
      (including headers and body) then the whole message MUST be sent
      within a WebSocket binary message.

Just for clarity, I suggest instead:

      If there is any non-UTF-8 text data (or any non-textual data) in
      the any part of the SIP message (including headers and body) then
      the whole message MUST be sent within a WebSocket binary message.

Support Stephen's discuss positions.

Thanks for handling my discuss.

I think there was one more thing from the mail on that, but 
am not sure if that needs to be included or not. The issue
was how to know to which server you're supposed to be
connected. Would it be better if 5.5 last para also said that
when NAPTR/SRV can't be used if you want to make a
call to "sips:joe@example.com" then your browser script
should start with:

    "var conn = new WebSocket('wss://example.com:443')"

Or is that stated somewhere else? (Or is it wrong
maybe;-)


-- old comments below

write-up nit: AD should now be Richard I guess

- Abstract: I don't get why this "updates" 3261? Isn't it ok
to implement 3261 without this? I guess that's down to
section 3 changing a 3261 MUST, but that still doesn't seem
to require a 3261 implementer to do something different if
they're not doing WS. (In which case, how is section 3
non-normative? Is the ref to section 3 in section 1 correct
actually?) Aha! Is it 5.2 that you mean and is the reason for
the updates because you're adding to the ABNF and don't want
someone else to add a conflicting thing later? If so, saying
that in section 1 would be good. (And its a fine reason to
update 3261.)

- 4.1: what is version 13?

- Is 5.2.4 really an update to 3261? It doesn't seem like one
to me, but rather you're saying that implementations of this
are ok if they only do WS. That is a difference from, but not
an update to, 3261.

- 9.1 s/recommended/RECOMMENDED/ would be better if you do
mean the 2119 term.

- 10.2 - what's D2W mean? No harm explaining.

- 9.2 seems to imply that 5246 needs to be a normative
reference.

Joel noticed this in his comment:

  wss = tls in 5.2.1 
  sips + ws = port 443 in 5.3. wouldn't it be  wss?

If this is in fact a problem, the same problem might exist in 10.2:

   Services Field   Protocol   Reference
   --------------   --------   ---------
   SIP+D2W          WS         TBD: this document
   SIPS+D2W         WS         TBD: this document

I am saying this not because I know it's wrong but because it looked like it might be, so no need to respond if this is in fact correct.

I agree with Barry's comment on "_this section is non-normative_", and would tend to echo Pete's recommendation not to use UTF-8 transport. Overall, the document looks good—thanks for doing this work!