[OpenSER-Devel] [ openser-Bugs-1805622 ] exec_dset should be able to return q val along URI

Fri Nov 16 16:10:19 UTC 2007

Bugs item #1805622, was opened at 2007-10-01 15:35
Message generated for change (Comment added) made by bogdan_iancu
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1805622&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: core
Group: ver devel
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Marcello Lupo (hunters_wolf)
Assigned to: Bogdan-Andrei Iancu (bogdan_iancu)
>Summary: exec_dset should be able to return q val along URI

Initial Comment:
After execution of exec_dset i get in output:

sip:123456 at 10.10.10.1:5062;q=0.2
sip:123456 at 10.10.10.10:5060;q=0.1

$ds become : Contact: sip:123456 at 10.10.10.1:5062;q=0.2, sip:123456 at 10.10.10.10:5060;q=0.1

RURI become: sip:123456 at 10.10.10.1:5062;q=0.2

$bR become: sip:123456 at 10.10.10.10:5060;q=0.1 

All seem to be fine but when i run serialize_branches() i get:

DEBUG:serialize_branches: nothing to do - all same q!

And the branches are not serialized.

I tested it with qvalue as:

q=1.0
q=0.1
q=1 

both with \n and \r\n at the end of lines and serialize_branches() give me all the time the same result.

I tested the exec_dset without the serialize_branches() and it is working correctly going in parallel forking to
the 2 gateways in parallel. So I'm sure that branches are correct.
It seems to be really a serialize_branches() bug. 

Let me know,
Thanks,
Bye,
Marcello

----------------------------------------------------------------------

>Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2007-11-16 18:10

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi,

I move this item as a Feature Request because, as previously said, there
is no standard why to return the q as part of SIP URI; So it is not a bug,
but an enhancement.

Regards,
Bogdan

----------------------------------------------------------------------

Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2007-11-13 11:19

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Marcello,

There is one issue here. "q" is not a URI parameter, so from SIP point fo
view:
   sip:123456 at 10.10.10.1:5062;q=0.2
is not a correct URI - or at least q parameter is not intrepreted as
expected.

Q is a header parameter (as for contact header). And the exec module (as
functionalitity) returns only SIP URIs...

Regards,
Bogdan

----------------------------------------------------------------------

Comment By: Marcello Lupo (hunters_wolf)
Date: 2007-10-23 12:16

Message:
Logged In: YES 
user_id=1902881
Originator: YES

Hi Bogdan,
i made  a patch to exec module to have it working as it should.

The module export 2 new parameters: 

modparam("exec", "dset_parse_uri_qvalue", 1)
modparam("exec", "dset_strip_uri_qvalue", 1)

dset_parse_uri_qvalue : default 0, if set to 1 tell the module to parse
the qvalue in the URI got from the exec_dset, pass the qvalue of first uri
retrived with set_ruri_q and pass it to append_branch from the second
ahead. It leave the uri retrived as is.

dset_strip_uri_qvalue : default 0, if set to 1 (and only if
dset_parse_uri_qvalue is set to 1) tell the module to strip the q=n
parameter from the retrived uri before put it in the RURI or
append_branch() . It strip the parameter and leave the rest unchanged even
if there are other parameters in the URI.
I'm not able to produce a diff for this patch (i'm not a coder) but i
attach to this thread the tgz with the exec src dir modified against v
1.2.2 notls . I tested the patch and seems to be working, i think it only
require some fine tuning from someone that can code better than me. I think
the best is to put the code in a separate function and to have a check on
the size of qvalue present in the URI otherwise there is the risk to write
a bigger value on the temp_qvalue variable i used.
You can integrate the patch in the version as you like.
Hope that this helped.
Bye,
Marcello

File Added: new_exec_module.tgz

----------------------------------------------------------------------

Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2007-10-23 11:29

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi,

Thanks for report - I will take a look on this.

Regards,
Bogdan

----------------------------------------------------------------------

Comment By: Marcello Lupo (hunters_wolf)
Date: 2007-10-03 00:59

Message:
Logged In: YES 
user_id=1902881
Originator: YES

I found 2 issues on this.

1) Digging in the source code I found in the cfg.y that grammar for
append_branch is valid in this way:

append_branch("","")  and i didn't found any documentation on it. It wants
the qvalue as a separated field.

If i pass value with append_branch in this way:

append_branch("sip:123456 at 10.0.0.1:5062","0.3");
append_branch("sip:123456 at 10.0.0.2:5060","0.2");
append_branch("sip:123456 at 10.0.0.3:5060","0.1");
serialize_branches();

It works.

2) The bug matter is that in the exec_dset is not possible to have the
same behavior because it use the standard
append_branch function in this way (exec.c line 172) :

if (append_branch(msg, &uri, 0, 0, Q_UNSPECIFIED, 0, 0)==-1) {
        LOG(L_ERR, "ERROR: exec_str: append_branch failed;"
                " too many or too long URIs?\n");
        goto error02;
}

It pass the qvalue as Q_UNSPECIFIED to append_branch. This is the reason
it is not working. It is not considering the qvalue in the URI at all.
I think a solution can be to parse the lines received form exec_dset with
a check on the  line for a  q= match and take the
value, or much more simple, assume that lines should be considered
returned by exec_dset in order of q value already and let the
funtion to populate it with a FOR cycle. The second method cannot be
applied if you want to have parallel forking obviously.
I will try to patch it by myself but if someone can help me it will be
very appreciated because i'm not a C coder.

If you are interested on it please follow all the thread :
"serialize_branches seems to be not working" on openser user list.
Thanks,
Bye,
Marcello

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1805622&group_id=139143