2
\$\begingroup\$

I have a PHP form that uses the POST method to get information from form to db. I have my form separate from the form validation and insertion page.

Here is my form:

<?php
//get the header and the navigation bar inside the page.
$page_title = 'Demographics';
include('php_includes/header.html');
?>

<div class="container-fluid" id="emergency_dept">
    <form class="form" action="insert_demographics.php" method="post" name="demographics" id="demographics">
    <h3>Demographics</h3>

    <div class="form-group">
        <div>Medical Record Number / Visit ID (Number 17 Characters):</div>
        <input class="form-control" id="mrn" name="mrn" type="text" maxlength="17">
    </div>

    <div class="form-group">
        <div>Last Name:</div>
        <input class="form-control" id="LastName" name="LastName" type="text" onfocus="emptyElement('status')" maxlength="75">
    </div>

    <div class="form-group">
        <div>First Name:</div>
        <input class="form-control" id="FirstName" name="FirstName" type="text" onfocus="emptyElement('status')" maxlength="75">
    </div>

    <div class="form-group">
        <div>Social Security Number:</div>
        <input class="form-control" id="SSN" name="SSN" type="text" onfocus="emptyElement('status')" maxlength="9">
    </div>

    <p></p>

    <button type="submit" id="demographicsbtn">Insert Demographics Data</button>
    <span id="status"></span>
</form>
</div>
<?php
include('php_includes/footer.html');
?>

Here is my validation and insert page:

<?php
$page_title = 'Insert Demographics';
include('php_includes/header.html');

// Connect to the database
include_once("php_includes/db_connect.php");

// Gather the posted data into local variables
$mrn_demographics = mysqli_real_escape_string($db_connect, preg_replace('/[^a-zA-Z0-9]/', '', $_POST['mrn']));
$last_name        = mysqli_real_escape_string($db_connect, preg_replace('/[^a-zA-Z]/', '', $_POST['LastName']));
$first_name       = mysqli_real_escape_string($db_connect, preg_replace('/[^a-zA-Z]/', '', $_POST['FirstName']));
$ssn              = mysqli_real_escape_string($db_connect, preg_replace('/[^0-9]/', '', $_POST['SSN']));

// Get user IP Address
$ip = preg_replace('#[^0-9.]#', '', getenv('REMOTE_ADDR'));

// Form data error handling
if($mrn_demographics == "" || $last_name == "" || $first_name == "" || $ssn == ""){
echo "The form submission is missing data.";
exit();
} else {
// End form data error handling
$sql = "INSERT INTO nys_demographics (
    mrn,
    pt_last_name, 
    pt_first_name, 
    pt_ssn, 
    ip_address, 
    record_insert_dtime
)
VALUES('$mrn_demographics' ,'$last_name','$first_name','$ssn','$ip', 
now())";
}

if (mysqli_query($db_connect, $sql)) { 
  echo "<h3>Demographics have been inserted successfully, you will be auto-directed to the Injury Details form.<br /><br /></h3>";
  echo "MRN: $mrn_demographics <br />";
  echo "Last Name: $last_name <br >";
  echo "First Name: $first_name <br />";
  echo "SSN: $ssn <br />";
  echo "From IP Address: $ip <br />";
  echo "On " .date("Y-m-d") ." at " .date("h:i:sa");
  header("refresh:3; url=injury_details.php");
} else {
  printf("Error: %s\n", mysqli_error($db_connect));
}

include('php_includes/footer.html');
?>

I have a couple of questions:

  1. Should my validation code be in a different PHP page or the same?
  2. Could my validation be better and better written?

I want to echo out what has been input into the form.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Essential drawbacks are:

  1. During validation, do not silently malform the entered values. Instead, verify them against a pattern ans display back in the form letting a user to correct them.
  2. Forget that mysqli_real_escape_string thing forever. Use prepared statements.
  3. Never show a database error to a site user. Just think, you are not the only user of your site.
  4. Never output a single byte before having all the database interactions finished. Thus move HTML header below.

Also some minor issues:

  1. Like it said above, there is no point in showing the entered values for 3 seconds. It's an outdated approach from the last century. Redirect them to a page where entered values are shown.
  2. There is no point in validating REMOTE_ADDR, it is always correct

Given all the above, here is your code without the validation part which is up to you

<?php
if ($_POST) {
    // do your validations


    if (everything is correct) {
        include_once("php_includes/db_connect.php");

        $sql = "INSERT INTO nys_demographics VALUES(NULL,?,?,?,?,?,now())";
        $stmt = $db_connect->prepare($sql);
        $stmt->bind_param("sssis", $_POST['mrn'], $_POST['LastName'], $_POST['FirstName'], $_POST['SSN'], $_SERVER['REMOTE_ADDR']);
        $stmt->execute();
        $id = $db_connect->insert_id;

        header("Location:injury_details.php?id=$id");
        exit;
    }
}
$page_title = 'Insert Demographics';
include('php_includes/header.html');
?>
<form>
... display your form
\$\endgroup\$
0
3
\$\begingroup\$
  1. You don't need to have your validation codes written in different page. But if you want to organize your code, you can go and separate your validation block.
  2. Consider changing your validation. Your preg_replace is quite limiting. An example would be for names. Spaces, numbers, special characters are not allowed. Don't forget that there are names that has the mentioned above. It would work. Your validation could be written better if you want to increase security. There are tons of practice for validation of data and securing it for SQL query.
  3. You could add some client side validation using HTML5 such as pattern, and required attribute.
  4. You said you want to output what has been submitted in the form. Do you really need to output it in the validation/insertion page and then get redirected to the other page? You can instead put the details on sessions and head directly to the page you need to be and output what you have put on the sessions.
  5. It is best to avoid putting PHP variables inside the quotation marks. "Your name is $name". Instead you can use {} to include it. "Your name is {$name}". However, it is recommended that you concatenate it instead "Your name is" . $name.
  6. When including database page, it is also recommended to use require instead of include. This is to make the script stop upon error on getting the file.
\$\endgroup\$
2
  • \$\begingroup\$ 5. is but a strange superstition. \$\endgroup\$ Apr 28, 2017 at 7:36
  • \$\begingroup\$ Thank you for all the comments in your answer. I will work through my page. \$\endgroup\$ Apr 28, 2017 at 11:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.