| Summary: | ASTERISK-13684: [patch] export the SIP peer username of the transferer | ||||
| Reporter: | klaus3000 (klaus3000) | Labels: | patch | ||
| Date Opened: | 2009-03-03 09:42:50.000-0600 | Date Closed: | |||
| Priority: | Major | Regression? | No | ||
| Status: | In Progress/In Progress | Components: | Channels/chan_sip/NewFeature | ||
| Versions: | Frequency of Occurrence | ||||
| Related Issues: |
| ||||
| Environment: | Attachments: | ( 0) app_dial--FORWARDER.diff ( 1) asterisk-patch-sip-transfer-peername-trunk.txt ( 2) patch_chan_sip_transfer_forward_peername-1.4.23.txt ( 3) patch_chan_sip_transfer_forward_peername-1.6.2.txt | |||
| Description: | Hi! When a blind transfer is initiated from a SIP, often the name of the transferer is needed (e.g. billing, applying restrictions ...). Currently there is a variable called SIPTRANSFER_REFERER which contains the value of the Refered-By header - but this variable is not trustworthy, as the SIP client can put anything into this variable. Attached patch adds the variables SIPTRANSFERER_PEERNAME and SIPTRANSFERER_USERNAME which contain the respective SIP username (peer or user) ****** ADDITIONAL INFORMATION ****** As I am not up2date with the current peer/user behavior maybe the SIPTRANSFERER_USERNAME can be removed. I greped the code and it seems that p->peername is also set in case of a SIP user, but not vice versa. | ||||
| Comments: | By: Tilghman Lesher (tilghman) 2009-03-03 10:55:23.000-0600 Is there not a header which you can access with SIP_HEADER() that will provide the same information? By: klaus3000 (klaus3000) 2009-03-03 17:03:29.000-0600 In general: no Further question: using SIP_HEADER in the TRANSFER_CONTEXT - which message does it use? REFER or initial INVITE ? In case of REFER it may happen (only in certain scenarios) that the username can also be found in the From header - but then the whole From header must be parsed which is for sure not trivial, and you never now if the From header contains the username at all. So, why taking the approach parsing a header manually? The authoritative, 100 % correct information is in the sip_pvt structure of the transferer. By: Tilghman Lesher (tilghman) 2009-03-03 17:14:48.000-0600 It may be, but we try to avoid setting arbitrary variables, other than what is there for backwards compatibility. We prefer if people implement dialplan functions which provide a wide assortment of information, without needlessly polluting the channel variable space. Upon further examination, the peername is already available as ${SIPCHANINFO(peername)}. If you wanted to add a similar value for username, that would be an acceptable change. By: klaus3000 (klaus3000) 2009-03-03 17:40:25.000-0600 The TRANSFER_CONTEXT is entered with the channel of the transferee. Thus I suspected that ${SIPCHANINFO(peername)} is the transferee peer, not the transferer. But I have not tested, I will try tomorrow. In case I am right, then also a function could not help as the information is lost, only the ->referedby member is copied but this one is useless. By: klaus3000 (klaus3000) 2009-03-04 04:42:21.000-0600 I just verified it: after the transfer the ${SIPCHANINFO(peername)} contains the transferee, not the transferer. By: Tilghman Lesher (tilghman) 2009-03-04 12:30:26.000-0600 Still, I'd like to see a dialplan function access the values, instead of polluting the variable namespace. If that means that you pass more values through the referedby structure, then that may be the approach to take. By: klaus3000 (klaus3000) 2009-03-05 05:56:21.000-0600 So a read only function TRANSFER(option), where option is: peer: returns the peer/user name of the transferer referedby: information from the SIP Refered-By header Obviously this function can be useful to other channels too. I wonder how transfers are signaled to the core. I couldn't find something like AST_CONTROL_TRANSFER By: klaus3000 (klaus3000) 2009-03-19 10:59:16 Hi! I attached a new patch. It is still not based on functions, but nice and also handles the case of a SIP forward (302 moved). I really wonder how people currently get the information "who" did the transfer/forward into the CDR. For the case of the forward I wonder what could be a nice way to implement this. In this FORWARD_CONTEXT there should be an option to fin out who did the forward (e.g. to apply restrictions (national, international) and perform correct billing). Thus, the peer information (in case of SIP) or maybe tech/peername would have to be copied from the original outgoing channel to the new outgoing channel. This is different to the case of transfer: Forward: the FORWARD_CONTEXT is executed with a new Local channel Transfer: The TRANSFER_CONTEXT is executed with the TRANSFEREE channel I think struct ast_chan needs to be extended to handle both cases (e.g. introducing a member "transforwarder" which contains for both cases the forwarder/transferer in the format technology/username) By: Olle Johansson (oej) 2009-09-03 14:50:26 We just discussed this on IRC. I wasn't aware of this bug report then. I think we need to look into this, we need to identify who caused a call forward or a transfer, if it's an identifiable peer. Maybe it's possible as tilghman says, but then we need to document it clearly. THink about a call to multiple devices dial(SIP/345&SIP/567) - one of them can cause a forward, we need to identfy whom. By: klaus3000 (klaus3000) 2009-09-04 03:44:45 I think my patch already handles such situations By: Olle Johansson (oej) 2009-09-04 04:12:50 Cool. Then we really need to look at it :-) By: Kai Hoerner (kaii) 2009-09-21 12:53:42 In the discussion on IRC with oej and others, we made clear that there is a difference between "call transfer" and "call forward". TRANSFER requests are (at least in channel SIP) handled inside the sip channel module. a variable BLINDTRANSFER is already being set when a SIP REPLACE occurs. this is not the case for attended transfers, but i think scenarios are rare where the dialplan needs information about who did an attended transfer _after_ a call is completed. (thus rendering an "ATTENDEDTRANSFER" variable unneccessary) FORWARD requests are raised from chan_sip into the core and the forward is actually processed in app_dial. in this case, no variable is set to indicate who actually transfered the call -- a valuable information in some scenarios. i started the discussion on IRC asking if it was a bug that "BLINDTRANSFER" was not set when a call forward occured. it is set on REPLACE, but not on REDIR events.. i was then told the difference between TRANSFER and FORWARD and we discussed about other solutions to retrieve this information from the dialplan. oej came up with a few ideas to solve this in dialplan, but none worked for me.. i totally agree on the need of such a variable. i patched this in 1.2 and 1.4 for our own needs. i just want to add a few points to think about in the discussion: * a call can be forwarded multiple times: SIP/10 -> SIP/20 -> SIP/30 ... i suspect that in your solution, as in mine too, the dialplan only recognizes a forward from SIP/20 to SIP/30, but not that SIP/10 was initially forwarding. * as forward requests are raised through the core and are handled inside the app itself (dial), i see this feature independent of channels. i implemented it inside app_dial and it works like a charm. (and i suspect it will work with other channels than chan_sip, too) By: klaus3000 (klaus3000) 2009-09-21 14:50:39 Yes, forward and transfer are different - in their meaning and implementation. Nevertheless, for billing purpose they might be identical (it is a common practice that the party who does the forward or transfer pays for the second call leg). I needed the information of the transferer or forwarder for billing purposes. Thus, I exported the information (the corresponding SIP peer) as channel variables. If I got it right, BLINDTRANSFER only exports the channel name, not the SIP peer. Further, I also have some dialplan magic to find out if a certain peer is actually allowed to do transfers or forwardings. Thus, I think it is very useful to have this information in the dialplan. There are already contexts which handle the transfer/fowarding: TRANSFER_CONTEXT and FORWARD_CONTEXT. Regarding multiple forwardings and transfers: No problem. The patch just exports the relevant peer as channel variable and in the config you can decide if you need both forwarders or only the last/first one. Eg. Caller -> SIP/10 -> SIP/20 -> SIP/30. SIP/10 sends a redirect. FORWARD_CONTEXT will be executed with FORWARDER_SIPPEER=SIP/10. Then SIP/20 sends a redirect. FORWARD_CONTEXT will be executed again with FORWARDER_SIPPEER=SIP/20. Thus, in FORWARD_CONTEXT I read the FORWARDER_SIPPEER and do the authorization and billing processing. Regarding your patch. I think it only exports the channel-name, but not the peer info. By: Kai Hoerner (kaii) 2009-09-21 15:24:09 Agreed, transfer and call forward may be the same under a users or billing point of view. that is also the reason why i wrote such a patch, too. on snom phones, the "transfer" button actually "forwards" any unanswered call. so for users this is totally transparent .. they just do a "transfer". Maybe I have a fundamental misunderstanding of the channel naming scheme, but doesn't the channel name contain the relevant peer name? In dialplan, you can use ${CUT(BLINDTRANSFER,,1)} to strip "SIP/10" from a channel name like "SIP/10-23cb1a". By: klaus3000 (klaus3000) 2009-09-22 03:38:47 > Maybe I have a fundamental misunderstanding of the channel naming scheme, but doesn't the channel name contain the relevant peer name? I guess this depends on the implementation in the channel driver. In my case I only use SIP thus I preferred to use the SIP-peername directly. By: Olle Johansson (oej) 2009-09-22 03:43:56 The channel name contains the creator's name. Outbound calls may have other names. You need to check patch with both outbound and incoming calls. Either way, a peer can transfer. By: klaus3000 (klaus3000) 2009-09-22 04:15:06 I have tested both possibilities: transferrer=caller, transferrer=callee, and it worked. By: Olle Johansson (oej) 2009-09-22 10:15:50 Will go through this and commit as soon as my build system is back up after upgrading to the Snow Leopard... :-) By: klaus3000 (klaus3000) 2009-09-23 07:26:38 uploaded patch for 1.6.2. quick note: FORWARDER_SIPPEER works only if promiscredir=no. Not sure if it makes sense to support promiscredir as it does not trigger dialplan processing. By: Sverre G (sverre) 2013-06-19 19:55:22.385-0500 I've recently upgraded to the latest version of Asterisk, and noticed the my extensions.conf wasn't working correctly on forwards, and it's because I was previously using 1.4 with this patch... but the latest version doesn't have this patch committed into the trunk. Why did this not get committed to trunk? Will this patch work on Asterisk 11.2.1? By: Walter Doekes (wdoekes) 2013-06-20 02:42:46.126-0500 Sverre G: does this work for you? {noformat} ... same => n,Set(_transferer=${destination}) same => n,Dial(SIP/${destination}) [inbound] exten => X!,1,Set(CDR(accountcode)=${IF($["${transferer}"!=""]?${transferer}:${SIPCHANINFO(peername)})}) {noformat} By: klaus3000 (klaus3000) 2013-06-20 03:28:07.953-0500 You set _transferer as global variable to the called party. But if A calls B, also A could initiate the transfer. Also after a transfer SIPCHANINFO(peername) would always be a wrong name, as the transferer is not in the call anymore. So I think in this case this might be wrong. Further, I would be careful if the variable magic also works with in complex scenarios after multiple call forwarding and transfers. My patch should always work, as it fetches the authoritative data (the party who initiated the transfer) directly from the respective SIP structures. By: Corey Farrell (coreyfarrell) 2017-12-14 13:35:37.414-0600 Are you interested in pursuing this further? If so the patch will need to be rebased so it can apply the the current {{master}} branch and it will need to go through code review. ---- Thanks for the contribution! If you'd like your contribution to be included faster, you should submit your patch for code review by the Asterisk Developer Community. To do so, please follow the Code Review \[1\] instructions on the wiki. Be sure to: * Verify that your patch conforms to the Coding Guidelines \[2\] * Review the Code Review Checklist \[3\] for common items reviewers will look for * If necessary, provide tests for the Asterisk Test Suite that verify the correctness of your patch \[4\] When ready, submit your patch and any tests to Gerrit \[5\] for code review. Thanks! \[1\] https://wiki.asterisk.org/wiki/display/AST/Code+Review \[2\] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines \[3\] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist \[4\] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Documentation \[5\] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage | ||||