[asterisk-dev] [Code Review] 3817: res_pjsip_notify: Add the ability for PJSIPNotify AMI command and pjsip send notify CLI command to send to a URI instead of an endpoint

[Mark Michelson]

> On July 18, 2014, 3:40 p.m., Mark Michelson wrote:
> > /trunk/include/asterisk/res_pjsip.h, lines 1195-1203
> > <https://reviewboard.asterisk.org/r/3817/diff/2/?file=64713#file64713line1195>
> >
> >     This function isn't necessary. When PJSIP is passed a URI string, PJSIP will perform URI validation for us and return an error if a badly-formed URI is passed in.
> 
> Jonathan Rose wrote:
>     I was absolutely getting crashes when using URIs that didn't start with 'sip:' or else were only 'sip:' without anything appended to them.  I'm not sure if that will keep being the case if we use the default endpoint like suggested below, but if it does then I might need to keep this around.
> 
> Jonathan Rose wrote:
>     After testing it with the default outbound endpoint, it was still able to cause crashes. Rats.

Where does the crash occur? Is it in Asterisk's lower-level res_pjsip code, or is it in PJSIP itself? If the crash is in Asterisk, it sounds like there are some assumptions being made in the lower levels that shouldn't be made and those should be fixed there. If the crash is in PJSIP (especially if it's an assertion failure), then that means we do have to do some validation before calling down into PJSIP, but that validation should be left to the core res_pjsip code rather than being the duty of users of the res_pjsip API.

If we do need to perform validation of the URI, the implementation has some issues. It's limiting since it only allows for sip: URIs and not, for instance, sips: URIs. It also only checks that the prefix is valid for a sip: URI. It does no other checking of the rest of the URI. A more thorough way of checking validity of the URI is to use pjsip_parse_uri() from PJSIP. If you then want to make sure that the URI is a sip: or sips: URI, you can use the PJSIP_URI_SCHEME_IS_SIP() and PJSIP_URI_SCHEME_IS_SIPS() macros to do so.


- Mark


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


On July 18, 2014, 6:32 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3817/
> -----------------------------------------------------------
> 
> (Updated July 18, 2014, 6:32 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Example:
> 
> Action: PJSIPNotify
> URI: sip:10.10.10.10
> 
> pjsip send notify uri <type> uri <uri> [uri...]
> 
> 
> Adds the ability to use URIs with the PJSIP notify commands instead of endpoints. These mostly work the same as the endpoint notifications.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_pjsip_notify.c 418868 
>   /trunk/res/res_pjsip.c 418868 
>   /trunk/include/asterisk/res_pjsip.h 418868 
> 
> Diff: https://reviewboard.asterisk.org/r/3817/diff/
> 
> 
> Testing
> -------
> 
> Used existing endpoint notification and compared it to similar notifies done against URIs.  The outgoing messages looked the same as far as the details I was concerned with went.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140721/402a0a62/attachment.html>