Generating the property name of an object

Question

$roleId = pg_escape_string($_POST['role']);
$grantValue = pg_escape_string($_POST['grant']);
$feature = pg_escape_string($_POST['feature']);
$permission = pg_escape_string($_POST['permission']);
$permissionsModel = new Permissions();
$record = $permissionsModel->getPermission($roleId);
//assemble object's property name based on the action taking place and save changes.
$property = trim($feature . '_' . $permission);
$record->$property = $grantValue;
$record->save(false);

Here is an example of an actual property of the object:

$record->report_w;

$record->facebook_d;

My second question is about escaping the data: can you suggest a better way of validating user input?

Answer 1

Yes, this is bad practice, sometimes

You are giving the user the option to dynamically change object properties. What if the property doesn't exist? What if a boogie monster passes by?

Sometimes, generating the property could be usefull, but only do this when you trust the thing that is generating the property. Is it a user? bad practice. Is it the program itself, wel, maybe.

escaping data

Don't escape data, don't try and remove all 'bad' data. A better approach is to check for a whitelist.

$role has to be an integer, so make sure it is

<?php
if ( preg_match('/^[0-9]+$/',$_POST['role']) != 1 )
{
 throw new Exception('I need an int');
}

feature and permission probably only accept alpha characters? And probably only lower case? So let's check that:

if ( preg_match('/^[0-9a-z]+$/',$_POST['role']) != 1 )
{
 throw new Exception('I need an int');
}

whitelists are better

Use whitelists instead of blacklist. It's easier to know what is allowed then to know what isn't. Simply because we can't know all the bad things out there

• tareq: thanks for your reply. I may mislead you when i said "user input", the user doesn't really input the data in my case, he merely selects from some options by clicking a button, so all the passed data is always (unless somehow hacked) gonna be the same. and role actually doesn't have to be an integer, it is concatenated with a string in the end to produce one string which is the property name. also why did you use regex instead of the php functions is_int or is_numeric and strtolower for lowercase? can you elaborate what u meant by whitelists in this context?
• Pinoniq: and what stops a scriptkiddie/hacker to post bad data to your application?
• Pinoniq: is_int will always be fault because it checks the type of the variable, not the content (is_int('1') == false) where as is_numeric also returns true for hexadecimal, negative values (unwanted for an id). That is why I use regex. Then on the strtolower case: why accept Capital letters if you actually need lower-case... Then whitelist, just look up what it means. You whitelist a set of characters (e.G. 0-9 and only allow them) instead of filtering out all bad-data (impossible)
• CodeX: A plugin like firebug for firefox will allow the user to change the value sent from all inputs. Pinoniq's answer is the one i would go for.