0

I have a basic form with a dozen fields (I would take 3 for example):

<input type="text" name="user_first_name" class="form-control" pattern="[A-Za-z-]+" required />
<input type="text" name="user_last_name" class="form-control" pattern="[A-Za-z-]+" required />
<input type="tel"  name="user_phone" class="form-control" />
...

Only the phone number can be empty, the last name and first name are obligatory and can contain only letters and dashes (the technical constraints were imposed on me by our old ERP)

I created a function to clean up all my fields that looks like this:

public function sanitizeInfo($user_first_name, $user_last_name, $user_phone) {

    $user_first_name  = preg_replace('/[^A-Za-z-]/', '', $user_first_name);
    $user_last_name   = preg_replace('/[^A-Za-z-]/', '', $user_last_name);
    $user_phone       = (isset($user_phone) and !empty($user_phone)) ? preg_replace('/[^A-Za-z0-9+-.)(]/', '', $user_phone) : NULL;

    $array = array(
      "first_name" => $user_first_name,
      "last_name"  => $user_last_name,
      "phone"      => $user_phone
    );

    return $array;
  }

In my PHP script I make this first check:

$fields = array('user_first_name', 'user_last_name');
$error = FALSE; 

foreach ($fields as $fieldname) { 
  if(!isset($_POST[$fieldname]) or empty($_POST[$fieldname])) {
    $error = TRUE; 
    $message = 'err';
  }
}

if (error === TRUE) {
  echo "Error !"
} else {
  $info = sanitizeInfo($_POST['user_first_name'], $_POST['user_last_name'], $_POST['user_phone']);
  ...
  ** QUERY **
}

I want to check, before sending this in database, that the fields are not empty (only the telephone number can be NULL)

But the problem right now is that I do not know if my non-required fields exist and especially my sanatizeInfo function is problematic because it allows to put empty fields in database

Example: The user enters "!! -" as firstname and the sanitizeInfo function returns "" because the preg_replace to delete these characters

How to avoid this?

Rocstar
  • 1,427
  • 3
  • 23
  • 41
  • 1
    You can call `sanitizeInfo` before checking empty fields. `empty` would alone do the job, no need for `!isset`. – Sougata Bose Jun 13 '19 at 12:33
  • 2
    Turn on error reporting. It will help find errors like `if (error === TRUE) {`. https://stackoverflow.com/a/5438125/296555 – waterloomatt Jun 13 '19 at 12:34
  • @SougataBose The problem is that a user can delete a field in the HTML form and cause PHP errors in the script – Rocstar Jun 13 '19 at 12:35
  • Just a heads up: Some people with double names do actually have space instead of dash between them. Your regex would remove the space and combine those names into one. – M. Eriksson Jun 13 '19 at 12:36
  • For that you can do all in that function. First check if it is set or not then sanitize. If the problem persists throw error. – Sougata Bose Jun 13 '19 at 12:36
  • 1
    Addition to @MagnusEriksson Some do have `.` is their names too. – Sougata Bose Jun 13 '19 at 12:37
  • 1
    ...also, many people have other characters in their names than the ones in the English alphabet. Over here in Sweden, having `åäö` isn't uncommon. If someone is named `Åke`, you would store it as: `ke`. – M. Eriksson Jun 13 '19 at 12:37
  • @SougataBose I can't do it in the function because I can't send ``$_POST['user_phone']`` in the function without this ``error : Notice: Undefined index: user_phone`` – Rocstar Jun 13 '19 at 12:43
  • 1
    Accessing elements in the `$_POST`-super global should be the same regardless where you do it since it's a _super global_ (accessible everywhere) – M. Eriksson Jun 13 '19 at 12:59

1 Answers1

0

All you need to do is calling the function sanitizeInfo before checking empty fields and replacing your foreach loop with a simple array_search function.

This can be implemented as follows:

$fields = array('user_first_name', 'user_last_name');
$error  = FALSE;

$values = sanitizeInfo($fields[0], $fields[1], @$fields[2]);
$arSear = array_search('', $values);
if ($arSear !== FALSE && $arSear != 'phone'){
  $error   = TRUE;
  $message = 'The field `'.$arSear.'` is empty.';
}

if ($error) {
  echo 'Error: '. $message;
} else {
  // QUERY ....
}

As a matter of consistency, I recommend always using && and || in php versus AND and OR. This will prevent any trip ups regarding precedence. 'AND' vs '&&' as operator

RyadPasha
  • 333
  • 3
  • 9
  • Thank you (for the answer and the tips for operator logic) but I don't want to disable any error – Rocstar Jun 13 '19 at 13:12
  • If you don't wanna show any errors, then the last code can be: `if(!$error) { /* QUERY HERE */ }` – RyadPasha Jun 13 '19 at 13:19
  • We misunderstood each other, I was talking about that: ``@$fields[2]`` – Rocstar Jun 13 '19 at 13:24
  • Your function `sanitizeInfo` doesn't have a default value for the variable `$user_phone`, so I used the 'Error Control Operator' (@) to tell PHP "Please don't show errors when the user didn't pass it". – RyadPasha Jun 13 '19 at 13:34
  • That is because you said "only the telephone number can be NULL" and hence you should either put a default value for it in the function definition, it will be `function sanitizeInfo($user_first_name, $user_last_name, $user_phone = NULL){ ...` or check if it's not given and set it to 0 or NULL, or simply adding the 'Error Control Operator' (@) before its variable as I did. – RyadPasha Jun 13 '19 at 13:38