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

[Olle E Johansson]

> On March 21, 2014, 7: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...

No, please do not add more to the pedantic variable. 


- Olle E


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


On March 22, 2014, 2: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, 2: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/4cb9db0f/attachment.html>