-1

This is a quick one...

I'm currently learning about the use of global/static/local PHP variables, and would like some clarification on whether or not the use of a 'global' variable in this function is correct:

$mysqli = new mysqli($DBServer, $DBUser, $DBPass, $DBName);

function sanitise($var) {

    global $mysqli;

    $var = strip_tags($var);
    $var = htmlentities($var, ENT_NOQUOTES);
    $var = stripslashes($var);
    return $mysqli->real_escape_string($var);
}

The function will be used across a number of PHP files.

Many thanks!!

Sam Holguin
  • 553
  • 2
  • 9
  • 24
  • 2
    Global variables should be avoided. They make it very hard to maintain the software in the long run. Also, I wonder why there is `strip_tags()`, `htmlentities()` and `stripslashes()` applied to the variable. It shouldn't be done! If you have a reason to do it, you probably haven't understood the problem correctly. – Sven Aug 15 '13 at 12:59
  • 2
    Usage is correct. As long as the $mysqli variable has been initialised before you call the `sanitise` function. It would be a better idea though to remove the need for GLOBALS by passing $mysqli as a parameter to the sanitise function – RiggsFolly Aug 15 '13 at 13:00
  • @Sven I had the idea from a book I read that mentioned it is best to use these functions to fully sanitise a string. I shouldn't believe everything I read though :) teaching myself has its drawbacks. Should I avoid these and simply stick to the real_escape_string?? – Sam Holguin Aug 15 '13 at 13:02
  • Thank you @RiggsFolly I'll do that – Sam Holguin Aug 15 '13 at 13:02
  • Duplicate of http://stackoverflow.com/questions/129677/whats-the-best-method-for-sanitizing-user-input-with-php – Your Common Sense Aug 15 '13 at 13:04
  • 1
    Yeah, well... `strip_tags` does not correctly filter out only HTML tags. After that, any special characters get converted to entities, so you cannot use the database to sort those strings correctly, and the output can only be used as HTML. And then slashes get removed even if they were not there. The result is a complete mess, but it might look promising if applied to an innocent set of strings. IF SANITIZING ANYTHING, MAKE SURE TO EXPLAIN THE PURPOSE. THERE IS NO GENERAL SANITIZATION FUNCTION AVAILABLE. You could also use `$var=""` as a replacement for that kind of sanitization. – Sven Aug 15 '13 at 13:10
  • Find a book or tutorial that teaches using prepared statements – Mark Baker Aug 15 '13 at 13:12
  • 1
    Re the various functions you're using here: (1) There should be absolutely zero need to use `stripslashes()` in PHP today; it does the same job as `real_escape_string()` but not as well. (2) I would question the wisdom of calling `htmlentities()` here; it should be called when outputting the string, not when saving it. (3) `strip_tags()` yep, that's good to do, although again I don't think the DB layer is the right place for it. Finally, as others have said, you should really consider using mysqli's "paramterised queries" feature instead of escape strings. – Spudley Aug 15 '13 at 13:12
  • Points taken onboard!! Thank you very much for the advise – Sam Holguin Aug 15 '13 at 13:15
  • 1
    The current version of PHP is a reasonably good language, but the one thing to bear in mind about PHP is that older versions were really *really* bad. They were riddled with bad practices built into the language and virtually every tutorial and book back in the day was full of poor coding techniques. Sadly a lot of that bad advice is still circulating and people are still following it. If you're going to learn PHP, it is really important to ensure that you're learning from an up-to-date resource. (the same applies to other language too of course, but PHP is particularly prone to bad tutorials) – Spudley Aug 15 '13 at 13:21
  • @Spudley I am using "paramterised queries" further down in my code. Does this mean using real_escape_string() is wasted?? – Sam Holguin Aug 15 '13 at 13:34
  • 1
    @SamHolguin - Yes it does. Parameterised queries do not need to be escaped. In fact, if you do escape the string and then use a param query, you'll get uwanted slashes in your output. (And if you've done `stripslashes()` as well, you'll get even more of them!) – Spudley Aug 15 '13 at 13:36
  • @Spudley That I didn't know! I appreciate your help on this, thank you – Sam Holguin Aug 15 '13 at 13:45
  • 1
    It is "sanitize string" functions that have to be strictly avoided. while this global stuff doesn't really matter – Your Common Sense Aug 15 '13 at 13:03

1 Answers1

1

It's not best practice to use globals in this way, but it's correct syntax.

A better way would be to pass in mysqli as a parameter:

$mysqli = new mysqli([..]);

function my_cool_function($mysqli, $var) {
  //do work
  return;
}

echo my_cool_function($mysqli, $var);
phpisuber01
  • 7,585
  • 3
  • 22
  • 26