10

I've just completed my registration form for my website and for the action page where all the SQL takes place I've just skipped assigning the POST variable to actual ones, like this...

$username = $_POST['username'];

Instead I've just been using the POST variables throughout the PHP page. Are there any risks or errors that can be met whilst practicing?

Also, excuse me if I am using incorrect terminology...

aelsheikh
  • 2,268
  • 5
  • 26
  • 41
  • 2
    If you are going to use that variable in your SQL query it would be a good idea to filter it before with `mysql_real_escape_string()` – Mike Jan 29 '12 at 00:10
  • 1
    To assign a variable simply means to make a value available under another name. So no, it does not matter whether you run `mysql_real_escape_string()` on `$username` or on `$_POST['username']` as long as you keep using one of those options consequently within their scope. – Quasdunk Jan 29 '12 at 00:12
  • Thanks for your great comments. Unfortunately I'm using sql server so those that function won't work in my code – aelsheikh Jan 29 '12 at 00:30

5 Answers5

5

One risk you might be running is dealing with raw user data, still saved in the raw $_POST[] variable. I tend to save all the raw data I work with to other variables, like you mentioned with $username = $_POST['username'] so I can manipulate and sanitize that input more efficiently. Rather than save any adjustments I make to the global $_POST array, all my changes are saved temporarily and at a more manageable scope.

For example:

$username = mysql_real_escape_string($_POST['username']);

... is better than:

$_POST['username'] = mysql_real_escape_string($_POST['username']);

It's generally better to leave the raw user data as is and make your adjustments in other variables.

Andrew
  • 36,541
  • 13
  • 67
  • 93
  • I agree with this. In vanilla PHP, I always assign POST vars to their own unique variable simply to stay organized. – Graham Swan Jan 29 '12 at 01:08
5

I see no advantage or disadvantage. Once you start modifying the values, you should put them into their own variable, but if you're just reading them, you can leave them where they are. Only two points:

  • If it makes your source code more readable to use short variables names instead of $_POST[...], that's a good reason to put the values into their own variables.
  • Don't necessarily take values out one by one, but just assign the array contents into another array:

    $values = $_POST;
    
    // not:
    
    $foo = $_POST['foo'];
    $bar = $_POST['bar'];
    ...
    
deceze
  • 510,633
  • 85
  • 743
  • 889
1

Assigning it to another variable will serve you well when you decide to implement another method of input (json-encoded posts, xml-rpc, soap, etc.). Making sure you get what you need from the $_POST array at the start early on and working with those values later will make it easier to reuse the code with those other inputs: the only thing that needs to change is the instantiation of those inputs.

Also, often you want to change a value somewhat (default trim()-ing, etc.), which is better done on a local variable then an item in a $_POST array. Certainly on bigger projects with dozens of coders it is in my opinion a good practice to always keep the $_POST array as received, and not fiddle in it directly infuriating a hopelessly debugging coworker...

The risks and errors do not change: it is still user-input which you should never trust, and always assume the worst case scenario of. Standard SQL-injection, XSS, and other attacks are not prevented with the practise alone.

Wrikken
  • 69,272
  • 8
  • 97
  • 136
0

I ran into the same question this very day and I came to conclusion that you should always check if the index exists in order to prevent an "Undefined index" error. If you use the POST data multiple times I would suggest assigning the result of your test to a new variable as you might modify the POST data:

//at least PHP 7.0.0 required
$foo = $_POST['foo'] ?? '';
$bar = $_POST['bar'] ?? '';

Which is aquivalent to:

$foo = isset($_POST['foo']) ? $_POST['foo'] : '';
$bar = isset($_POST['bar']) ? $_POST['bar'] : '';

If you only use the POST data once one can skip assigning a new variable and just use $_POST['foo'] ?? '' as argument.

This solution is based on this excellent answer. Further reading: https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op

Reki
  • 1
0

I personally dislike dupe variables. Stick to what you got until u need to drasticly transform it. Dupe variables makes it harder to track and just wastes memory and time. Why bring sand to the beach.

select * from tbl where this = '". mysql_real_escape_string(trim($_POST['that'])) ."'

tim
  • 2,530
  • 3
  • 26
  • 45
  • `$foo = $_POST` wastes hardly any memory, since no data is being copied. – deceze Jan 29 '12 at 00:53
  • 1
    I didnt say it wastes a lot of memory. Whats the real win in duping variables? Unless a split usage? – tim Jan 29 '12 at 01:00
  • Quote: *"Dupe variables [...] just wastes memory [...]"*... Not saying the rest of your statement isn't true, just saying that it doesn't waste memory (beyond what is necessary to store a new *pointer* to the values), which is what you stated. :) – deceze Jan 29 '12 at 01:07
  • So what holds the data then? Air? Try compiling and see how many bytes get compiled. There will be a byte difference. – tim Jan 29 '12 at 01:11
  • 1
    OK, what exactly are you talking about then? The added characters in the source code? The extra entry in the symbol table? Or the actual data? Because the former two are the only things that change, and they're hardly worth mentioning. – deceze Jan 29 '12 at 01:21