[asterisk-dev] [Code Review] Fix RURI generation in chan_sip so that URIs are correct when SRV records are involved or when port 5060 is specified

[Matthew Nicholson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/369/
-----------------------------------------------------------

Review request for Asterisk Developers and Olle E Johansson.


Summary
-------

This patch ensures that RURIs generated by asterisk are correct when a port is specified and in the presence of an SRV lookup.  Without this patch, asterisk will include a port in an RUI when port != 5060 and will not include a port if port == 5060.  This behavior is not correct.  According to RFC 3261, the URIs "host.tld" and "host.tld:5060" are not the same.  Additionally the URI "host.tld" indicates that an SRV lookup should be performed (which asterisk handles properly) BUT the SRV lookup should NOT change the RURI.

Note: This patch does not address asterisk generation REGISTER requests, which may also have RURI problems.


This addresses bug 14418.
    https://issues.asterisk.org/view.php?id=14418


Diffs
-----

  /branches/1.4/channels/chan_sip.c 219135 

Diff: https://reviewboard.asterisk.org/r/369/diff


Testing
-------

I tested the following cases where RURIs are generated.  In all cases the proper behavior was observed.

* port specified in dialplan as follows: Dial(SIP/user at host:port).  Testing was done with port = 5060, 5061 and with an SRV lookup that altered the host and port.

* port provided by SIP client during registration

* port provided in sip.conf peer definition.  In this case the RURI is generated properly, but SRV lookups are done regardless if a port is specified in the peer definition or not.  This is not correct (SRV lookups should only occur in the absence of a port specification).  I consider that an unrelated issue.

* port specified in dialplan and specified in sip.conf or during peer registration: Dial(SIP/123 at peer:1234).  This works properly with the same caveats as the previous case.


Thanks,

Matthew