Is this messy???

[completeamateur]

I am trying to rid my scripts of potential XSS/injection attacks.

 

Can anyone tell me if it is messy to nest functions like this...

 

$topicname = mysql_real_escape_string(strip_tags($_POST['topicname']));

 

...is there a better way?

 

Thanks in advance.

[clown[NOR]]

from my experience you cannot use mysql_real_escape_string() until after you have connected to the database... but yes... by using strip_tags and maybe stripslashes and also mysql_real_escape_string you shohuld be pretty safe when it comes to injections

[btherl]

I personally find nested functions difficult to read.  I would write it.

 

$topicname_stripped = strip_tags($_POST['topicname']);
$topicname_esc = mysql_real_escape_string($topicname_stripped);

 

It's an extra line, but the combined benefit of seperate variable names and a single function per line outweighs the increase in number of lines, in my opinion.  Plus, with this method you know that you must only use a "$varname_esc" in your mysql, and never the original "$varname".  But it's all a matter of preference.

 

A good exercise is to look over old code that you wrote and see which parts you can understand easily, and which parts take some effort.  Whatever is easy to understand, use that style more often.  Whatever is confusing, stop using that style

[clown[NOR]]

but then again... if you have let's say a registration form with maybe 10-15 fields... or maybe even more... then that 1 extra line will turn into much more

[kenrbnsn]

If you have a form with many fields, I would use a "foreach" loop with a "switch" statement. Something like this:

<?php
if (isset($_POST['submit']))
   foreach($_POST as $fld => $val)
       switch($fld) {
              case 'fld1':
//
//    validation for fld1
//
                   break;
              case 'fld2':
              case 'fld3':
//
//  group the validations for like fields togetther
// 
                    break;
//
//  etc...
//
     }
?>

 

Ken

[clown[NOR]]

hmm.. looks nice Ken.. maybe I should read up on switch