[asterisk-bugs] [JIRA] (ASTERISK-23679) CDR userfield merged incorrectly in ast_bridge_call

[Matt Jordan (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-23679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=217696#comment-217696 ] 

Matt Jordan commented on ASTERISK-23679:
----------------------------------------

I can't say that the behaviour in this patch isn't better than what exists, but there is a slight problem with it.

Technically, if I had a channel that was previously in a bridge with another channel, this patch blows away the userfield that was previously constructed. For example, same SIP/foo (with userfield foo) is bridged with SIP/bar (with userfield bar). I would expect the userfield on bar to be "foo;bar". If SIP/foo 'survives' the bridge (due to some dial flag), and enters into a bridge with SIP/yackity, the userfield would be "foo;bar;yackity". With this patch, however, it becomes "foo;yackity".

Now, the current code is still wrong - because as you pointed out, it actually concatenates the userfield portion of the first channel twice. Ew. But that doesn't mean getting rid of the concatenation is correct.

Different + CDRs is a dangerous thing.

This is one of those behaviours that is just "this is always how it's been" and was never really written down. So there's no canonical definition of the behaviour of userfield, The closest we've come is in [Asterisk 12|https://wiki.asterisk.org/wiki/display/AST/Asterisk+12+CDR+Specification], where we documented that the behaviour of the userfield is:

{quote}
A user defined field set on the channels. If set on both the Party A and Party B channel, the userfields of both are concatenated and separated by a ;
{quote}

This is now done in {{cdr.c}}, and will always concatenate just the two fields of the Party A and Party B channels. Because these fields are not on the channel themselves, there is no cumulative effect:

{noformat}
			if (!ast_strlen_zero(it_cdr->party_b.userfield)) {
				snprintf(cdr_copy->userfield, sizeof(cdr_copy->userfield), "%s;%s", it_cdr->party_a.userfield, it_cdr->party_b.userfield);
			}
{noformat}

*tl;dr*: There's a bug here, but I'm not sure the patch is the safest way to fix it. If you went with appending the userfield but corrected the duplication of the first channel, that would be 'safe', in that the simplest case would be fixed and anyone depending on weird continuation of concatenation wouldn't be broken.

> CDR userfield merged incorrectly in ast_bridge_call 
> ----------------------------------------------------
>
>                 Key: ASTERISK-23679
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-23679
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Features
>    Affects Versions: 11.9.0
>            Reporter: Vitezslav Novy
>            Severity: Minor
>         Attachments: 11-cdr_userfield.patch
>
>
> In function ast_bridge_call (in main/features.c) CDR field of B leg is appended to  CDR userfield of A leg.
> when at start userfield of leg A is 'aaa' and userfield of leg B is 'bbb' result in leg A userfield should be 'aaa;bbb'.
> Because of incorrect merge result is 'aaaaaa;bbb' 



--
This message was sent by Atlassian JIRA
(v6.2#6252)