Download & Extend
SMS Framework » Issues

Call the sms_validate_number function when sending a message from the sms_send_form

Posted by jpmckinney on August 21, 2009 at 5:42pm
Project:SMS Framework
Version:6.x-2.x-dev
Component:Core Framework
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work

Issue Summary

sms_send_form_validate() checks if the number passes validation using sms_formatter():

<?php
function sms_send_form_validate($form, &$form_state) {
  if (!sms_formatter($form_state['values']['number'])) {
    form_set_error('number', t('You must enter a valid phone number.'));
  }
}
?>

Curiously, sms_validate_number() does not perform this check:

<?php
  if (!strlen($number)) {
    return t('The phone number is invalid.');
  }

  // Allow the active gateway to provide number validation
  $gateway = sms_default_gateway();
  if (function_exists($gateway['validate number']) && $error = $gateway['validate number']($number, $options)) {
    return $error;
  }
?>

I propose it should, as that would make a lot of sense:

<?php
  if (!strlen($number) || sms_formatter($number) === FALSE) {
    return t('The phone number is invalid.');
  }

  // Allow the active gateway to provide number validation
  $gateway = sms_default_gateway();
  if (function_exists($gateway['validate number']) && $error = $gateway['validate number']($number, $options)) {
    return $error;
  }
?>
Login or register to post comments

Comments

#1

Posted by jpmckinney on August 21, 2009 at 6:18pm
Status:active» needs review

#2

Posted by univate on January 10, 2011 at 8:30am
Status:needs review» fixed

The sms_formatter function is not a validation check but a function to converts various sms formats into a common format for use in this module.

What we should be doing calling the sms_validate_number function here:

<?php
diff --git sms.module sms.module
index 57598a2..3a8329b 100644
--- sms.module
+++ sms.module
@@ -383,7 +383,7 @@ function sms_send_form($required = FALSE) {
  * @see sms_send_form_submit().
  */
function sms_send_form_validate($form, &$form_state) {
-  if (!sms_formatter($form_state['values']['number'])) {
+  if (!sms_validate_number(sms_formatter($form_state['values']['number']), $form_state['values']['gateway'])) {
     form_set_error('number', t('You must enter a valid phone number.'));
   }
}
?>

#3

Posted by univate on January 10, 2011 at 8:32am
Title:Validation allows numbers that are considered invalid by other parts of the code to be submitted» Call the sms_validate_number function when sending a message from the sms_send_form

#4

Posted by rjbrown99 on January 22, 2011 at 7:17pm

I'm testing this with the email gateway and the send_form_validate is failing, even when sms_validate_number does not return an error. In this case, I get to sms_validate_number and it makes it through both checks successfully. Nothing is returned in that case since it wasn't an error.

Reverting the change from post #2 fixes this behavior. I'm not going to troubleshoot further, but I advise either backing out the change from #2 or enhacing the logic.

#5

Posted by univate on January 23, 2011 at 5:31am
Status:fixed» needs work

#6

Posted by NROTC_Webmaster on July 1, 2011 at 12:45am

I was having the same problem when trying to send a test message and what it seemed like to me was the actual validation called

sms.module line 604

<?php
function sms_validate_number(&$number, $options = array()) {
  if (!strlen($number)) {
    return t('The phone number is invalid.');
  }
?>

What worked for me was to check and see if it had 9 or 10 digits instead

<?php
function sms_validate_number(&$number, $options = array()) {
  if (strlen($number) == 9 || strlen($number) == 10) {
    return t('The phone number is invalid.');
  }
?>

obviously this is a very simple test but I think it is a better test than what was there