Subject: Re: [sipcore] AD Evaluation of draft-ietf-sipcore-sip-push-20 - Ben's editorial comments

From: Christer Holmberg <[email-masked]>
Date: Fri, 30 Nov 2018 16:41:43 +0000

Hi,
    
>    - All instances of "binding refresh REGISTER" should be "binding-refresh REGISTER". (This is true anywhere binding-refresh is used as an adjective.)
  
I will change as suggested.

>    - There is still a lot of incorrect comma usage, especially commas where they don't belong. I will call out the ones I noticed.
  
Ok.

>  - The phrases "request for a new dialog" and  "standalone  SIP request" are used heavily and repeatedly, often in combination. The common terminology has 
> historically been "dialog-initiating request" and "out-of-dialog request". These are, at least in combination, a bit shorter.

I can modify as suggested, but in the past people have claimed that an initial INVITE would count as an "out-of-dialog request".

---

>  §1
>    - paragraph 5: Missing word in "awaken other means". I suggest "awakened by other means".
>    - Paragraph 6: "... SIP request for a new dialog, or a standalone
>    SIP request, addressed towards a UA...": Both commas should be removed.
  
Will fix.

>    - Figure 1: The "SIP 200 Okay" label is separated from the line it labels by a page break. (Other text changes may change the 
> pagination, so this is probably one for the RFC editor if it's still an issue in the final version.)
   
Yes. Because, even if I fix it now, any change based on the IESG review may mess it up again.

---

>    §4.1, 7th paragraph ("Note"): The first sentence seems odd in the context of the previous paragraph, which talked extensively 
> about UAs using non-push mechanisms.
   
Perhaps I could say:

"NOTE: If the UA uses a non-push mechanism to wake and send a binding refresh REGISTER request, such REGISTER..."

---

>    §4.2,
>    - first paragraph:
>   
>    -- first sentence: s/"network"/"SIP Network".
>    -- "... UA has received a response to the REGISTER request, with the
>    push notification information ...": improper comma.
  
Will fix as suggested an remove comma.

>    - 3rd paragraph, "... early enough, in order for...": I suggest "... early enough for..."
  
Will fix as suggested.
 
---

 >   §5.3.1
 >   - 1st paragraph, "... towards the UA associated with the
 >   REGISTER request, or perform any other procedures...": improper comma.

Will fix as suggested.
   
>    - 5th paragraph:
>    -- "... proxy that is not between the push proxy and the UA...". Doesn't this really mean a proxy that is between the push 
> proxy and the registrar? If so, saying where the proxy _is_ would be clearer than saying where it is _not_.
  
I used to have upstream/downstream terminology here, but was asked to change it (since people confuse what is upstream and what is downstream). The text talks about the push proxy knowing that there is another push proxy (between itself and the registrar) that will be able to handle the PNS.

>    -- "... (if the Contact header field URI of the REGISTER request  contains a pn-prid SIP URI parameter) ..." Could we 
> have gotten this far if that were not true? The sentence would be easier to read without it. (I think this repeats a few times.)
   
I could remove the following part:

"and will request (if the Contact header fieldURI of the REGISTER request contains a pn-prid SIP URI parameter),"

...if it makes the text easier to read and understand.

>    - 6th paragraph: "MUST only" constructions can be ambiguous. It is better to state them in terms of "MUST NOT unless..."
  
I don't think MUST NOT fits here. Could we say "MUST indicate support only of the"?

>    - 2nd to last paragraph, "The proxy MUST be able..." It would be better to state this in terms of what the proxy _does_, not 
>    what it is _able_ to do. For example, "... MUST NOT ... unless it has determined that the PNS supports the VAPID mechanism."

Or, could we just remove "be able to"?
    
---

>    §5.3.2
>    -  3rd paragraph: "This can happen if the UA sends a binding refresh REGISTER request with a new contact...": This 
>    seems to ignore the potential of multiple contacts. I think the real condition is when the previous binding is _removed_.

Correct. I suggest:

   "This can happen if the UA sends a binding refresh REGISTER request that does not contain the previously registered contact at the
   same time the registrar forwards a SIP request towards a UA using the previously registered contact in the Request-URI."
    
>    -6th paragraph: I don't understand the meaning of this paragraph. (Also, both commas are incorrect.)
  
It means that, even if the contact URI and Request-URI don't match (because the UA has removed a previous contact), the pn- parameters of the other contact(s) will still match.
  
>    -11th paragraph: s/"losing race that results"/ "losing the race, which results"  ; or "... a race..."
  
I suggest: "a race".

---

>    §6.2.3: Parts of this section seem specific to SIP-Events dialogs. I assume that's not the intent?

Correct, not sure where the NOTIFY comes from. It shall be "the mid-dialog request".

Regards,

Christer

---

From: Ben Campbell <[email-masked]>
Date: Fri, 30 Nov 2018 15:39:36 -0600

Hi,

This looks mostly good. Just a few follow-up comments below:

Thanks!

Ben.

> On Nov 30, 2018, at 10:41 AM, Christer Holmberg <[email-masked]> wrote:
> 
> 

[...]

> I can modify as suggested, but in the past people have claimed that an initial INVITE would count as an "out-of-dialog request”.

Sigh. Okay :-)

[...]

> I used to have upstream/downstream terminology here, but was asked to change it (since people confuse what is upstream and what is downstream). The text talks about the push proxy knowing that ther…

Right, but wouldn’t “between the proxy and the registrar” be clearer than “not between the proxy and the UA”?

> I don't think MUST NOT fits here. Could we say "MUST indicate support only of the”?

That probably works.

> Or, could we just remove "be able to”?

Okay

[...]

[...]

> It means that, even if the contact URI and Request-URI don't match (because the UA has removed a previous contact), the pn- parameters of the other contact(s) will still match.

Okay.

> I suggest: "a race”.

WFM.

[...]