Escaping PHP GET and POST values

Question

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 :)

Solution 1

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.

Solution 2

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.

Comments

[jribeiro]

Hmmm i don't use it in all POST values. It's just for re-use when really needed. Anyway great answer. Thanks

[itachi]

That sql insert query looks little bit of ugly....

[Adrian Günter]

Also use mysqli or PDO, not the mysql_ extension. And use prepared statements, not mysql*_real_escape_string.