Re: [asterisk-dev] [Code Review] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- (Updated March 17, 2014, 8:46 a.m.) Status -- This change has been discarded. Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
On Feb. 23, 2014, 9:04 p.m., Corey Farrell wrote: /trunk/channels/sip/reqresp_parser.c, lines 103-118 https://reviewboard.asterisk.org/r/3250/diff/1/?file=54390#file54390line103 I feel this section should only apply when scheme is 'tel:'. I'm concerned with changes to how sip URI's are handled. For example: sip:example.com;phone-context=spoof.domain.com sip:+example.com The first URI should result in hostport=example.com, userinfo=. This change causes it to be hostport=spoof.domain.com, userinfo=example.com. The second URI should result in the invalid hostport +example.com, where this puts the value in userinfo. What happens to invalid tel: URI's? For example tel:1 - no phone-context or + would cause 1 to be used as hostport (like in SIP uri). I'd like to see test cases added to sip_parse_uri_full_test and/or sip_parse_uri_test. At minimum the tests need to verify no change in results for URI scheme sip. wdoekes wrote: Thanks for the speedy response :) Let me forward your concerns. I have uploaded a new patch for TEL URI reqresp_parser.c to https://issues.asterisk.org/jira/browse/ASTERISK-17179 see asterisk-12.0.0-reqresp_parser-RFC3966_patch.txt. Taking into account the remarks from Corey Farrell. Sorry, but I do not know (yet) how to use the Review board... If there would be an invalid TEL URI (e.g. no phone-context, nor +global-number) then an error indicating such failure is issued. - Geert --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/#review10929 --- On Feb. 23, 2014, 12:17 p.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- (Updated Feb. 23, 2014, 12:17 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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
[asterisk-dev] [Code Review] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/#review10929 --- /trunk/channels/sip/reqresp_parser.c https://reviewboard.asterisk.org/r/3250/#comment20556 I feel this section should only apply when scheme is 'tel:'. I'm concerned with changes to how sip URI's are handled. For example: sip:example.com;phone-context=spoof.domain.com sip:+example.com The first URI should result in hostport=example.com, userinfo=. This change causes it to be hostport=spoof.domain.com, userinfo=example.com. The second URI should result in the invalid hostport +example.com, where this puts the value in userinfo. What happens to invalid tel: URI's? For example tel:1 - no phone-context or + would cause 1 to be used as hostport (like in SIP uri). I'd like to see test cases added to sip_parse_uri_full_test and/or sip_parse_uri_test. At minimum the tests need to verify no change in results for URI scheme sip. - Corey Farrell On Feb. 23, 2014, 6:17 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- (Updated Feb. 23, 2014, 6:17 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
On Feb. 23, 2014, 8:04 p.m., Corey Farrell wrote: /trunk/channels/sip/reqresp_parser.c, lines 103-118 https://reviewboard.asterisk.org/r/3250/diff/1/?file=54390#file54390line103 I feel this section should only apply when scheme is 'tel:'. I'm concerned with changes to how sip URI's are handled. For example: sip:example.com;phone-context=spoof.domain.com sip:+example.com The first URI should result in hostport=example.com, userinfo=. This change causes it to be hostport=spoof.domain.com, userinfo=example.com. The second URI should result in the invalid hostport +example.com, where this puts the value in userinfo. What happens to invalid tel: URI's? For example tel:1 - no phone-context or + would cause 1 to be used as hostport (like in SIP uri). I'd like to see test cases added to sip_parse_uri_full_test and/or sip_parse_uri_test. At minimum the tests need to verify no change in results for URI scheme sip. Thanks for the speedy response :) Let me forward your concerns. - wdoekes --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/#review10929 --- On Feb. 23, 2014, 11:17 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- (Updated Feb. 23, 2014, 11:17 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/#review10931 --- /trunk/channels/sip/reqresp_parser.c https://reviewboard.asterisk.org/r/3250/#comment20558 Please add the appropriate unit tests here too. - Paul Belanger On Feb. 23, 2014, 11:17 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- (Updated Feb. 23, 2014, 11:17 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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] 3250: chan_sip: Add incoming tel: uri support (rfc3966)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/#review10932 --- I've gone ahead and rectified the access issue. If Geert would like, he can open a new review that resolves these findings. - Matt Jordan On Feb. 23, 2014, 5:17 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3250/ --- (Updated Feb. 23, 2014, 5:17 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is filed on behalf of Geert Van Pamel as filed against Asterisk-12 on ASTERISK-17179. It was cleaned up by me. The patch should allow incoming INVITEs with a tel: uri. An IMS server apparently uses it. Geert would appreciate it if this was looked at and checked in, so he won't have to patch Asterisk 13. He has been patching this since Asterisk 1.6.2.x. Diffs - /trunk/channels/sip/reqresp_parser.c 408868 /trunk/channels/chan_sip.c 408868 Diff: https://reviewboard.asterisk.org/r/3250/diff/ Testing --- Not by me. It compiles. I'm just filing it because Geert doesn't have an account and I understand his frustration. Thanks, wdoekes -- _ -- 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
