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

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:

  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:

  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;
  }

Comments

jpmckinney’s picture

jpmckinney CreditAttribution: jpmckinney commented
Status: Active » Needs review
univate’s picture

univate CreditAttribution: univate commented
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:

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.'));
   }
 }
univate’s picture

univate CreditAttribution: univate commented
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
rjbrown99’s picture

rjbrown99 CreditAttribution: rjbrown99 commented

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.

univate’s picture

univate CreditAttribution: univate commented
Status: Fixed » Needs work
NROTC_Webmaster’s picture

NROTC_Webmaster CreditAttribution: NROTC_Webmaster commented

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

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

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

  • Commit 6aa2b73 on 6.x-2.x, 8.x-1.x by univate: 
    #555940 by univate: Call the sms_validate_number function when sending a...
dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Issue summary: View changes
Status: Needs work » Closed (fixed)