[asterisk-dev] [Code Review] chan_sip: removing some non-compliant code in v10 and minor fixes

[David Vossel]

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


I'm not sure we should make the registration change change for rejecting Registrations without a user name in the To header.  We have probably treated the domain as the user name when no user name is present since the beginning of chan_sip.  I certainly hope this logic isn't being depended on somewhere, but since we allow it, it probably is. Below is what I found on the user name in the To header URI.  Unless there is some other verbiage I missed that uses the "MUST" key word in relation to this behavior, it seems to me like this falls in the category of being liberal about what we accept and strict about what we send.  Does it cause any problems the way it is?


RFC 3261 section 10.2.
      To: The To header field contains the address of record whose
           registration is to be created, queried, or modified.  The To
           header field and the Request-URI field typically differ, as
           the former contains a user name.  This address-of-record MUST
           be a SIP URI or SIPS URI.

- David


On Oct. 19, 2011, 12:30 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1533/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2011, 12:30 p.m.)
> 
> 
> Review request for Asterisk Developers and Paul Belanger.
> 
> 
> Summary
> -------
> 
> See the bug report: there were some XXX'es in the code about code that should be removed.
> 
> The patch does this:
> 
> (1) register_verify won't accept a To: without user-part anymore (illegal according to rfc3261, 10.2)
> 
> (2) check_user_full still doesn't require a user-part, but it won't match usernames by domain anymore. (i.e. it doesn't treat sip:domain as sip:domain at domain anymore)
> 
> (3) there was some freaky logic going on in get_msg_text, I had to rewrite it to make it make sense.
> 
> (4) in the reqresp parser there were lots of if (params) inside a big if (params) block. I scrapped the useless if's.
> 
> 
> This addresses bug ASTERISK-18389.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18389
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 341249 
>   /trunk/channels/sip/reqresp_parser.c 341249 
> 
> Diff: https://reviewboard.asterisk.org/r/1533/diff
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111020/33ce73d5/attachment.htm>