filter $_POST in PHP pdo

Alex Lord
Alex Lord used Ask the Experts™
on
foreach($_POST as $key => $val){
			
			$k = $key;
			$v = $val; 
			if($v=='NULL'){
	
				/*$stmt = new Database();
				
				$query = "UPDATE TXT2GIVE_CONTACTS SET :k = :v WHERE ID = :contactId";
				$stmt->query( $query );
				$stmt->bind( ':k', $k);
				$stmt->bind( ':contactId', $contactId );
				$stmt->bind( ':v', $v);
				$stmt->execute();
				$result = $stmt->all(); */
				$updatedFields = "v is null";

			} else {
				$updatedFields = "v is not empty";

				$stmt = new Database();

				$query = "UPDATE TXT2GIVE_CONTACTS SET :k = :v WHERE ID = :contactId";
				$stmt->query( $query );
				$stmt->bind( ':k', $k);
				$stmt->bind( ':contactId', $contactId );
				$stmt->bind( ':v', $v);
				$stmt->execute();
				$result = $stmt->all();
			} 

Open in new window




here is my statement, so as u see it goes into the $_POST and uses these to update the database,  however i also have a field within $_POST that isnt in that database such as subaction which triggers the case this code is currently sitting,  how can i remove these from the $_POST ? or filter them ?
Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
Most Valuable Expert 2018
Distinguished Expert 2018

Commented:
You can just call unset() to remove variable before your loop:

unset($_POST['someKey']);

Author

Commented:
thanks chris, i may have another question in relation to this when i hit it lol

Commented:
This is bad way to do what you're doing. There are two big problems:

1. It's inefficient - it runs a separate query for each field that is being updated. So if you update 3 fields, then you're running 3 queries instead of just updating 3 fields with one query.

2. It's susceptible to parameter manipulation. For example, if I mess with the contact form (using the developer tools built into every major browser) and add an input like this:
<input name="ID" value="-1">

Open in new window

...then your code will blindly follow the instructions to update the ID field for that record to -1. Granted, ID might not be your biggest field of concern, but if there's some other valuable field, it's equally susceptible.

Trying to modify the $_POST to accommodate the loop is a bad answer, because now you've permanently lost that data. The $_POST and $_GET should never be modified. It only creates confusion later on (is that truly the data that's coming over or is a field missing because I erased it from the array.....?). Just because you CAN modify it does not mean you should.

You should RARELY EVER take the raw value from the end user and put it right into the database. You should always try to validate the input if you can (did they REALLY donate a billion dollars or is that just what the input value said?).

To fix these problems (and your original issue), don't loop through POST.

Yes, it can be kind of cool and convenient to have something dynamic like that, but it also sets you up as a hacking target. Your code should individually extract the desired fields from the $POST array and then build a single update query. There's probably an even cleaner way of doing this, but here's the general idea:

$changedFields = array();

if(isset($_POST["fieldX"]) && ($_POST["fieldX"] != "NULL"))
{
  // Validate the value of fieldX to make sure it's correct, and then once you've done that...
  $changedFields["fieldX"] = validated / sanitized value of fieldX;
}

// Repeat for fieldY, fieldZ, etc...

// Then build your UDPATE
$stmt = new Database();

$query = "UPDATE TXT2GIVE_CONTACTS SET ";
foreach($changedFields as $k => $v)
{
  $query .= ":{$k}_key = :{$k}_value ";
}
$query .= "WHERE ID = :contactId";
$stmt->query( $query );
foreach($changedFields as $k => $v)
{
  $stmt->bind( ':{$k}_key', $k);
  $stmt->bind( ':{$k}_value', $v);
}
$stmt->bind( ':contactId', $contactId );
$stmt->execute();
$result = $stmt->all();

Open in new window

Ensure you’re charging the right price for your IT

Do you wonder if your IT business is truly profitable or if you should raise your prices? Learn how to calculate your overhead burden using our free interactive tool and use it to determine the right price for your IT services. Start calculating Now!

Commented:
@Chris - Really? You suggested modifying $_POST? Come on - you know that's not the right answer here.
Most Valuable Expert 2018
Distinguished Expert 2018

Commented:
@gr8gonzo ... :)

Generally not something I would normally do ( but I have to admit - I have done it in the past !! )

Author

Commented:
Hmm ok i can see how that will be an issue,

for Validate you got field x than repeact, what do you mean for this section as my $_POST array contains values from 10 inputs and one of them is called subaction which triggers a case statement and isnt within the table.
Most Valuable Expert 2018
Distinguished Expert 2018

Commented:
You might want to show us an example of your POST data. There's nothing fundamentally wrong with looping over your POST data but to give you the best advice, we'd want to see what you're working with.
Commented:
So I was just stating that you should write code for each of the 10 fields so you're specifying the names of each. That way, you're not accidentally grabbing an unrelated field like subaction.

