On Tue, Nov 15, 2022 at 7:12 PM <aster...@phreaknet.org> wrote: > On 11/15/2022 9:56 AM, Joshua C. Colp wrote: > > On Tue, Nov 15, 2022 at 10:50 AM <aster...@phreaknet.org > > <mailto:aster...@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. >
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. -- Joshua C. Colp Asterisk Project Lead Sangoma Technologies Check us out at www.sangoma.com and www.asterisk.org
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev