[asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE

[wdoekes]

> On March 21, 2014, 6:01 p.m., Corey Farrell wrote:
> > /trunk/channels/sip/reqresp_parser.c, line 130
> > <https://reviewboard.asterisk.org/r/3349/diff/7-8/?file=56285#file56285line130>
> >
> >     This needs to blank both variables:
> >     userinfo = uri = "";
> 
> Geert Van Pamel wrote:
>     We return the local number anyway when an incoming RFC 3966 TEL URI INVITE call 
>     does not contain a global number nor a phone-context.
> 
> Corey Farrell wrote:
>     First sentence of 3rd paragraph of section 5.1.5:
>     Local numbers MUST have a 'phone-context' parameter that identifies the scope of their validity.
>     
>     Note the word "MUST", this has specific meaning in RFC's.  I will not approve this review if it's going to contradict the RFC it's claiming to implement.
> 
> Olle E Johansson wrote:
>     You have to be strict in what you send, but open for receiving stuff that doesn't always follow the RFC. We can add an option that sets strictness. I haven't seen many implementations of Tel: uri's sadly, but many of the few did not follow the RFC.
>     
>
> 
> Corey Farrell wrote:
>     If that is the case then should we not return error = -1?  As for optional strictness maybe use sip_settings.pedanticsipchecking?
> 
> Geert Van Pamel wrote:
>     I do return both the local number, and throw the error -1:
>     
>     	ast_debug(1, "No RFC 3966 global number or context found in '%s'; returning local number anyway\n", uri);
>             userinfo = uri;		/* Return local number anyway */
>     	error = -1;
>     
>     This would take care of both alerting the non RFC-compliance, and allowing some openness for receiving stuff that doesn't follow strictly the RFC...
> 
> Olle E Johansson wrote:
>     No, please do not add more to the pedantic variable.

Indeed, don't touch the pedantic var, and don't create new variables.

Being generous in what you accept is what we do in the rest of chan_sip anyway.


- wdoekes


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


On March 22, 2014, 1:08 p.m., Geert Van Pamel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3349/
> -----------------------------------------------------------
> 
> (Updated March 22, 2014, 1:08 p.m.)
> 
> 
> Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt Jordan, and wdoekes.
> 
> 
> Bugs: ASTERISK-17179
>     https://issues.asterisk.org/jira/browse/ASTERISK-17179
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Implements RFC-3966 TEL URI incoming INVITE.
> 
> See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description of the original isssue.
> 
> I have been patching all versions since Asterisk 1.6. I would like to include the code into the main trunk for version 13.
> 
> Previously Asterisk was failing with error on incoming IMS call:
> 
> Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address missing 'sip:', using it anyway
> 
> Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a SIP header (tel:0987654321;phone-context=+32987654321)?
> 
> Reason: tel: protocol was not recognized.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/sip/reqresp_parser.c 410429 
>   /trunk/channels/chan_sip.c 410429 
> 
> Diff: https://reviewboard.asterisk.org/r/3349/diff/
> 
> 
> Testing
> -------
> 
> Executed an incoming TEL URI INVITE connection.
> CLI was present on the display and in the CDR file.
> No errors on SIP debug output.
> 
> 
> File Attachments
> ----------------
> 
> RFC-3966 tel URI patch
>   https://reviewboard.asterisk.org/media/uploaded/files/2014/03/13/cad7a996-88c1-47fe-a2a9-cc6987af3b75__rfc-3966-tel-uri-patch-diff.txt
> 
> 
> Thanks,
> 
> Geert Van Pamel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140323/64b402db/attachment.html>