Function - need a second set of eyes -

[webdev007]

I am putting together a one-size-fits-all function

could you give it a look and let me know about possible errors/flaws/mistake
thank you

function CleanDb($value)
{
// Stripslashes
if (get_magic_quotes_gpc()) {
$value = stripslashes($value);
}
else
if (is_numeric($value))
{
$value=$value;
}

// if not a number or a numeric string
else
{
$value = "'" . mysql_real_escape_string($value) . "'";
}
return $value;
}

 

[jpadie]

you're not handling arrays in your code. i'd also recommend applying trim() to the variable before you escape them.

 

[webdev007]

Correct
Good advise
thanks

 

[vacunita]

This is totally pointless:

if (is_numeric($value))
   {
[red]    $value=$value; [/red]
  }

Why not set a flag or something you can check, In case you are expecting a number or something. assigning the variable to itself is redundant, is of no use, and might cause confusion down the line.

Or change your code, to not require the else statement:

if ([red]![/red]is_numeric($value))
   {
 $value = "'" . mysql_real_escape_string($value) . "'";
  }

----------------------------------
Ignorance is not necessarily Bliss, case in point:
Unknown has caused an Unknown Error on Unknown and must be shutdown to prevent damage to Unknown.

 

[jpadie]

@vacunita

your suggestion would not deal with arrays/objects/resources etc. but i agree that an assignment of a variable to itself is a bit pointless.

something like this might be better perhaps

function cleanDB($var){
 

 $type = gettype($var);
 switch ($type) {
  
   case "integer":
   case "boolean":
   case "float":
   case "double":
    return $var;
    break;
   case "string":
     if (get_magic_quotes_gpc()) {
       $var = stripslashes($var);
     }
     return '"'.mysql_escape_string(trim($var)).'"';
     break;
   case array:
   case object:
     return '"'.mysql_escape_string(serialize($var)).'"';
     break;
   default:
     return false;
 }
}

 

[vacunita]

I never claimed it did, I was only commenting on the existing code. But I agree, var type handling including arrays should also be performed.

----------------------------------
Ignorance is not necessarily Bliss, case in point:
Unknown has caused an Unknown Error on Unknown and must be shutdown to prevent damage to Unknown.

 

[webdev007]

Point well taken
Thank you for showing the way!

 

[Itshim]

From the name of your function it appears as if you are planning on using it as a security measure for SQL statements, and not just a function to quote strings. If this is the case, then I would rework the function to accept a second parameter 'type'. Since the variable passed into the function will be part of an SQL statement you should know what the variable type is supposed to be.

From a security standpoint, it is always better to check that a value is what you expect, instead of trying to dynamically figure out what the value is and have the program act accordingly.

As for the validation, personally I would use type casting, it will be faster than any function.

 

[webdev007]

ITshim,
thanks, this is really the very last step before insert/update
I have already a class that includes:
IP check and not accepting any other than coming from the States (just because the site is only meant for US users)
A bad word check
A bunch of regex checking well formed email and other input
I even check if input coming from DD box are the expected one!

However my last step comes out of trying to avoid typing too many times mysql_real_escape_string()

BTW
if anyone tries the above function
the last part of the function (2 last cases)
results in SQl errors
I tried a whole bunch of modif without luck!
so I did not use those last two and went for regular if..
Strange it's adding a whole set of double quotes

 

[jpadie]

the code is fine (if you are referring to mine) i suspect the error is caused by the wrong column type for receiving the data.

in specie, i agree with itshim: you should validate incoming data against what you expect it should be (normally as defined by your column type, but also, for example, that a value deriving from a select box must actually have been one of the options in that select box).

 

[webdev007]

jpadie, you are recommending my posted set of steps
this is exactly what I do.

Some cols are char, varchar, int and text
size goes from 2 to text
DB is MyISAM

input is either $_POST or $_SESSION

thanks