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

Geert Van Pamel reviewboard at asterisk.org
Thu Mar 20 17:06:22 CDT 2014



> On March 20, 2014, 7:37 a.m., Corey Farrell wrote:
> > /trunk/channels/sip/reqresp_parser.c, line 525
> > <https://reviewboard.asterisk.org/r/3349/diff/6/?file=56222#file56222line525>
> >
> >     This name is exposed through asterisk CLI, so say don't change it (especially since this has nothing to do with your change).
> >     
> >     If you want to cleanup test names that should be a separate review, and cleanup all chan_sip test names.  I'm not sure anyone will want to bother changing the names, it is of little benefit and could possibly break someone's test script.

Rolled back to test case name "sip_uri_parse_test".
Although it looks strange to me because the function is named sip_parse_uri_test?


> On March 20, 2014, 7:37 a.m., Corey Farrell wrote:
> > /trunk/channels/sip/reqresp_parser.c, line 653
> > <https://reviewboard.asterisk.org/r/3349/diff/6/?file=56222#file56222line653>
> >
> >     Why change this from !ast_strlen_zero(name)?

Rolled back to my previous change.


> On March 20, 2014, 7:37 a.m., Corey Farrell wrote:
> > /trunk/channels/sip/reqresp_parser.c, lines 411-419
> > <https://reviewboard.asterisk.org/r/3349/diff/6/?file=56222#file56222line411>
> >
> >     What happened to residue and params.transport?  We really should test expected results for parameters after phone-context.  This would ensure the code does what is expected now, and that future changes do not cause unexpected changes.

Added a 3rd (complex) TEL URI invite with parameters and headers.
Documented clearly that currently the ext= or isub= parameters remain unparsed;
but should be further parsed from userinfo when the RFC 3966 will be fully implemented (with ISDN or PSTN local extensions).


- Geert


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


On March 20, 2014, 10:59 p.m., Geert Van Pamel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3349/
> -----------------------------------------------------------
> 
> (Updated March 20, 2014, 10:59 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/20140320/eb401876/attachment.html>


More information about the asterisk-dev mailing list