[asterisk-dev] Adding headers to NOTIFY responses

[asterisk at phreaknet.org]

On 11/15/2022 9:56 AM, Joshua C. Colp wrote:
> On Tue, Nov 15, 2022 at 10:50 AM <asterisk at phreaknet.org 
> <mailto:asterisk at phreaknet.org>> wrote:
>
>
>     If res_pjsip_pubsub would need to be extended to support this,
>     would it
>     reasonable to add a callback to a pubsub module that allows it
>     access to
>     the pjsip_tx_data, so it can do whatever it needs to with it,
>     before the
>     response gets sent? Or another preferred method of allowing
>     modules to
>     add headers?
>
>
> At a surface it is probably fine.

Thanks, doing that allowed just what I needed to do.

Next limitation... the new_subscribe callback is supposed to return 200 
(or some other code) to accept or reject the subscription. The only 
arguments are the endpoint name and resource name.
This is not really always sufficient; it may be necessary to approve or 
reject the subscription using some information present in the 
subscription itself (for example, a header). I think this is all 
consequent of the very narrow range of scenarios that res_pjsip_pubsub 
was written for originally.

The subscription_established callback is actually perfectly set up for 
this. We have a handle on the ast_sip_subscription, and can call 
ast_sip_subscription_get_header if needed to get the header.
However, this requires approving all subscriptions with a 200 in the 
new_subscribe callback, only to potentially realize it should be 
rejected in the subscription_established callback. This is too late 
because the 200 OK already gets sent to the endpoint before 
subscription_established gets called.

So, the only good solution is to extend new_subscribe to accept a third 
argument: rdata, since a subscription hasn't yet been created at that 
point so we could not use ast_sip_subscription_get_header to fetch 
headers. Yuck, since it's a public API... there could also be a 
new_subscribe_with_rdata callback that gets executed instead if a module 
defines one. Or maybe we can break ABI and go master only here if that 
would be too inelegant.

In any case, the problem could be solved by passing rdata into all of 
the functions in res_pjsip_pubsub that call the new_subscribe callback, 
since it's not currently available in those functions.
I think the only reason it isn't done this way now is so that 
new_subscribe can be unit tested, without having a pjsip_rx_data. The 
unit test could simply pass in NULL for rdata, though, since it won't 
actually need it for what it does (nor would any existing modules).

Any thoughts/objections?
Thanks!