[RESOLVED] Form submitting to database security question

ukphp

Hi all,

I'm fairly new to PHP and MySQL and gather there is quite a bit of security risks around having fields which users are able to input data which gets written to a database, so thought I'd seek advise.

I've created a form (locally), which captures data such as name, address, email, telephone etc. Just wanting to check the security of it is ok. Below is bits from the files. Anything else I need to be doing to it to ensure everything is secure? In terms of the field validation (e.g. name should only be a-z or A-Z) I've done it using jQuery, however is this safe enough or should I really do serverside validation too?

functions.php

function sanitise_input($data)
{
	$data = trim($data);
	$data = stripslashes($data);
	$data = htmlspecialchars($data);
	$data = mysql_real_escape_string($data);
	return $data;
	}

process-form.php

$NAME = sanitise_input($_POST["NAME"]);
$EMAIL = sanitise_input($_POST["EMAIL"]);

$query="INSERT INTO table(NAME, EMAIL) values('$NAME','$EMAIL')";

Derokorian

Client side validation should always be duplicated server-side due to the fact that someone could disable javascript, or by-pass your form all together (sending their own POST request from a different server all together).

stripslashes shouldn't be necessary. If you see slashes appearing in your input, the correct fix would be to disable the highly deprecated magic_quotes

I see here the 2 fields you use are strings but note that mysql_real_escape_string is only meant for just that, strings. If you have a number it is better to type cast it as int, float, double whatever the case may be.

ukphp

Derokorian;10997385 wrote:
Client side validation should always be duplicated server-side due to the fact that someone could disable javascript, or by-pass your form all together (sending their own POST request from a different server all together).

stripslashes shouldn't be necessary. If you see slashes appearing in your input, the correct fix would be to disable the highly deprecated magic_quotes

I see here the 2 fields you use are strings but note that mysql_real_escape_string is only meant for just that, strings. If you have a number it is better to type cast it as int, float, double whatever the case may be.

Hi Derokorian,

Thanks! Ok I see, so I should replace stripslashes with magic quotes instead? How would I go abouts adding this in? I'm fairly new to PHP so sorry if this is an easy one.

Telephone number is a field I have, so should this be set to "INTEGER" in my database intead of "VARCHAR" and use something like the following....

$TELEPHONE = sanitise_input($_POST(integer)["TELEPHONE"]);

Derokorian

ukphp;10997386 wrote:
Hi Derokorian,

Thanks! Ok I see, so I should replace stripslashes with magic quotes instead? How would I go abouts adding this in? I'm fairly new to PHP so sorry if this is an easy one.

No the point is you shouldn't need to call stripslashes unless magic quotes is on, and even then the better solution is to turn magic_quotes OFF.

Telephone number is a field I have, so should this be set to "INTEGER" in my database intead of "VARCHAR" and use something like the following....
$TELEPHONE = sanitise_input($_POST(integer)["TELEPHONE"]); 

Not exactly, first because telephone "number" is a misnomer in the sense that you won't perform numeric operations. Its really a string, a string of number so varchar is best. Casting it as an integer will cause problems on 32bit systems because the highest signed integer is 2³¹ - 1 or 2,147,483,647... I don't know about you but my phone number is numerically much larger than that!

Second point: you would have attempted to type cast incorrectly, the type goes BEFORE the variable: (int) $_POST['TELEPHONE'] also there would be no reason to run a number type cast as in integer through you sanitize_input especially if you look at what it does: trim leading and trailing spaces (which a true integer won't have), stripslashes (when's the last time you saw slashes in a number?), htmlspecialchars (numbers are 0-9 with . and , none of which will be affected by this function), and mysql_real_escape_string (which should be obvious has no effect since type casting as an integer inherently means its not a string).

bradgrafelman

ukphp;10997384 wrote:
I'm fairly new to PHP and MySQL and gather there is quite a bit of security risks around having fields which users are able to input data which gets written to a database

You've actually lumped a couple of different types of security risks into a single pile.