What I would normally do in this situation is have an array that lists out fields and a validation function to call on each one. I'll simplify this down to some sample fields, but you don't need to follow this - it's just for example purposes.

Let's say my table looks like this:
CREATE TABLE foo (
  id int(11),
  name varchar(50),
  lastPaid timestamp,
  numPayments int(11),
  moneyDue decimal(5,2),
  moneyPaid decimal(5,2)  
);

Open in new window


Now let's say we want to update all values except the "id" field, and we want to make sure that all the values are actually formatted correctly. So before I do anything with the database, I set up a small routine that loops through the incoming POST values and runs a different function on each one to format/validate/sanitize the value. So if a person sends in "numPayments" with a value of "abc" instead of a number, we can stop and report the error instead of trying to run a query that will definitely fail when we try to put letters into an integer field.

// ======== START OF SANITIZE FUNCTIONS =========
function sanitize_limit50($value)
{
  if($value == "NULL") { return null; }
  return substr($value, 0, 50);
}

function sanitize_timestamp($value)
{
  if($value == "NULL") { return null; }
  try
  {
    $dt = new DateTime($value); // Allow the DateTime class to try and parse it to see if it's a valid date/time
    return $dt->format("Y-m-d H:i:s"); // Format it as YYYY-MM-DD HH:MM:SS
  }
  catch(Exception $ex)
  {
    throw new Exception("Value " . htmlspecialchars($value) . " is not a valid date/time!");
  }
}

function sanitize_numeric($value)
{
  if($value == "NULL") { return null; }
  if(!is_numeric($value) { throw new Exception("Value " . htmlspecialchars($value) . " is not numeric!"); }
  return $value + 0;
}
// ======== END OF SANITIZE FUNCTIONS =========


// ======== START OF INPUT SANITATION / VALIDATION =========

// Define which fields we're going to sanitize / validate from POST and how
$inputs = array(
  "name" => "sanitize_limit50",
  "lastPaid" => "sanitize_timestamp",
  "numPayments" => "sanitize_numeric",
  "moneyDue" => "sanitize_numeric",
  "moneyPaid" => "sanitize_numeric",
);

// Now actually run the sanitation functions against our POST input
try
{
  $sanitizedFields = array();
  foreach($inputs as $postField => $sanitation_function)
  {
    if(isset($_POST[$postField]))
    {
      // Run the sanitation / validation function on the value from POST - if it throws an exception, then we have an error. If it doesn't throw an exception, then we have a working value.
      $sanitizedValue = call_user_func($sanitation_function, $_POST[$postField]);

      // If there's a non-null value, then add it to our final array of $sanitizedFields
      if($sanitizedValue !== null)
      {
        $sanitizedFields[$postField] = $sanitizedValue;
      }
    }
  }
}
catch(Exception $ex)
{
  // Display our validation error so we know which field failed and why, and then exit the script
  echo "There was a validation error while processing the " . htmlspecialchars($postField) . " field. " . $ex->getMessage();
  exit();
}
// ======== END OF INPUT SANITATION / VALIDATION =========


// ======== START OF DATABASE QUERY =========

// In here, build your query based off of $sanitizedFields.

// ======== END OF DATABASE QUERY =========

Open in new window


So in this code, we're using some functions along with the try/catch error handling methodology to make sure that ALL of our incoming POST data is formatted correctly before trying to run the database query. That way, you nearly guarantee that if you try to run your query, it will work each time. It saves load on the database, and makes your script feel more snappy and responsive because it can validate errors faster.

Author

Commented:
@gr8gonzo

hey ill give this a good look over and get back to you on outcome, thank you for learning advice.
Top Expert 2004

Commented:
Just a couple of additional thoughts:

First and foremost, gr8gonzo's admonition regarding actually altering the $_POST data is right on target.  That's the original version of the data, and should be sacrosanct.  Any alterations to that data should be done, minimally, on a copy, not on the original array.  Ideally, no alteration would be necessary - filtering the keys into a working copy should be sufficient for your purposes.

Regarding which keys to use from $_POST, a very common strategy is to make the application aware of the schema with which it is working.  This is normally done during bootstrap, or through a lazy-load bootstrap strategy, and involves the application actually querying the DB for the table schema prior to work being done.  Assuming your $_POST data matches your field names (or if you have an appropriate map for mismatched names), then filtering becomes very simple.

Finally, gr8gonzo's initial reply mentioned running multiple queries.  Again, the observation is on target.  If you know this update will affect a single table, it is much better efficiency to collect all the changes, then run a single UPDATE query.  The execution of a loop to validate the fields and data is minimal time, but there is a relatively huge amount of overhead involved in sending commands to the database service, even with the connection already open.

Author

Commented:
Im working on this area but taking it slow using gr8gonzo's advice and taking my time, thax for the help :)

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial