It looks like you're trying to create a generic INSERT function to put any data into the given table based on an array, securely.
Since you're using mysqli_
a better approach might be to use bound parameters.
It's not really "good practice" to require your config file in the function (I'm assuming it'll be pulled in anyway), it would be better to pass your $conn
variable (which I assume is a mysqli object) into the function.
I'd also structure $data
so that it's an associative array of fields to values, for example (as a sort of theoretical person insert). This means your values are bound to the field names within that array which makes a foreach
loop less error prone.
$data = array(
'forename' => "Fred",
'surname' => "Bloggs",
'email' => "fred.bloggs@example.com"
);
You'd have to have done any validation checks beforehand of course (making sure that the email is valid for instance).
Now your function could be written:
function dbRowInsert($conn, $table, array $data) {
$fieldInsert = "";
$valueInsert = "";
$valueTypes = "";
//array for bound parameters
$paramData = array();
//loop through the fields
foreach($data as $fieldName => &$fieldData) {
//SQL inserts
$fieldInsert .= "{$table}.{$fieldName}, ";
$valueInsert .= "?, ";
//determine the data types for the bound parameters
if(is_float($fieldData)) { $valueTypes .= "d"; }
elseif(is_int($fieldData)) { $valueTypes .= "i"; }
else { $valueTypes .= "s"; }
//push the field data to the bound parameters array (by reference)
$paramData[] = &$fieldData;
}
//trim the trailing ", " that will have been left
$fieldInsert = substr($fieldInsert, 0, -2);
$valueInsert = substr($valueInsert, 0, -2);
//prepend the bound parameters array with the data types string
array_unshift($paramData, $valueTypes);
//prepare the statement
$stmt = $conn->prepare("INSERT INTO {$table} ({$fieldInsert}) VALUES ({$valueInsert})");
//error trapping
if(!$stmt) { throw new Exception("Failed to prepare statement - ERR: {$conn->error}", E_USER_ERROR); }
//call_user_func_array wouldn't be necessary with PDO as
//you could bind parameters one at a time in the loop
call_user_func_array(array($stmt, 'bind_param'), $paramData);
//execute ... and relax
$stmt->execute();
}
It looks a bit longer but is has the following advantages:
- it uses a prepared statement
- the parameters are bound by type so it'll not be attempting to
escape integers as strings for instance
The biggest problem is that it's using mysqli_
which doesn't have a nice way to bind parameters (or values) to the statement when you have an unknown number of parameters to bind ... hence the ugly call_user_func_array
usage.
This could easily be mitigated by using PDO
rather than mysqli_
though.