For example, there is one set of security risks introduced simply because you're incorporating user-supplied data somewhere inside of a SQL query. You should sanitize/validate this data to ensure that it actually matches what you expected to get as well as making it safe to use in the SQL query. For example, if you're expecting a U.S. telephone number, you might want to ensure that the string matches a certain pattern, such as the following regular expression pattern:

/\(?[0-9]{3}\)?[. -]?[0-9]{3}[. -][0-9]{4}/

(which would match all sorts of patterns like "(555) 555-5555", "5555555555", "555.555.5555", etc.).

In regards to the "making it safe to use" part, take the surname O'Fallon for example; it's a perfectly legitimate last name, and there was probably no malicious intent behind the user who entered that data. However, that's not to say that it's safe to use in a SQL query... example:

SELECT * FROM users WHERE last_name = 'O'Fallon'

(note the syntax error caused by the usage of unsafe user-supplied data).

On the flip side, there are HTML/XSS/etc. issues to worry about that have nothing to do with SQL security at all. For example, if you had a comment box and someone typed in this:

Hi there! <script type="text/javascript">window.location = 'http://malicious-website.example.com'; </script>

then none of the above sanitization addresses the concern that you're using user-supplied data stored in the DB and dumping it into an HTML document. This is where things like [man]htmlentities/man come into play to prevent unwanted HTML data to actually become a part of your HTML document.

ukphp;10997384 wrote:
Just wanting to check the security of it is ok.

What type of security? 😉

ukphp;10997384 wrote:
In terms of the field validation (e.g. name should only be a-z or A-Z)

What about names like O'Fallon? Or Smith-Jones? Or Raúl? Or 井深 大?

ukphp;10997384 wrote:

function sanitise_input($data)
{
	$data = trim($data);
	$data = stripslashes($data);
	$data = htmlspecialchars($data);
	$data = mysql_real_escape_string($data);
	return $data;
	}

