9d4eaaf6c763c9fc01a9356ef58dbe72

This is the function I've been using to clean up user input.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<?php
function makeSafe($variable, $escape=true) {
	$variable = htmlentities($variable, ENT_QUOTES);
  	if (get_magic_quotes_gpc()) 
	{ 
	   $variable = stripslashes($variable); 
	}
	if ($escape==true){
  		$variable = mysql_escape_string(trim($variable));
	}
 	$variable = strip_tags($variable);
	$variable = str_replace("\r\n", "", $variable);
 	return $variable;
}
?>

Refactorings

No refactoring yet !

3fd9b06332a078b20711de31074ea2f6

Matt Farina

October 2, 2007, October 02, 2007 11:27, permalink

No rating. Login to rate!

Cleaned up code.... spacing to pear standards.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
<?php
function makeSafe($variable, $escape = true) {
    $variable = htmlentities($variable, ENT_QUOTES);
    if (get_magic_quotes_gpc()) { 
        $variable = stripslashes($variable); 
    }
    if ($escape == true){
        $variable = mysql_escape_string(trim($variable));
    }
    $variable = strip_tags($variable);
    $variable = str_replace("\r\n", "", $variable);
    return $variable;
}
?>
F196cc1d57ca5b0f58efb3254925e6a3

Edward Z. Yang

October 2, 2007, October 02, 2007 19:51, permalink

No rating. Login to rate!

The order of the procedures in this function is completely wrong. First, you get the variable into a "pure" state, which involves undoing magic quotes. Then, you perform your filtering on the variable, in your case strip_tags. Then, you encode for the output format: that's htmlentities and possibly mysql_real_escape_string. These procedures should not all be in one function.

A2c8fecfd1fb707dd0a8f292ade77e1e

typefreak

October 3, 2007, October 03, 2007 03:47, permalink

No rating. Login to rate!

if ($escape==true) equals if ($escape)

What if the input uses "\n" instead of "\r\n" ? (won't be replaced)

Also, if you use htmlentities, < and > are translated to &lt; and &gt;, so strip-tags won't be of any use. (As far As I know)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<?php
function makeSafe($variable, $escape=true) {
  if (get_magic_quotes_gpc()) { 
    $variable = stripslashes($variable); 
  }

  $variable = htmlentities($variable, ENT_QUOTES);
  $variable = str_replace("\r\n", "", $variable);

  if ($escape) {
    $variable = mysql_escape_string(trim($variable));
  }
  return $variable;
}
?>
584799d026024e108d87aeceb51804d3

JWvdV

October 3, 2007, October 03, 2007 09:40, permalink

2 ratings. Login to rate!

For what the variable should be escaped? For mysql I suggest, because of your topicstart :D
Variables should not be htmlentited before saving it in a database. It should be done when you need it for echoing or anything like that. Who's saying you're only using data for the web? I should separate variables used for saving in a DB and variable that are used for on the web.
For what sake are you trimming your variables? You can also think about implementing a function that's rewriting spaces to &nbsp; and tabs to eight &nbsp;-s

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
<?php
/* First of all we are going to clean up our input, if magic_quotes is On
 * So we don't have to find out whether magic_quotes is on, each time we need some input
 */
if(get_magic_quotes_gpc()){
	function striparrayslashes(&$array){
		if(is_array($array)){
			foreach($array as $index => $value){
				if(is_string($value)) $array[$index] = stripslashes($value);
				else if(is_array($value)) $array[$index] = striparrayslashes($value);
			}
		}
	}

	striparrayslashes($_POST);
	striparrayslashes($_GET);
	striparrayslashes($_COOKIE);
	striparrayslashes($_REQUEST);
}

function makeSafe($variable, $escape=true){
	if(!is_string($variable)) return false;
	if($escape) mysql_escape_string($variable);
}
584799d026024e108d87aeceb51804d3

JWvdV

October 4, 2007, October 04, 2007 07:30, permalink

1 rating. Login to rate!

@Permalink:
For what sake you are using brackets if it's on one line? Also some more bullshut could be cut (but is it functional to cut code?)...
As earlier mentioned in this topic: htmlentities('<>') => &lt;&gt;, so strip_tags cannot be used...

But i didn't change my mind in using htmlentities in your database (should not be done) and trimming space for database (only if space can't have a function)...

1
2
3
4
5
6
<?php
function makeSafe($v,$e=1){
	$v = htmlentities((get_magic_quotes_gpc()? stripslashes($v) : $v), ENT_QUOTES);
 	return str_replace("\r\n", "", ($e? mysql_escape_string($v) : $v));
}
?>
08b49e3d543f221ed51cdbe118448275

zaadjis

October 10, 2007, October 10, 2007 14:02, permalink

No rating. Login to rate!

Don't http://en.wikipedia.org/wiki/Reinvent_the_wheel

1
2
3
4
5
6
7
8
9
10
11
<?php

/**
 * @link http://php.net/filter
 * @link http://devzone.zend.com/node/view/id/1113
 * @link http://phpro.org/tutorials/Filtering-Data-with-PHP.html
 */

define ('MY_FILTERS', FILTER_SANITIZE_STRING | FILTER_SANITIZE_MAGIC_QUOTES | etc....);

$sanitizedUserInput = filter_var($userInput, MY_FILTERS);
347a959d16ea34efea0461d824ebce09

Mike Weller

October 11, 2007, October 11, 2007 15:04, permalink

No rating. Login to rate!

Some of the code I see around here makes me scared. And people are still making the mistake of adding data to the DB in html-escaped form, which IMO amounts to data corruption.

First, disable or undo magic quotes at the start of your application, i.e. convert GET/POST etc to their pure forms.

When inserting into a db, use ONLY a sql escaping function like mysql_escape_string. DO NOT use htmlentities for data that is heading to the database. In the code above, people have combined sql and html escaping into the same function, this just doesn't work.

The data in your database is not HTML, but that's what you are making it. What happens if you want to get data in a plain format? You have to SELECT it out, then html decode it. This is just wrong.

So for database data, sql escape only.

Then, when you need to output to html, use htmlentities.

Mike

Your refactoring





Format Copy from initial code

or Cancel