[Devel] Patch: cpl-c bug fix for nat flag in branches

[Bogdan-Andrei Iancu]

Hi John,

I did some reviewing of the patch.

first of all, there was no bug about flag processing. The variable was 
keeping the flag index and the function setflag() takes an index; for 
append_branch() the mask was computed.
but I agree that keeping the mask is a much better approach.

secondly, yes, there was a bug in pushing the NAT flag into messages 
when only the branches were nated.

I will apply your patch a slightly modified version. Thanks again for it!

regards,
bogdan

Bogdan-Andrei Iancu wrote:

> Hi John,
>
> at a first look it might be correct what you are saying....I will take 
> a closer look in the next days (right now I'm deep involved with the 
> OpenSER Summit) and if ok, I will commit it on CVS.
>
> regards,
> bogdan
>
> John Riordan wrote:
>
>> Hi,
>>
>>
>> Issue:
>>
>> In CPL, during a proxy to a location set the
>> value of the branch flag being passed to append_branch
>> is currently the configured value for "nat_flag" (ie 6).
>> However, append_branch is expecting a bit mapped flag value (ie 0x40).
>>
>> Also in CPL, the nat flag is getting set for the msg
>> if any branches have the nat flag set. This can cause
>> the RURI branch to have the nat flag set when the
>> location associated with the RURI branch does not
>> have the branch flag set.
>>
>>
>> Patch:
>>
>> Addressed the two issues above.
>>
>> Also, to be consistant with the default used in the
>> registrar module, made the default for nat flag -1
>> in the cpl-c module.
>>
>>
>> John
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: sip-server/modules/cpl-c/README
>> ===================================================================
>> RCS file: /cvsroot/openser/sip-server/modules/cpl-c/README,v
>> retrieving revision 1.5
>> diff -u -r1.5 README
>> --- sip-server/modules/cpl-c/README    9 Feb 2006 17:28:25 -0000    1.5
>> +++ sip-server/modules/cpl-c/README    29 Oct 2006 05:28:27 -0000
>> @@ -304,11 +304,11 @@
>>    tag when retrieving a contact behind a NAT (this flag will be
>>    set).
>>
>> -   Default value of this parameter is 6. +   Default value is -1 
>> (disabled).
>>
>>    Example 1-14. Set nat_flag parameter
>> ...
>> -modparam("cpl_c","nat_flag",4)
>> +modparam("cpl_c","nat_flag",6)
>> ...
>>      _________________________________________________________
>>
>> Index: sip-server/modules/cpl-c/cpl.c
>> ===================================================================
>> RCS file: /cvsroot/openser/sip-server/modules/cpl-c/cpl.c,v
>> retrieving revision 1.13
>> diff -u -r1.13 cpl.c
>> --- sip-server/modules/cpl-c/cpl.c    10 Oct 2006 18:45:56 -0000    1.13
>> +++ sip-server/modules/cpl-c/cpl.c    29 Oct 2006 05:28:27 -0000
>> @@ -83,7 +83,7 @@
>>         0, /* no cpl logging */
>>         0, /* recurse proxy level is 0 */
>>         0, /* no script route to be run before proxy */
>> -        6, /* nat flag */
>> +        -1, /* nat flag */
>>         0, /* user part is not case sensitive */
>>         {0,0},   /* no domain prefix to be ignored */
>>         {-1,-1}, /* communication pipe to aux_process */
>> @@ -404,6 +404,9 @@
>>         strlower( &cpl_env.realm_prefix );
>>     }
>>
>> +    /* fix the flags */
>> +    cpl_env.nat_flag = (cpl_env.nat_flag!=-1)?(1<<cpl_env.nat_flag):0;
>> +
>>     return 0;
>> error:
>>     return -1;
>> Index: sip-server/modules/cpl-c/cpl_sig.c
>> ===================================================================
>> RCS file: /cvsroot/openser/sip-server/modules/cpl-c/cpl_sig.c,v
>> retrieving revision 1.7
>> diff -u -r1.7 cpl_sig.c
>> --- sip-server/modules/cpl-c/cpl_sig.c    6 Apr 2006 18:21:56 
>> -0000    1.7
>> +++ sip-server/modules/cpl-c/cpl_sig.c    29 Oct 2006 05:28:27 -0000
>> @@ -78,10 +78,9 @@
>>                 goto error;
>>             }
>>         }
>> -        /* is the location NATED? */
>> +        /* for RURI branch, the nat flag goes into msg */
>>         if ((*locs)->flags&CPL_LOC_NATED)
>> -            /* for RURI branch, the nat flag goes into msg */
>> -            setflag(msg,cpl_env.nat_flag);
>> +            msg->flags |= cpl_env.nat_flag;
>>         /* free the location and point to the next one */
>>         foo = (*locs)->next;
>>         free_location( *locs );
>> @@ -100,8 +99,6 @@
>>                 "appending branch <%s>\n",(*locs)->addr.uri.s);
>>             goto error;
>>         }
>> -        /* is the location NATED? */
>> -        if (bflags) setflag(msg,bflags);
>>         /* free the location and point to the next one */
>>         foo = (*locs)->next;
>>         free_location( *locs );
>> Index: sip-server/modules/cpl-c/doc/cpl-c_user.sgml
>> ===================================================================
>> RCS file: 
>> /cvsroot/openser/sip-server/modules/cpl-c/doc/cpl-c_user.sgml,v
>> retrieving revision 1.6
>> diff -u -r1.6 cpl-c_user.sgml
>> --- sip-server/modules/cpl-c/doc/cpl-c_user.sgml    9 Feb 2006 
>> 17:28:25 -0000    1.6
>> +++ sip-server/modules/cpl-c/doc/cpl-c_user.sgml    29 Oct 2006 
>> 05:28:27 -0000
>> @@ -361,14 +361,14 @@
>>             </para>
>>             <para>
>>                 <emphasis>
>> -                    Default value of this parameter is 6.
>> +                    Default value is -1 (disabled).
>>                 </emphasis>
>>             </para>
>>             <example>
>>                 <title>Set <varname>nat_flag</varname> parameter</title>
>>                 <programlisting format="linespecific">
>> ...
>> -modparam("cpl_c","nat_flag",4)
>> +modparam("cpl_c","nat_flag",6)
>> ...
>> </programlisting>
>>             </example>
>>
>>  
>