Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Escaping PHP GET and POST values [duplicate]

Possible Duplicate:
The ultimate clean/secure function

I was informed in another thread that this bit of code was pretty useless:

function getPost($s) {
        if (array_key_exists($s, $_POST))
            return mysql_real_escape_string(htmlspecialchars($_POST[$s]));
        else return false;
    }


    function getGet($s) {
        if (array_key_exists($s, $_GET))
            return mysql_real_escape_string(htmlspecialchars($_GET[$s]));
        else return false;
    }

Can anybody help understand why and how I can make it better please? Links or references are welcome also.

Just trying to always improve :)

like image 404
jribeiro Avatar asked Feb 25 '12 17:02

jribeiro


2 Answers

Well, it's bad for the same way magic_quotes_gpc is bad. It's magic and will escape everything, whether you want it to or not. Instead, handle the escaping where it's used, and you can change things without any problem. So:

function post($key) {
    if(array_key_exists($key, $_POST)) {
        return $_POST[$key];
    }

    return false;
}

And do your escaping where it's needed. Otherwise, things can look strange, and unescaping them will defeat the point. Consider this; I input my last name, O'Hara, in a textbox. You want to echo it back, but you fetch it using getPost. Here's what I get back:

O\'Hara

Did you htmlspecialchars it again? Well, then I get:

O\'ara

or something. This happens to me a lot and it's incredibly annoying - please don't do it.

like image 80
Ry- Avatar answered Nov 20 '22 21:11

Ry-


I wouldn't say useless, just a bit misguided. You should do the escaping immediately before you use it in the context it needs to be escaped for. For example, if you want to send the value back to the browser you might do this:

echo htmlspecialchars($_GET['name']);

But if you want to send it to the database you might do this:

mysql_query(... 'INSERT INTO users VALUES ("'.mysql_real_escape_string($_GET['name']).'")');

With your method you are fixed in what you can do with it. If you do this:

echo getGet('name');

You are going to print out a MySQL escaped string rather than the actual name.

like image 37
spencercw Avatar answered Nov 20 '22 22:11

spencercw