Re: [asterisk-dev] [Code Review] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 9, 2015, 11:12 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 432638 Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'endpoint_identifier_order'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432482 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432482 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432482 branches/13/res/res_pjsip/config_global.c 432482 branches/13/res/res_pjsip.c 432482 branches/13/include/asterisk/res_pjsip.h 432482 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_endpoint_identifier_order.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432482 branches/13/CHANGES 432482 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/#review14606 --- Ship it! Ship It! - Mark Michelson On March 5, 2015, 11:40 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 5, 2015, 11:40 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'endpoint_identifier_order'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432482 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432482 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432482 branches/13/res/res_pjsip/config_global.c 432482 branches/13/res/res_pjsip.c 432482 branches/13/include/asterisk/res_pjsip.h 432482 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_endpoint_identifier_order.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432482 branches/13/CHANGES 432482 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 5, 2015, 5:40 p.m.) Review request for Asterisk Developers. Changes --- Addressed various feedback. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description (updated) --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'endpoint_identifier_order'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs (updated) - branches/13/res/res_pjsip_endpoint_identifier_user.c 432482 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432482 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432482 branches/13/res/res_pjsip/config_global.c 432482 branches/13/res/res_pjsip.c 432482 branches/13/include/asterisk/res_pjsip.h 432482 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_endpoint_identifier_order.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432482 branches/13/CHANGES 432482 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/#review14574 --- branches/13/CHANGES https://reviewboard.asterisk.org/r/4455/#comment25123 Pedantic: Not a fan of identify_by_priority. How's about endpoint_identifier_order? branches/13/res/res_pjsip/config_global.c https://reviewboard.asterisk.org/r/4455/#comment25124 This isn't safe. The global config may get reloaded, causing this pointer to point to invalid memory. The return value needs to be strduped. - Joshua Colp On March 2, 2015, 8:04 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 2, 2015, 8:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'identify_by_priority'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 branches/13/res/res_pjsip/config_global.c 432422 branches/13/res/res_pjsip.c 432422 branches/13/include/asterisk/res_pjsip.h 432422 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432422 branches/13/CHANGES 432422 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/#review14578 --- branches/13/res/res_pjsip/config_global.c https://reviewboard.asterisk.org/r/4455/#comment25127 I think the default order should be ip,username,anonymous. Matching on IP address should supersede matching on a username since an IP match is more likely to be desired than a username match if an IP match has been set up for an endpoint. - Mark Michelson On March 2, 2015, 8:04 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 2, 2015, 8:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'identify_by_priority'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 branches/13/res/res_pjsip/config_global.c 432422 branches/13/res/res_pjsip.c 432422 branches/13/include/asterisk/res_pjsip.h 432422 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432422 branches/13/CHANGES 432422 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
On March 3, 2015, 12:43 p.m., Joshua Colp wrote: branches/13/CHANGES, line 31 https://reviewboard.asterisk.org/r/4455/diff/1/?file=71719#file71719line31 Pedantic: Not a fan of identify_by_priority. How's about endpoint_identifier_order? heh the last thing I did before posting this review was change the option name from 'identify_by_order' to 'identify_by_priority'. However, I think I like yours even better. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/#review14574 --- On March 2, 2015, 2:04 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 2, 2015, 2:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'identify_by_priority'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 branches/13/res/res_pjsip/config_global.c 432422 branches/13/res/res_pjsip.c 432422 branches/13/include/asterisk/res_pjsip.h 432422 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432422 branches/13/CHANGES 432422 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/#review14571 --- I think a cli command that lists the currently registered identifiers might be needed. Otherwise how would you know, especially if the name of the module didn't start with res_pjsip_endpoint_identifier_*, or if you're the admin but not the compiler/installer and don't know what modules are available. pjsip list identifiers Name. Module anonymous res_pjsip_endpoint_identifier_anonymous ip res_pjsip_endpoint_identifier_ip usernameres_pjsip_endpoint_identifier_user - George Joseph On March 2, 2015, 1:04 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 2, 2015, 1:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'identify_by_priority'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 branches/13/res/res_pjsip/config_global.c 432422 branches/13/res/res_pjsip.c 432422 branches/13/include/asterisk/res_pjsip.h 432422 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432422 branches/13/CHANGES 432422 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/#review14570 --- branches/13/configs/samples/pjsip.conf.sample https://reviewboard.asterisk.org/r/4455/#comment25120 I think you need to specify how the identifiers are derived. I.E. from the res_pjsip_endpoint_identifier_* modules. branches/13/res/res_pjsip.c https://reviewboard.asterisk.org/r/4455/#comment25121 I'd add a comment here as well as to where to look for the names. - George Joseph On March 2, 2015, 1:04 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- (Updated March 2, 2015, 1:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'identify_by_priority'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 branches/13/res/res_pjsip/config_global.c 432422 branches/13/res/res_pjsip.c 432422 branches/13/include/asterisk/res_pjsip.h 432422 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432422 branches/13/CHANGES 432422 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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] 4455: res_pjsip: conflicting endpoint identifiers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4455/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24840 https://issues.asterisk.org/jira/browse/ASTERISK-24840 Repository: Asterisk Description --- It's possible to have a scenario that will create a conflict between endpoint identifiers. For instance an incoming call could be identified by two different endpoint identifiers and the one chosen depended upon which identifier module loaded first. This of course causes problems when, for example, the incoming call is expected to be identified by username, but instead is identified by ip. This patch adds a new 'global' option to res_pjsip called 'identify_by_priority'. It is a comma separated list of endpoint identifier names that specifies the order by which identifiers are processed and checked. Diffs - branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 branches/13/res/res_pjsip/config_global.c 432422 branches/13/res/res_pjsip.c 432422 branches/13/include/asterisk/res_pjsip.h 432422 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py PRE-CREATION branches/13/configs/samples/pjsip.conf.sample 432422 branches/13/CHANGES 432422 Diff: https://reviewboard.asterisk.org/r/4455/diff/ Testing --- Added a testsuite test: https://reviewboard.asterisk.org/r/4456/ Thanks, Kevin Harwell -- _ -- 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
