Skip to content
This repository has been archived by the owner on Mar 4, 2021. It is now read-only.

Rudimentary SIP support #243

Closed
wants to merge 3 commits into from
Closed

Rudimentary SIP support #243

wants to merge 3 commits into from

Conversation

ryanpetris
Copy link

I needed/wanted this for my own OpenVBX installation so I added some very rudimentary support for SIP urls in both the devices page and in the dial applet. If anything you could use this as a base for adding proper SIP support.

@nsteffan
Copy link

+1

@ghost
Copy link

ghost commented Jan 29, 2014

+1 Great work Ryan.

@Gipetto
Copy link
Contributor

Gipetto commented Feb 2, 2014

@ryanpetris I'm hoping to dive deeply in to this soon, but off the cuff I see that there's a lot of rudimentary sip address checking. The develop branch of OpenVBX, where all the development work happens before moving to master, has a helper method for validating sip addresses.

https://code.hq.twilio.com/shawn/OpenVBX/blob/develop/OpenVBX/helpers/format_helper.php

Can you cherry-pick this work from your master in to your develop branch instead and resubmit the pull request against develop?

You can read a bit more about the repo layout here: https://github.com/twilio/OpenVBX/wiki as well as get a link in to the git-flow methodology. It makes pull requests from forks a bit tedious but the workflow itself is solid.

@@ -101,9 +101,12 @@ public function make_call($from, $to, $callerid, $rest_access)
{
try
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire try {} catch() {} block is old, rickety, and needs to be reworked. PhoneNumber::validatePhoneNumber() will throw a validation exception that we catch and re-raise as a VBX_CallException(). This should be smarter about the different formats that are passed in and throw context appropriate exceptions.

I can work on this after merging the request, but if you want to take a whack at it my preliminary thought is that it should be something like:

try {

    foreach (array('from' => $from, 'to' => $to) as $type => $value) 
    {
        // to
        if (PhoneNumber::analyzePhoneNumber($value) != PhoneNumber::TYPE_UNKNOWN) 
        {
            PhoneNumber::validatePhoneNumber($value);
        } 
        elseif (strpos($value, 'sip') !== false && !validate_sip_address($value)) 
        {
            throw new Exception(sprintf('%s phone number %s is not a valid sip address', $type, $value));
        } 
        elseif (strpos($value, '@') !== false && !filter_var($value, FILTER_VALIDATE_EMAIL)) 
        {
            throw new Exception(sprintf('%s phone number %s is not a valid email address', $type, $value));
        }
        else 
        {
            throw new Exception(sprintf('Could not validate %s phone number: %s' $type, $value));
        }
    }

} catch (Exception $e) {
    throw new VBX_CallException($e->getMessage);

}

Its a lot more strict and more informative on malformed data.

@Gipetto
Copy link
Contributor

Gipetto commented Feb 3, 2014

Looking back at my earlier comment about the number of times we use strpos($var, 'sip') to check the sip address: after reading the entirety of the pull request and reacquainting myself with the codebase a bit I wouldn't worry about that so much. Sheesh... I really should have written some quick and easy helpers like is_phone_number() is_client_address() as we now need is_sip_address().

@ryanpetris ryanpetris closed this Sep 1, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants