4
\$\begingroup\$

I've made about 2 forms that have a server side action using PHP. For the first one, I made a variable for every form field and mapped it to $_POST["name"]. For small amount of fields, it's not a problem, but it's tiring for large amounts.

For my second form, I used the scripts below. The problem I had with it was how unflexible it was, due to the order of the HTML fields and DB column names having to match. At the end of this post, I also attempt to provide a fix to it.

Here's a simplified example to the forms, and how I've been approaching them.

<!DOCTYPE html>
<html>

<head>
    <meta charset="UTF-8">
    <title></title>
    <style>
        label {
            line-height: 200%;
        }
    </style>
</head>

<body>
    <label for="first_name">First Name:
        <input name="first_name" type="text" id="first_name">
    </label>
    <br/>
    <label for="last_name">Last Name:
        <input name="last_name" type="text" id="last_name">
    </label>
    <br/>
    <label for="address">Street Address:
        <input name="address" type="text" id="address">
    </label>
    <br/>
    <label for="state">State:
        <input name="state" type="text" id="state">
    </label>
    <br/>
    <label for="zip_code">Zip Code:
        <input name="zip_code" type="text" id="zip_code">
    </label>
    <br/>
    <label for="phone_number">Phone Number:
        <input name="phone_number" type="text" id="phone_number">
    </label>
    <br/>
</body>

</html>