This function is bad for one reason: it's trying to do more than what it's supposed to (and/or doesn't have a clearly defined purpose). It's sanitizing user input, sure.. but for what purpose? Remember, there are different types of security risks and thus different methods to mitigate each one; trying to lump them all into a single "sanitize" function just isn't going to work.

For example, I've got issues with every line of that function other than the return statement:

```
$data = trim($data);
```
Why would any and all data never be allowed to contain leading whitespace characters? For example, if I was storing my e-mail signature in a database, I would first enter several line breaks so that the signature appears several lines below the body of the e-mail. Your "sanitize" function, however, would strip off those spaces and I'd be unable to do anything about it.

Furthermore, what's "unsanitary" about data that contains leading whitespace? After all, that's supposedly the role/purpose of this function - to make data sanitary for use... somewhere?
```
$data = stripslashes($data);
```
This is just plain wrong for reasons Derokorian already mentioned above.
```
$data = htmlspecialchars($data);
```
Wait a minute - now we're sanitizing data for the purpose of using it inside of an HTML document?

Furthermore, what if you actually do want to allow a limited subset of HTML to be submitted in a given form field?
```
$data = mysql_real_escape_string($data);
```
... and now we're sanitizing data for a completely different purpose, suggesting that this function truly has no clearly defined purpose/role. Plus, now we've got a rather hidden dependency on this function and a resource to a MySQL server created by mysql_connect() (and not any other extension, such as the [man]MySQLi[/man] extension you should be using anyway).

Further more, you're now assuming that all data this function is going to be used for will only be string data (and not numerical, boolean, etc.).

Finally, note that as mentioned above, the entire [man]mysql[/man] extension is outdated and has been superseded by the [man]MySQLi[/man] extension; new projects are advised to use this extension (or others, such as [man]PDO[/man]) instead.

ukphp

Thanks both of you for your replies, very informative and clearly being new to this I have a lot to consider and get my head around. I've done some searching to try and find examples of what others have done that I could use to see how to do this correctly. I came across this article http://yensdesign.com/2009/01/how-validate-forms-both-sides-using-php-jquery/ which appears quite good. Any reccommendations of examples I can use or follow or is this one ok?

Thanks once again for your help, most appreciated.

traq

ukphp;10997397 wrote:
... I came across this article http://yensdesign.com/2009/01/how-validate-forms-both-sides-using-php-jquery/ which appears quite good. Any reccommendations of examples I can use or follow or is this one ok?

not that one.

among other things:
-- it uses [man]ereg/man functions, which are depreciated
-- it assumes that no 1,2, or 3-letter names exist
-- (for some reason) disallows spaces in passwords
-- won't validate all emails correctly (i.e., will reject some valid emails, including several of mine)
-- it assumes that no messages will be submitted with less than 10 characters
-- presumably this info is destined to be put into a database, but the validation functions do nothing to make the data safe for insertion (don't really make it "safe" for anything, just checks against a few arbitrary requirements).
-- not-great coding practices (somewhat subjective, but it uses shorttags, mixes processing logic with output, and so forth).

ukphp

Hi Traq,

Ok yeah after looking into it in more detail didn't look too great. After doing some research the following hopefully should address the issues you guys mentioned. Well maybe not all but hopefully some. Please let me know what else I need to do.

<?php

$errors = array();

// If request is a form submission
if($_SERVER['REQUEST_METHOD'] == 'POST'){
  // Validation

  // Check NAME is not empty
  if(0 === preg_match("/^[a-zA-Z\-\']+$/", $_POST['NAME'])){
    $errors['NAME'] = "Please enter a name.";
  }

  // Check TELEPHONE is not empty
  if(0 === preg_match("/^((\(44\))( )?|(\(\+44\))( )?|(\+44)( )?|(44)( )?)?((0)|(\(0\)))?( )?(((1[0-9]{3})|(7[1-9]{1}[0-9]{2})|(20)( )?[7-8]{1})( )?([0-9]{3}[ -]?[0-9]{3})|(2[0-9]{2}( )?[0-9]{3}[ -]?[0-9]{4}))$/}", $_POST['BNAME_TITLE'])){
    $errors['TELEPHONE'] = "Please enter valid phone number";
  }

  // Check EMAIL is valid
  if(0 === preg_match("^((?:(?:(?:\w[\.\-\+]?)*)\w)+)\@((?:(?:(?:\w[\.\-\+]?){0,62})\w)+)\.(\w{2,6})$", $_POST['EMAIL'])){
    $errors['EMAIL'] = "Please enter a valid EMAIL address.";
  }

// Check COMMENTS is valid
  if(0 === preg_match("(\/\*(\s*|.*?)*\*\/)|(\/\/.*)", $_POST['COMMENTS'])){
    $errors['COMMENTS'] = "Please enter special charactors from your comments.";
  }

 // If no validation errors
  if(0 === count($errors)){

// Sanitize details
$NAME = mysql_real_escape_string($_POST['NAME']);
$TELEPHONE = mysql_real_escape_string($_POST['TELEPHONE']);
$EMAIL = mysql_real_escape_string($_POST['EMAIL']);

 // Insert user into the database
    $query = "INSERT INTO 'table' 
             ('NAME', 'TELEPHONE', 'EMAIL', 'COMMENTS')
             VALUES
             ('$NAME', '$TELEPHONE', '$EMAIL', '$COMMENTS')";

$result = mysql_query($query);
  }
  }


// helper functions
function form_row_class($name){
  global $errors;
  return $errors[$name] ? "form_error_row" : "";
}

function error_for($name){
  global $errors;
  if($errors[$name]){
    return "<div class='form_error'>" . $errors[$name] . "</div>";
  }
}

function hsc($string){
  return htmlspecialchars($string);
}


?>

<form action="some-page.php" method="POST">
  <table class="form">
    <tr class="<?php echo form_row_class("NAME") ?>" >
      <th><label for="NAME">Name</label></th>
      <td><input name="NAME" id="NAME" type="text" value="<?php echo hsc($_POST['NAME']); ?>" />
        <?php echo error_for('NAME') ?></td>
    </tr>
    <tr class="<?php echo form_row_class("TELEPHONE") ?>">
      <th><label for="TELEPHONE">Last Name</label></th>
      <td><input name="TELEPHONE" id="TELEPHONE" type="text" value="<?php echo hsc($_POST['TELEPHONE']); ?>" />
        <?php echo error_for('TELEPHONE') ?></td>
    </tr>
    <tr class="<?php echo form_row_class("EMAIL") ?>">
      <th><label for="EMAIL">Email Address</label></th>
      <td><input name="EMAIL" id="EMAIL" type="text" value="<?php echo hsc($_POST['EMAIL']); ?>" />
        <?php echo error_for('EMAIL') ?></td>
    </tr>
    <tr class="<?php echo form_row_class("COMMENTS") ?>">
      <th><label for="COMMENTS">Comments</label></th>
      <td><textarea name="COMMENTS" id="COMMENTS"><?php echo hsc($_POST['COMMENTS']); ?></textarea>
        <?php echo error_for('COMMENTS') ?></td>
    </tr>
    <tr>
      <th></th>
      <td><input type="submit" value="Go!" /></td>
    </tr>
  </table>
</form>

johanafm

Have you tried executing that code? Did it pass without a single notice, warning or other error? If not, fix those things first.

Personally, I'd put some comments in the code to describe the purporse of each of those reg exps. If you go back to make modifications to either one of them in just a month or two, you'll have "fun" figuring out exactly what they did again.

You are STILL using mysql to connect to and query the database. As suggested several times before in this thread: DON'T.
There should be an i at the very end of it: [man]mysqli[/man]
Or use [man]PDO[/man]

Pass $error to the functions that need it as a function parameter rather than use it as a global.

ukphp

johanafm;10997459 wrote:
Have you tried executing that code? Did it pass without a single notice, warning or other error? If not, fix those things first.

Personally, I'd put some comments in the code to describe the purporse of each of those reg exps. If you go back to make modifications to either one of them in just a month or two, you'll have "fun" figuring out exactly what they did again.

You are STILL using mysql to connect to and query the database. As suggested several times before in this thread: DON'T.
There should be an i at the very end of it: [man]mysqli[/man]
Or use [man]PDO[/man]

Pass $error to the functions that need it as a function parameter rather than use it as a global.

Ok I see thanks 🙂

I've amended the code and have ran it but appear to be getting "Undefined index" errors on line 76. I've spent hours trying to fix but am unsure of what to do. Any ideas/help please?

<?php

// setting variable for db connection
$host = "localhost";
$username = "root";
$password = "myPassword";
$database = "myDatabase";

// connect to database
$con = mysqli_connect("$host", "$username", "$password", "$database");
if (!$con) {
    die("Could not connect: " . mysqli_error());
}


$errors = array();

// If request is a form submission
if ($_SERVER['REQUEST_METHOD'] == 'POST') {

// Validation
// Check NAME is not empty
    if (0 === preg_match("/^[a-zA-Z\-\']+$/", $_POST['NAME'])) {
        $errors['NAME'] = "Please enter a name.";
    }

// Check TELEPHONE is not empty
if (0 === preg_match("/^((\(44\))( )?|(\(\+44\))( )?|(\+44)( )?|(44)( )?)?((0)|(\(0\)))?( )?(((1[0-9]{3})|(7[1-9]{1}[0-9]{2})|(20)( )?[7-8]{1})( )?([0-9]{3}[ -]?[0-9]{3})|(2[0-9]{2}( )?[0-9]{3}[ -]?[0-9]{4}))$/}", $_POST['TELEPHONE'])) {
    $errors['TELEPHONE'] = "Please enter valid phone number";
}

// Check EMAIL is valid
if (0 === preg_match("^((?:(?:(?:\w[\.\-\+]?)*)\w)+)\@((?:(?:(?:\w[\.\-\+]?){0,62})\w)+)\.(\w{2,6})$", $_POST['TELEPHONE'])) {
    $errors['TELEPHONE'] = "Please enter a valid EMAIL address.";
}

// Check COMMENTS is valid
if (0 === preg_match("(\/\*(\s*|.*?)*\*\/)|(\/\/.*)", $_POST['COMMENTS'])) {
    $errors['COMMENTS'] = "Please enter special charactors from your comments.";
}

// If no validation errors
if (0 === count($errors)) {

    // Sanitize details
    $NAME = mysqli_real_escape_string($_POST['NAME']);
    $TELEPHONE = mysqli_real_escape_string($_POST['TELEPHONE']);
    $EMAIL = mysqli_real_escape_string($_POST['EMAIL']);
	$COMMENTS = mysqli_real_escape_string($_POST['COMMENTS']);

    // Insert user into the database
    $query = "INSERT INTO 'nominations' 
         ('NAME', 'TELEPHONE', 'EMAIL', 'COMMENTS')
         VALUES
         ('$NAME', '$TELEPHONE', '$EMAIL', '$COMMENTS')";

    $result = mysqli_query($query);

if(mysql_errno() === 0){
  // Form submitted successfully
  header("Location: success.php");

  } 
  }
 } 


// Helpers
function form_row_class($eName){
  global $errors;
  return $errors[$eName] ? "form_error_row" : "";
}

function error_for($eName){
  global $errors;
  if($errors[$eName]){
    return "<div class='form_error'>" . $errors[$eName] . "</div>";
  }
}

function hsc($string){
  return htmlspecialchars($string);
}

?>


<form action="process.php" method="POST">
            <table class="form">
                <tr class="<?php echo form_row_class("NAME") ?>" >
                    <th><label for="NAME">Name</label></th>
                    <td><input name="NAME" id="NAME" type="text" value="<?php echo hsc($_POST['NAME']); ?>" />
                        <?php echo error_for('NAME') ?></td>
                </tr>
                <tr class="<?php echo form_row_class("TELEPHONE") ?>">
                    <th><label for="TELEPHONE">Telephone</label></th>
                    <td><input name="TELEPHONE" id="TELEPHONE" type="text" value="<?php echo hsc($_POST['TELEPHONE']); ?>" />
                        <?php echo error_for('TELEPHONE') ?></td>
                </tr>
                <tr class="<?php echo form_row_class("EMAIL") ?>">
                    <th><label for="EMAIL">Email Address</label></th>
                    <td><input name="EMAIL" id="EMAIL" type="text" value="<?php echo hsc($_POST['EMAIL']); ?>" />
                        <?php echo error_for('EMAIL') ?></td>
                </tr>
                <tr class="<?php echo form_row_class("COMMENTS") ?>">
                    <th><label for="COMMENTS">Comments</label></th>
                    <td><textarea name="COMMENTS" id="COMMENTS"><?php echo hsc($_POST['COMMENTS']); ?></textarea>
                        <?php echo error_for('COMMENTS') ?></td>
                </tr>
                <tr>
                    <th></th>
                    <td><input type="submit" value="Go!" /></td>
                </tr>
            </table>
        </form>

Pass $error to the functions that need it as a function parameter rather than use it as a global.

Is this a case of changing

function form_row_class($eName){
  global $errors;
  return $errors[$eName] ? "form_error_row" : "";
}

function form_row_class($eName){
  $errors;
  return $errors[$eName] ? "form_error_row" : "";
}

I'll add in some comments around the regular expressions too. Sorry for all the question if they seem silly. Thank you once again for the help.

Derokorian

Is this a case of changing

function form_row_class($eName){ 
  global $errors; 
  return $errors[$eName] ? "form_error_row" : ""; 
}

function form_row_class($eName){ 
  $errors; 
  return $errors[$eName] ? "form_error_row" : ""; 
}

No it would be more like:

function form_row_class($eName,$errors){ 
  return isset($errors[$eName]) ? "form_error_row" : "";  // Using isset to prevent undefined index
}

// then you would call it like
echo form_row_class("NAME",$errors);

ukphp

Thanks Derokorian,

I've tried doing this for the next function called "error_for" however keep getting errors. Really struggling with all this! Sorry for all the questions.

Original

function error_for($eName){
  global $errors;
  if($errors[$eName]){
    return "<div class='form_error'>" . $errors[$eName] . "</div>";
  }
}

My attempt

function error_for($eName,$errors){
	return isset($errors[$eName]) ? "<div class='form_error'>" .$errors[$eName] . "</div>";


// then  call it like this?
echo error_for("NAME",$errors);

I presume the last function is ok and how it being called?

function hsc($string){
  return htmlspecialchars($string);
}

Derokorian

ukphp wrote:

function error_for($eName,$errors){ 
    return isset($errors[$eName]) ? "<div class='form_error'>" .$errors[$eName] . "</div>"; 


// then  call it like this? 
echo error_for("NAME",$errors);

if this is what you really have, you dont have a closing brace for the function defintion:

function error_for($eName,$errors){ 
    return isset($errors[$eName]) ? "<div class='form_error'>" .$errors[$eName] . "</div>" : ''; 
} // this is missing in your post

// then  call it like this? 
echo error_for("NAME",$errors);

Edit: also missing the second part of ternary, fixed my code to reflect this

ukphp

Ahhh I see, thanks Derokorian. Hopefully last question, I seem to still be getting "Undefined index" errors in the table where it is using the "hsc" function. So this part...

<td><input name="NAME" id="NAME" type="text" value="<?php echo hsc($_POST['NAME']); ?>" />

The function being....

function hsc($string){
  return htmlspecialchars($string);
}

If you need the whole code its here.....

<?php

// setting variable for db connection
$host = "localhost";
$username = "root";
$password = "myPassword";
$database = "myDatabase";

// connect to database
$con = mysqli_connect("$host", "$username", "$password", "$database");
if (!$con) {
    die("Could not connect: " . mysqli_error());
}


$errors = array();

// If request is a form submission
if ($_SERVER['REQUEST_METHOD'] == 'POST') {

// Validation
// Check NAME is not empty
    if (0 === preg_match("/^[a-zA-Z\-\']+$/", $_POST['NAME'])) {
        $errors['NAME'] = "Please enter a name.";
    }

// Check TELEPHONE is not empty
if (0 === preg_match("/^((\(44\))( )?|(\(\+44\))( )?|(\+44)( )?|(44)( )?)?((0)|(\(0\)))?( )?(((1[0-9]{3})|(7[1-9]{1}[0-9]{2})|(20)( )?[7-8]{1})( )?([0-9]{3}[ -]?[0-9]{3})|(2[0-9]{2}( )?[0-9]{3}[ -]?[0-9]{4}))$/}", $_POST['TELEPHONE'])) {
    $errors['TELEPHONE'] = "Please enter valid phone number";
}

// Check EMAIL is valid
if (0 === preg_match("^((?:(?:(?:\w[\.\-\+]?)*)\w)+)\@((?:(?:(?:\w[\.\-\+]?){0,62})\w)+)\.(\w{2,6})$", $_POST['TELEPHONE'])) {
    $errors['TELEPHONE'] = "Please enter a valid EMAIL address.";
}

// Check COMMENTS is valid
if (0 === preg_match("(\/\*(\s*|.*?)*\*\/)|(\/\/.*)", $_POST['COMMENTS'])) {
    $errors['COMMENTS'] = "Please enter special charactors from your comments.";
}

// If no validation errors
if (0 === count($errors)) {

    // Sanitize details
    $NAME = mysqli_real_escape_string($_POST['NAME']);
    $TELEPHONE = mysqli_real_escape_string($_POST['TELEPHONE']);
    $EMAIL = mysqli_real_escape_string($_POST['EMAIL']);
	$COMMENTS = mysqli_real_escape_string($_POST['COMMENTS']);

    // Insert user into the database
    $query = "INSERT INTO 'nominations' 
         ('NAME', 'TELEPHONE', 'EMAIL', 'COMMENTS')
         VALUES
         ('$NAME', '$TELEPHONE', '$EMAIL', '$COMMENTS')";

    $result = mysqli_query($query);

if(mysql_errno() === 0){
  // Form submitted successfully
  header("Location: success.php");

  } 
  }
 } 


// Helpers
function form_row_class($eName,$errors){
  return isset($errors[$eName]) ? "form_error_row" : "";  // Using isset to prevent undefined index
}


function error_for($eName,$errors){
    return isset($errors[$eName]) ? "<div class='form_error'>" .$errors[$eName] . "</div>" : '';
} 


function hsc($string){
  return htmlspecialchars($string);
}

?>


<form action="process.php" method="POST">
            <table class="form">
                <tr class="<?php echo form_row_class("NAME",$errors); ?>" >
                    <th><label for="NAME">Name</label></th>
                    <td><input name="NAME" id="NAME" type="text" value="<?php echo hsc($_POST['NAME']); ?>" />
                        <?php echo error_for("NAME",$errors); ?></td>
                </tr>
                <tr class="<?php echo form_row_class("TELEPHONE",$errors); ?>">
                    <th><label for="TELEPHONE">Telephone</label></th>
                    <td><input name="TELEPHONE" id="TELEPHONE" type="text" value="<?php echo hsc($_POST['TELEPHONE']); ?>" />
                        <?php echo error_for("TELEPHONE",$errors); ?></td>
                </tr>
                <tr class="<?php echo form_row_class("EMAIL",$errors); ?>">
                    <th><label for="EMAIL">Email Address</label></th>
                    <td><input name="EMAIL" id="EMAIL" type="text" value="<?php echo hsc($_POST['EMAIL']); ?>" />
                        <?php echo error_for("EMAIL",$errors); ?></td>
                </tr>
                <tr class="<?php echo form_row_class("COMMENTS",$errors); ?>">
                    <th><label for="COMMENTS">Comments</label></th>
                    <td><textarea name="COMMENTS" id="COMMENTS"><?php echo hsc($_POST['COMMENTS']); ?></textarea>
                        <?php echo error_for("COMMENTS",$errors); ?></td>
                </tr>
                <tr>
                    <th></th>
                    <td><input type="submit" value="Go!" /></td>
                </tr>
            </table>
        </form>

Pretty lost with this one to be honest. Any idea?

Derokorian

Using ternary and isset again, because the first time you load the page those indices will not be set until after you submit the form. Using just the one (use it to base the others off of)

value="<?php echo isset($_POST['name']) ? hsc($_POST['name']) : ''; ?>"

ukphp

Ok makes sense, thank you. Just one more question, I seem to be getting the following error messages but I'm unsure how to fix this as it only has 1 parameter to pass does'nt it?

"Warning: mysqli_real_escape_string() expects exactly 2 parameters, 1 given in"

// Sanitize details
        $NAME = mysqli_real_escape_string($_POST['NAME']);
        $TELEPHONE = mysqli_real_escape_string($_POST['TELEPHONE']);
        $EMAIL = mysqli_real_escape_string($_POST['EMAIL']);
		$COMMENTS = mysqli_real_escape_string($_POST['COMMENTS']);

Derokorian

mysqli_real_escape_string is not the same as mysql_real_escape_string and just like most (if not all?) of the other mysqli functions it expects the first parameter to be the connection object returned by mysqli_connect.

ukphp

Thanks, now have that working 🙂 Just when I thought I had it all working, appears not! It does not seem to be submitting the data into the database, plus when I click the button to submit it does not go through to the "success.php" page. Any ideas where this is going wrong? Also how would it be best to split this up, as I gather I should keep things seperate such as database connecting in its own file, functions in another etc? I tried this but it broke everything. I was trying to have a "form.php" page which sends to a "process.php" which does all the work and writing to the database, then a "success.php" page which says thanks for submitting form.

<?php

// setting variable for db connection
$host = "localhost";
$username = "root";
$password = "myPassword";
$database = "myDatabase";

// connect to database
$con = mysqli_connect("$host", "$username", "$password", "$database");
if (!$con) {
    die("Could not connect: " . mysqli_error());
}


// date
$DATE = date("Y-m-d");

$errors = array();

// If request is a form submission
if ($_SERVER['REQUEST_METHOD'] == 'POST') {

// Validation
// Check NAME is not empty
    if (0 === preg_match("/[a-zA-Z]/", $_POST['NAME'])) {
        $errors['NAME'] = "Please enter a valid name.";
    }

// Check TELEPHONE is not empty
if (0 === preg_match("/[0-9]/", $_POST['TELEPHONE'])) {
    $errors['TELEPHONE'] = "Please enter valid phone number";
}

// Check EMAIL is valid
if (0 === preg_match("/^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$/", $_POST['EMAIL'])) {
    $errors['EMAIL'] = "Please enter a valid email address.";
}

// Check COMMENTS is valid
if (0 === preg_match("/[a-zA-Z]/", $_POST['COMMENTS'])) {
    $errors['COMMENTS'] = "Please remove special charactors from your comments.";
}

// If no validation errors
if (0 === count($errors)) {

    // Sanitize details
    $NAME = mysqli_real_escape_string($con, $_POST['NAME']);
    $TELEPHONE = mysqli_real_escape_string($con, $_POST['TELEPHONE']);
    $EMAIL = mysqli_real_escape_string($con, $_POST['EMAIL']);
	$COMMENTS = mysqli_real_escape_string($con, $_POST['COMMENTS']);

    // Insert user into the database
    $query = "INSERT INTO 'myForm' 
         ('DATE', 'NAME', 'TELEPHONE', 'EMAIL', 'COMMENTS')
         VALUES
         ('$DATE', '$NAME', '$TELEPHONE', '$EMAIL', '$COMMENTS')";

    $result = mysqli_query($con, $query);

if(mysql_errno() === 0){
  // Form submitted successfully
  header("Location: success.php");

  } 
  }
 } 


 // Helpers
function form_row_class($eName,$errors){
  return isset($errors[$eName]) ? "form_error_row" : "";  // Using isset to prevent undefined index
}


function error_for($eName,$errors){
    return isset($errors[$eName]) ? "<div class='form_error'>" .$errors[$eName] . "</div>" : '';
} 


function hsc($string){
  return htmlspecialchars($string);
}



?>


<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Untitled Document</title>
<link rel="stylesheet" type="text/css" href="styles.css" />
</head>
<body>
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="POST">
<table class="form">
    <tr class="<?php echo form_row_class("NAME",$errors); ?>" >
      <th><label for="NAME">Name</label></th>
      <td><input name="NAME" id="NAME" type="text" value="<?php echo isset($_POST['NAME']) ? hsc($_POST['NAME']) : ''; ?>" />
        <?php echo error_for("NAME",$errors); ?></td>
    </tr>
    <tr class="<?php echo form_row_class("TELEPHONE",$errors); ?>">
      <th><label for="TELEPHONE">Telephone</label></th>
      <td><input name="TELEPHONE" id="TELEPHONE" type="text" value="<?php echo isset($_POST['TELEPHONE']) ? hsc($_POST['TELEPHONE']) : ''; ?>" />
        <?php echo error_for("TELEPHONE",$errors); ?></td>
    </tr>
    <tr class="<?php echo form_row_class("EMAIL",$errors); ?>">
      <th><label for="EMAIL">Email Address</label></th>
      <td><input name="EMAIL" id="EMAIL" type="text" "value="<?php echo isset($_POST['EMAIL']) ? hsc($_POST['EMAIL']) : ''; ?>" />
        <?php echo error_for("EMAIL",$errors); ?></td>
    </tr>
    <tr class="<?php echo form_row_class("COMMENTS",$errors); ?>">
      <th><label for="COMMENTS">Comments</label></th>
      <td><textarea name="COMMENTS" id="COMMENTS"><?php echo isset($_POST['COMMENTS']) ? hsc($_POST['COMMENTS']) : ''; ?></textarea>
        <?php echo error_for("COMMENTS",$errors); ?></td>
    </tr>
    <tr>
      <th></th>
      <td>
      <input type="submit" value="Go!" /></td>
    </tr>
  </table>
</form>
</body>
</html>

Derokorian

        $result = mysqli_query($con, $query); 

if(mysql_errno() === 0){ 
  // Form submitted successfully 
  header("Location: success.php"); 

  }

And what does this code do when there is an error? Nothing. Also Header manual page says:

HTTP/1.1 requires an absolute URI as argument to » Location: including the scheme, hostname and absolute path

bradgrafelman

One problem I see is here:

if(mysql_errno() === 0){

Note that you can't mix [man]mysql[/man] functions with [man]MySQLi[/man] functions.

Furthermore, it'd probably be more helpful if you actually did something with the MySQL error message (such as outputting or logging it) if an error did indeed occur.