Subject: Re: [sipcore] Review of sip-push-10 - Martin's comments

From: Christer Holmberg <[email-masked]>
Date: Thu, 29 Mar 2018 07:41:47 +0000

Hi Martin,

Thank You for the review! Please see inline.

>I also found the prose a little hard to follow.  Some restructuring
>might go a long way to improving clarity.
>
>The description of RFC 8292 integration isn't particularly helpful.
>Does the UA register once, discover that the Proxy/Registrar supports
>vapid, then re-register immediately?  None of that is well-described.

I think this is related to your comment on Section 4. See below.

---

>The document talks in passing about its assumptions about the push
>message, but never actually states the requirements on that message.
>Please add a clear description of what the message is.  For instance,
>is any message sufficient?  Or does the message have to be empty?
>What would it mean to receive a message with a payload?

The text does say that the proxy MUST NOT include any payload. Only the
push notification itself is used.

>An important limitation of APNS and FCM here is that a single app
>can't hold multiple contexts, therefore any message it receives will
>cause re-registration even if the message was for other purposes
>("Scarves, 5% off until Monday!", bam).  You should document that.

Ok, I will look into that.

---

>Section 4:
>Can a UA include Feature-Caps on its own?  Maybe it doesn't want to do
>all the push service stuff until it knows that the server supports the
>features it needs (vapid for example).

A UA cannot include a Feature-Caps.

But, we could define a sip.pns media feature tag, that a UA can include to
indicate generic support of SIP Push, but it would not be used to request
usage of SIP Push.

Then, when the proxy receives it, it would return the normal information
about what it supports, but it would not yet start requesting push
notifications.

---

>Section 5.3.1:
>I don't understand the relevance of all the 423-related text here.
>Maybe it's a lack of clarity about the expected processing model.
>FWIW, the only thing of value in this very long paragraph is that
>sip.pns is not to be included with a 423.
>
>However, I would argue the opposite and that sip.pns is most valuable
>in a 423.

The text does not say that sip.pns is not included in 423.

But, the text does not say that sip.pns IS included either, so I can add
text about that.

>For example, a UA knows that it only stays alive for a
>short time, so it conservatively creates registrations with a short
>duration.  That's not very useful, but the UA might expect to only be
>available while the app is in the foreground where it is able to
>refresh frequently.  However, if it receives a signal from the
>proxy/registrar that indicates push support (in a 423 perhaps), it
>might be inspired to extend the registration and rely on push.
>
>How would a proxy decide to send a 555 response or ignore the push
>information in the REGISTER request?

Local configuration.

>Why would a proxy add Feature-Caps instead of just stripping out the
>push parameters?  (That's probably a consequence of my ignorance of
>REGISTER procedures, but I am curious.)  Why does the Feature-Caps
>need a value?  The downstream proxy only needs to know that push is
>taken care of, not which push is in use.  Is it just for symmetry with
>the downstream notification?

There is no technical reson for including a value - it is enough to inform
the downstream proxy that push is taken care of.

It¹s just for information. I can add a note indicating that.

>What value does the parameter take? The pn-provider label?

Yes.

---

>Section 5.3.2:
>Why include so much text here?  If the proxy considers the UA to be
>registered - and only challenged with respect to active connectivity -
>then normal SIP processing applies with only one tweak: send a push
>message and wait until the registration is restored.
>
>The idea that the proxy might be able to correlate a REGISTER request
>that includes a different contact URI with this new transaction/dialog
>is a little baffling - either the registration is restored with the
>right contact, or the registration doesn't happen and message delivery
>will ultimately time out and fail.

The text covers the ³race condition² case where the UA changes its contact
as the same time as the proxy receives a request towards the UA, using the
old contact. The request should not be forwarded towards the UA using the
old contact.

>In this regard, the only text you need is the timeout text you have near
>the end of this section.

In this case I disagree. I think the text is needed. In addition to the
³race condition² and timeout cases, it also specifies that the proxy needs
to wait for the REGISTER response before forwarding the request, and what
the proxy does in case of a non-2xx REGISTER response.

---

>Section 6 contains no information.  If anything, you can use NAT as
>further justification for use of a push notification service, but it
>doesn't warrant an entire section.

I suggest to move the first paragraph to Section 4, and second paragraph
to Section 5.3.2.

---

>Section 7.5:
>I don't understand the purpose behind sip.pnsreg is defined here.  How
>is this different than, say, the usual registration expiration logic?
>Maybe this goes back to the basic registration logic.

It is used to indicate that a UA can use e.g., internal timers to send
periodic re-registrations, but still want to use SIP Push when there are
incoming requests.

---

>Nits:
>Section 7.6: typo: indciates

Will fix.

---

>Section 11: "When Firebase..." seems to be copy-pasta

Will fix.

³When Generic Event Delivery Using HTTP Push is used,Š"

>s/push URI/push subscription URI/

"push URI² terminology is more common in RFC 8030, so that¹s why I used
it. But I can replace it with ³push subscription URI².

>I don't think that you need to include a URI for RFC 8030.

I¹ll remove it.

Regards,

Christer