Here is the database table associated with the form (the columns aren't necessarily supposed to be in order with the form fields):

+------------------------+
| column_name            |
+------------------------+
| applicant_id           |
| first_name             |
| last_name              |
| address                |
| city                   |
| state                  |
| zip_code               |
| phone_number           |
+------------------------+

I have a loop in a separate script that iterates through, and adds each $_POST value from the form (or $_POST array) in order into an empty array:

$input_array = []; 
//testInput() is the sanitation
foreach ($_POST as $key => $val)
{   if ($_SERVER["REQUEST_METHOD"] == "POST") {
        if (isset ( $_POST [$key] ) && ! empty ( $_POST [$key] )) {
            $input_array[] = testInput($val); 
        }
    }
}

This $input_array is the the array I provide as the $postArray parameter in the buildArr($postArray, $handle, $tableName) function shown later.

This is the script that:

  1. First gets the column names from the database and builds an array with them with getColumnNames($tableName, $handle).
  2. Builds a Query string for a PDO insert using named placeholders by looping through the array. The named placeholders are just the column names prepended with a ':'. The result is something like: INSERT INTO table_name (col1,col2,col3) VALUES (:col1,:col2,:col3)
  3. A PDO statement can take an array containing a mapping between the named placeholders and the actual values (look here). So buildArr($postArray, $handle, $tableName) takes values from the $_POST array and maps them to the named placeholders.

        $dbHandle = ConnectDB();
    
        /**
        * Gets the columns of a MySQL table
        * @param {String} $tableName is the name of the table
        * @param {Object} $handle is the PDO object
        * @return {array} returns a numeric array containing column names
        */
        function getColumnNames($tableName, $handle) {
            $query = $handle->prepare("SHOW COLUMNS FROM $tableName");
            $query->execute();
            $tableFields = $query->fetchAll(PDO::FETCH_COLUMN);
            return array_slice($tableFields, 1);
        }
    
        /**
        * Builds a INSERT query using named placeholders
        * @param {String} $tableName is the name of the table to be inserted
        * @param {Object} $handle is the PDO object
        * @return {String} returns a INSERT query
        */
        function getColumnStrings($tableName, $handle) {
    
            $columnArray = getColumnNames($tableName, $handle);
            $size = sizeof($columnArray);
            $sql = "(";
            for($i = 0; $i < $size; $i++) {
                $sql .= $columnArray[$i] . ",";
    
            }
            $sql = rtrim($sql, ",");
            $sql .= ")";
            $values = "VALUES (";
            for($i = 0; $i < $size; $i++) {
                $values .= ":" . $columnArray[$i] . ",";
    
            }
            $values = rtrim($values, ",");
            $values .= ")";
    
            return "INSERT INTO " . $tableName . " ". $sql . " " . $values;
    
        }
    
        /**
        * Builds an associative array to place inside query->execute()
        * @param {array} $postArray numeric array containing $_POST values
        *        $postArray is the array containing all the sanitized form values IN ORDER
        * @param {Object} $handle is the PDO object
        * @param {String} $tableName is the name of the table
        * @return {array} returns an associative array with
        *                 values to be replaced
        */
        function buildArr($postArray, $handle, $tableName) {
            $namesArray = getColumnNames($tableName, $handle);
            $size = sizeof($namesArray);
            $paramArray = [];
            //iterates through and
            //prepends all the column names in array with ':'
            //These refer to the named placeholders in the INSERT query
            for($i = 0; $i < $size; $i++) {
                $namesArray[$i] = ":" . $namesArray[$i];
    
            } 
            //Result is like array(':col1', ':col2', ':col3', ':col4'); etc.
    
    
            //iterates through and builds an associative array 
            //Association is namedplaceholderto actual value
            //This array is to be provided to the PDOstatement->EXECUTE()
            for($i = 0; $i < $size; $i++) {
                $paramArray[$namesArray[$i]] = $postArray[$i];
            }
            //result is like array(':col1' => "John", ':col2' => "Smith", ':col3' => "22 Ave.", ':col4' => "12345"); etc.
    
            return $paramArray;
        }
    
        $dbHandle = null;
        ?>
    

The problem with this approach is that it isn't very flexible. Order matters for the database column names and the order of the form fields. Since the names in the form fields are same as the database column names, I can remodel buildArr() to instead use the associative array $_POST:

//$postArray args not needed anymore
function buildArr($handle, $tableName) {
    $namesArray = getColumnNames($tableName, $handle);
    $size = sizeof($namesArray);
    $paramArray = [];

    for($i = 0; $i < $size; $i++) {
        //OMITTING USER INPUT SANITATION
        $paramArray[":" . $namesArray[$i]] = $_POST[$namesArray[$i]];
    } 

    return $paramArray;
}

This iterates through the given array containing column names, and maps that column name to the HTML Form field name attribute. So now, the order of the fields doesn't matter in either the DB or the HTML. However, this requires that the initial coding of the HTML and the creation of the table to have matching names for the fields.

Is this the best approach to have a flexible and dynamic way of pulling HTML form data, and storing it in a database using PHP?

Update:

This is the actual function that submits the data to the database, in case it comes in use:

function addRow($dbHandle, $dataArray, $tablename) {
    try {
        $query = getColumnStrings($tablename, $dbHandle);
        $statement = $dbHandle->prepare($query);
        $statement->execute(buildArr($dataArray, $dbHandle, $tablename));
    } catch(PDOException $e) {
        echo $e->getMessage();
    }
}

I prepare the statement first, and then execute it, by providing it an associative array, which is supplied by the buildArr($dataArray, $dbHandle, $tablename).

\$\endgroup\$
2
  • \$\begingroup\$ Interesting and very detailed question! I changed the title to reflect a bit better how the code works, after reading your question. You're welcome to edit it again though, if you think it doesn't fit. \$\endgroup\$
    – Phrancis
    Jul 16, 2015 at 20:48
  • \$\begingroup\$ @Phrancis Thanks, my titles are usually unrepresentative of what I ask \$\endgroup\$ Jul 16, 2015 at 21:17

1 Answer 1

3
\$\begingroup\$

There's a few things I noticed that could be improved:


In the following code snippet, checking isset and !empty are literally the same, exact thing.

If the content isn't empty, then it's set.
Also, you shouldn't be checking $_SERVER["REQUEST_METHOD"] for every item in $_POST.
You can take the request method check out the front, and return false in the case it doesn't meet your conditions.

foreach ($_POST as $key => $val)
{   if ($_SERVER["REQUEST_METHOD"] == "POST") {
        if (isset ( $_POST [$key] ) && ! empty ( $_POST [$key] )) {
            $input_array[] = testInput($val); 
        }
    }
}

With those changes in mind, the following code snippet would be a much better alternative:

if ($_SERVER["REQUEST_METHOD"] == "POST"){ die("UNALLOWED ACCESS METHOD"); }
foreach ($_POST as $key => $val){
    if (!empty($_POST[$key])){
        $input_array[] = testInput($val);
    }
}

Next, getColumnStrings(); this function doesn't actually get strings (plural), but builds a string (singular).
You shouldn't be iterating over every element in $columnArray like that, for future reference, you can use a foreach loop, but, you should use a implode() here, rather than a loop.
Using an implode() lets you join all the strings together, at those joining points, append a string (',').

You shouldn't build SQL Queries as strings like that.

Use bind_param() instead.

function getColumnStrings($tableName, $handle) {

   $columnArray = getColumnNames($tableName, $handle);
       $size = sizeof($columnArray);
       $sql = "(";
       for($i = 0; $i < $size; $i++) {
           $sql .= $columnArray[$i] . ",";

       }
       $sql = rtrim($sql, ",");
       $sql .= ")";
       $values = "VALUES (";
       for($i = 0; $i < $size; $i++) {
           $values .= ":" . $columnArray[$i] . ",";

       }
       $values = rtrim($values, ",");
       $values .= ")";

       return "INSERT INTO " . $tableName . " ". $sql . " " . $values;

   }

Commenting solely on the $values building as I'm not as experienced with bind_param() as I'd like to be:

    function getColumnStrings($tableName, $handle) {
        $columnArray = getColumnNames($tableName, $handle);
        $sql = "(". implode(", ", $columnArray) . ")";
        $values = "VALUES (";
        $valueArray = [];
        foreach($columnArray as $item){
            $valueArray[]= ":" . $item;
        }
        $values = "VALUES (" . implode(", ", $valueArray) . ")";

        return "INSERT INTO {$tableName} {$sql} {$values}";

    }

That is, simply an improvement of what you have: IT IS JUST AS SAFE AS YOUR IMPLEMENTATION, WHICH IS: not very.


Rather than having sizeOf()s everywhere, use foreach.

\$\endgroup\$
7
  • \$\begingroup\$ Thanks for the improvements. Could you expand on why the implementation is not very safe? \$\endgroup\$ Jul 17, 2015 at 1:17
  • 1
    \$\begingroup\$ @Abdul, without properly binding your parameters, your script is open to SQL injection. \$\endgroup\$
    – Quill
    Jul 17, 2015 at 2:01
  • \$\begingroup\$ I'm new to PHP, but I was under the impression that [PDO::prepare][php.net/manual/en/pdo.prepare.php] is what prepares the statement to attempt to take care of weaknesses in the query open to SQL injection. I left out the code that does the actual query statement execution, but I prepare the statement, before executing it, providing an associative array containing the named placeholders and the actual values. Also, I thought binding the parameters just creates a link between a named placeholder and a value, with the alternative being using an associative array, which I chose. \$\endgroup\$ Jul 17, 2015 at 2:07
  • \$\begingroup\$ Am I incorrect in my understanding? Also, if that was to be taken care of, is there anything else unsafe, or anything other criticism? And I added to the end of my question the actual code storing the data. \$\endgroup\$ Jul 17, 2015 at 2:08
  • \$\begingroup\$ I hate to nitpick but isset() and !empty() have some minor differences. For example, a variable set to zero will be considered empty -- and may need to be watched for if zero is a valid input. \$\endgroup\$
    – A Smith
    Jul 18, 2015 at 22:01

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.