[asterisk-dev] [Code Review] Preserve user defined CDR information after a Local channel is masqueraded

[Matt Jordan]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1985/
-----------------------------------------------------------

(Updated June 14, 2012, 3:24 p.m.)


Review request for Asterisk Developers, Terry Wilson, Tilghman Lesher, and rmudgett.


Changes
-------

So after some more thought, I addressed rmudgett's finding by checking that both the p->owner and p->chan->_bridge cdr pointers are non-null before doing the swapping.  It'd be an incredibly rare event for either to be NULL, but there's no point in risking it.

Additionally, this now destroys the variables on p->chan->_bridge before copying over p->owner's variables.  While it may be conceivable that someone would want p->chan->_bridge's variables to be preserved, this runs into two difficulties:
1) Its pretty difficult to set the CDR variables as it is on the chan->_bridge channel (which is why this stuff is getting set on the Local channel (p->owner) in the first place)
2) This keeps the semantics of "user defined stuff on the Local channel is kept; user defined stuff on the channel masquerading into the Local channel is not", as opposed to a 'merge' type operation

In testing this kept the other CDR fields intact, as desired:

"","1","2","default","1","SIP/phone_b-00000000","SIP/phone_a-00000001","Dial","SIP/phone_a","2012-06-14 20:15:09","2012-06-14 20:15:11","2012-06-14 20:15:13",4,2,"ANSWERED","DOCUMENTATION","1339704907.2","foo"

In the above example, userfield 'foo' was set on the Local channel, while phone_b-00000000 masqueraded into the Local channel.


Summary
-------

This patch preserves CDR information that was set on a Local channel before it is masqueraded into by another channel.  This includes accountcode, peeraccount, userfield, amaflags, and any variables that were set.

Consider a scenario in which a Local channel bridges two SIP channels (possibly the result of a channel originate Local/foo at default extension bar at default).  This would look something like the following:

SIP/A <--> Local;1 <> Local;2 <--> SIP/B

In this scenario, currently, the only opportunity for setting CDR information on the resulting call occurs on the Local channels - both SIP channels are created as the result of outbound calls.  As a result, it is desirable to have the CDR information that was set on the Local channel, i.e., Local;2, persist through the masquerade.  At the same time, most of the CDR information, e.g., caller ID information, channel information, etc., should be swapped with the channel being masqueraded away.

This patch does this by copying over the information in local_fixup - this is only an improvement that should be done with respect to local channels.  This should prevent impacting other masquerade operations (such as on SIP transfers).


Diffs (updated)
-----

  /branches/1.8/channels/chan_local.c 368965 

Diff: https://reviewboard.asterisk.org/r/1985/diff


Testing
-------

Tested the following scenario:

exten => 100,1,NoOp()
     same => n,Set(CDR(userfield)=foo)
     same => n,Dial(SIP/A)

exten => 101,1,NoOp()
     same => n,Set(CDR(userfield)=bar)
     same => n,Dial(SIP/B)

Performing "channel originate Local/100 at default extension 101 at default" prior to the patch would produce the following CDR record:

"","","100","default","","Local/100 at default-5160,2","SIP/A-00000000","Dial","SIP/A","2012-06-11 19:10:40","2012-06-11 19:10:42","2012-06-11 19:10:42",2,0,"ANSWERED","DOCUMENTATION","1339441840.1","foo"
"","100","101","default","100","SIP/A-00000000","SIP/B-00000001","Dial","SIP/B","2012-06-11 19:10:42","2012-06-11 19:10:43","2012-06-11 19:10:45",3,2,"ANSWERED","DOCUMENTATION","1339441840.2",""

Performing the same post patch produces the following CDR:

"","","100","default","","Local/100 at default-5160,2","SIP/A-00000000","Dial","SIP/A","2012-06-11 19:10:40","2012-06-11 19:10:42","2012-06-11 19:10:42",2,0,"ANSWERED","DOCUMENTATION","1339441840.1","foo"
"","100","101","default","100","SIP/A-00000000","SIP/B-00000001","Dial","SIP/B","2012-06-11 19:10:42","2012-06-11 19:10:43","2012-06-11 19:10:45",3,2,"ANSWERED","DOCUMENTATION","1339441840.2","bar"


Thanks,

Matt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120614/b35f0c75/attachment-0001.htm>