[asterisk-bugs] [Asterisk 0014185]: [patch] Setting registration expiry in registration string does not work

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=14185 
====================================================================== 
Reported By:                Nick_Lewis
Assigned To:                otherwiseguy
====================================================================== 
Project:                    Asterisk
Issue ID:                   14185
Category:                   Channels/chan_sip/Registration
Reproducibility:            always
Severity:                   major
Priority:                   normal
Status:                     ready for testing
Target Version:             1.6.2
Asterisk Version:           1.6.0 
Regression:                 No 
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Request Review:              
====================================================================== 
Date Submitted:             2009-01-07 05:09 CST
Last Modified:              2009-01-20 14:59 CST
====================================================================== 
Summary:                    [patch] Setting registration expiry in registration
string does not work
Description: 
In some applications it is necessary for an asterisk pbx to have very short
registrations with a voip trunk provider if for example the asterisk pbx is
behind a nat firewall and the provider does not provide other udp binding
keep alive mechanisms.

According to sip.conf the format for the registration string is
[transport://]user[:secret[:authuser]]@host[:port][/contact][~expiry]
so that it should be possible to set a shorter registration by appending a
~ and an expiry value e.g. http://bugs.digium.com/view.php?id=22#c60 for a one
minute registration

However the chan_sip.c code does not seem to make use of the [~expiry]
portion so the default expiry value is always used for all registration
strings.


====================================================================== 

---------------------------------------------------------------------- 
 (0098250) otherwiseguy (administrator) - 2009-01-20 14:59
 http://bugs.digium.com/view.php?id=14185#c98250 
---------------------------------------------------------------------- 
Actually, the documentation for 1.6.0 does not mention the ~expiry option,
only 1.6.1+ have that feature, which looks to be implemented in them.  I
only mention this because the version listed in the bug is 1.6.0.  It
doesn't look like the patch is actually against 1.6.0, though and applies
cleanly (with some fuzz) to trunk so I'll test it against that.

blitzrage lead you astray about the braces on the ifs, we require them
around single line if statements now (too many bugs introduced without
them).  It is a recent change, so forgive Leif for the confusion!  :-)

reg->expiryrequest = default_expiry;
if (expiryrequest)
    reg->expiryrequest = atoi(expiryrequest);

can be shortened to

reg->expiryquest = expiryrequest ? atoi(expiryrequest) : default_expiry;

also, if reg->expiryreqeust < 1 || < min_expiry, I would expect it to be
set to min_expiry instead of default_expiry.  If it is > max_expiry, I
would set it to max_expiry.  I would also use else if to chain those
together since you can't have any of the conditions make any since at the
same time (<1, < min_expiry, > max_expiry).

Other than that, it looks good to me.  I haven't tested it yet, but it
looks right. 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2009-01-20 14:59 otherwiseguy   Note Added: 0098250                          
======================================================================