Net::SIP::Simple->register() can return wrong 'expires' value
I think that the handling of expires value in the register() function is incorrect. The current logic in the function is this:
1) get value from Expires: header
2) examine contacts only if Expires is not defined (this is backwards, Expires should be fallback value, not priority)
3) examine *all* contacts returned in Contact: header, regardless if they are us (this is clearly wrong too, it should only evaluate the contact that matches us)
4) use the smallest expires value found among all contacts (this doesn't make sense to me)
the logic should be, imo:
1) get value from Expires: header
2) find *our* contact in Contact: header.
a) If not found, expires should be set to 0 (== we're not registered)
b) if contact found, and contains expires param, use that value
c) if contact found, and no expires param, fall back to value from Expires header
d) if contact found, quit looking (there should only be one contact to match ours, afaik)
here's a patch with uses this new logic:
~# diff -u /root/.cpan/build/Net-SIP-0.62-Ia3TBm/lib/Net/SIP/Simple.pm /usr/local/share/perl/5.10.0
+/Net/SIP/Simple.pm
--- /root/.cpan/build/Net-SIP-0.62-Ia3TBm/lib/Net/SIP/Simple.pm 2010-05-31 02:51:59.000000000 -0700
+++ /usr/local/share/perl/5.10.0/Net/SIP/Simple.pm 2011-04-12 08:59:09.000000000 -0700
@@ -323,14 +323,23 @@
my $cb = sub {
my ($self,$cb_final,$expires,$endpoint,$ctx,$errno,$code,$packet,$leg,$from) = @_;
if ( $code && $code =~m{^2\d\d} ) {
+ # prevailing logic should go like this:
+ # if we have a contact match, and ...
+ # if contact has 'expires' param, use this value
+ # else use Expires header
+ # else if response contains no contacts to match ours, our registration is expired, so return
+0
my $exp = $packet->get_header( 'Expires' );
- if ( ! defined $exp ) {
- foreach my $c ( $packet->get_header( 'contact' ) ) {
- my ($addr,$p) = sip_hdrval2parts( contact => $c );
- defined( my $e = $p->{expires} ) || next;
- $exp = $e if ! defined($exp) || $e < $exp;
+ my $match = 0;
+ foreach my $c ($packet->get_header( 'contact' )) {
+ my ($addr,$p) = sip_hdrval2parts( contact => $c );
+ if ($match = sip_uri_eq($contact,$addr) ) {
+ $exp = $p->{expires} if defined $p->{expires};
+ last;
}
}
+
+ $exp = 0 unless $match;
+
$$expires = $exp;
invoke_callback( $cb_final, 'OK', expires => $exp );
