<div dir="ltr"><div dir="ltr">On Tue, Nov 15, 2022 at 7:12 PM <<a href="mailto:asterisk@phreaknet.org">asterisk@phreaknet.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 11/15/2022 9:56 AM, Joshua C. Colp wrote:<br>
> On Tue, Nov 15, 2022 at 10:50 AM <<a href="mailto:asterisk@phreaknet.org" target="_blank">asterisk@phreaknet.org</a> <br>
> <mailto:<a href="mailto:asterisk@phreaknet.org" target="_blank">asterisk@phreaknet.org</a>>> wrote:<br>
><br>
><br>
>     If res_pjsip_pubsub would need to be extended to support this,<br>
>     would it<br>
>     reasonable to add a callback to a pubsub module that allows it<br>
>     access to<br>
>     the pjsip_tx_data, so it can do whatever it needs to with it,<br>
>     before the<br>
>     response gets sent? Or another preferred method of allowing<br>
>     modules to<br>
>     add headers?<br>
><br>
><br>
> At a surface it is probably fine.<br>
<br>
Thanks, doing that allowed just what I needed to do.<br>
<br>
Next limitation... the new_subscribe callback is supposed to return 200 <br>
(or some other code) to accept or reject the subscription. The only <br>
arguments are the endpoint name and resource name.<br>
This is not really always sufficient; it may be necessary to approve or <br>
reject the subscription using some information present in the <br>
subscription itself (for example, a header). I think this is all <br>
consequent of the very narrow range of scenarios that res_pjsip_pubsub <br>
was written for originally.<br>
<br>
The subscription_established callback is actually perfectly set up for <br>
this. We have a handle on the ast_sip_subscription, and can call <br>
ast_sip_subscription_get_header if needed to get the header.<br>
However, this requires approving all subscriptions with a 200 in the <br>
new_subscribe callback, only to potentially realize it should be <br>
rejected in the subscription_established callback. This is too late <br>
because the 200 OK already gets sent to the endpoint before <br>
subscription_established gets called.<br>
<br>
So, the only good solution is to extend new_subscribe to accept a third <br>
argument: rdata, since a subscription hasn't yet been created at that <br>
point so we could not use ast_sip_subscription_get_header to fetch <br>
headers. Yuck, since it's a public API... there could also be a <br>
new_subscribe_with_rdata callback that gets executed instead if a module <br>
defines one. Or maybe we can break ABI and go master only here if that <br>
would be too inelegant.<br></blockquote><div><br></div><div>Even adding a new callback will break the ABI. I think fundamentally this work should only occur in master where changing ABI is fine. I have concerns that it will cause unintended consequences in some way and I've had enough of those in the past year in release branches.</div></div><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div style="font-family:tahoma,sans-serif"><font color="#073763">Joshua C. Colp</font></div><div style="font-family:tahoma,sans-serif"><font color="#073763">Asterisk Project Lead</font></div><div style="font-family:tahoma,sans-serif"><font color="#073763">Sangoma Technologies</font></div><div style="font-family:tahoma,sans-serif"><font color="#073763">Check us out at <a href="http://www.sangoma.com" target="_blank">www.sangoma.com</a> and <a href="http://www.asterisk.org" target="_blank">www.asterisk.org</a></font><br></div></div></div></div></div></div></div></div></div></div></div>