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

2014-03-22 Thread Geert Van Pamel

---
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.


Changes
---

When an incoming RFC 3966 TEL URI INVITE call does not contain a global number 
nor a phone-context, we still return the local number, but we issue a warning 
message.

This could incur that local calls could be possible, but outgoing return calls 
might fail because there might be no route to the (original) caller.


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 (updated)
-

  /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

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-03-22 Thread Olle E Johansson

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


I don't see what happens with the phone-context argument. Shouldn't we pass 
that on as a channel variable? 

- Olle E Johansson


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
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-03-22 Thread 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.

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.


- 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
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-03-22 Thread Corey Farrell


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


If that is the case then should we not return error = -1?  As for optional 
strictness maybe use sip_settings.pedanticsipchecking?


- Corey


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


On March 22, 2014, 9:08 a.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, 9:08 a.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
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-03-22 Thread Geert Van Pamel


 On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
  I don't see what happens with the phone-context argument. Shouldn't we pass 
  that on as a channel variable?

We return this into the hostport.


- Geert


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


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
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-03-22 Thread Geert Van Pamel


 On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
  I don't see what happens with the phone-context argument. Shouldn't we pass 
  that on as a channel variable?
 
 Geert Van Pamel wrote:
 We return this into the hostport.

According to RFC 3966 phone-context is either a domain-name, or (part of) an 
international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the local 
number must be unique within its context...


- Geert


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


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
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-03-22 Thread Geert Van Pamel


 On March 15, 2014, 7:04 p.m., Corey Farrell wrote:
  /trunk/channels/sip/reqresp_parser.c, line 111
  https://reviewboard.asterisk.org/r/3349/diff/3/?file=56070#file56070line111
 
  scheme is the input parameter listing acceptable schemes, we don't need 
  to see it here.  The other ast_debug included scheme since the problem was 
  a failure to match the uri with any scheme.

I fixed this


- Geert


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


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
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev