why doesn't default values for this function work

[thoresson]

Hi,

I'm having problem with a function that I'll use to validate user input before passing it to MySQL. For strings, I want to make sure that they aren't to long, so I have written this function:

function secure_string($unsafe_string, $max_length = -1, $errormessage = "Too many characters.") */
{
    // verify that string isn't longer then $max_length, if $max_length is set
    if ($max_length > -1)
    {
        if (!is_int($max_length))
        {
            error("Variable max_length is not an integer.");
        }
        if (strlen($unsafe_string) > $max_length)
        {
            error($errormessage);
        }
    }
... and the validation will continue here. 

When I want to use the max length check I pass a value to the function like this:

$a_header = secure_string($_POST['a_header'], 60, "Header must not be more then 60 characters."); 

But I having to problems:
1) If no max length is passed, and $max_length gets the value -1, the if-loop if ($max_length > -1) is still run.
2) Calls to my own function error doesn't work. Instead of creating a popupwindow with javascript (which works in other places where error() is called) the errormessage is printed like html.

What's wrong?

Best regards,

Anders

[Helge]

1) If no max length is passed, and $max_length gets the value -1, the if-loop if ($max_length > -1) is still run.

Have you tried calling the function like this, when you don't want to pass a max length value?

$a_header = secure_string($_POST['a_header'], '', "Header must not be more then 60 characters."); 

Don't know if it makes any difference.

-Helge

[Helge]

Sorry. My suggestion above doesn't seem do work.

That means that you also have to specify the max length when you specify the errormessage in your function call (That means not using the default errormessage).

So when calling the function you need to do like:

$a_header = secure_string($_POST['a_header'], -1, "Header must not be more then 60 characters." ); 

Or not specify a new errormessage at all. Then you can just do

secure_string($_POST['a_header']); 

Another possibility is to change the order of the functions arguments

function secure_string($unsafe_string, $errormessage = "Too many characters.", $max_length = -1)


// Calling function

$a_header = secure_string($_POST['a_header'], "Header must not be more then 60 characters." ); 

Now, one of these will hopefully work!

-Helge

[brainpipe]

1) If no max length is passed, and $max_length gets the value -1, the if-loop if ($max_length > -1) is still run.

Can you give an example of how you are calling this function when no max length is passed? If you pass null, that if statement should evaluate to false; if you pass a string though, it will evaluate to true.

2) Calls to my own function error doesn't work. Instead of creating a popupwindow with javascript (which works in other places where error() is called) the errormessage is printed like html.

A code example would be helpful here as well, specifically the error() function and the code that calls it.

[thoresson]

Can you give an example of how you are calling this function when no max length is passed? If you pass null, that if statement should evaluate to false; if you pass a string though, it will evaluate to true.

With max_length I call it like this:

$a_header = secure_string($_POST['a_header'], 60, "Rubriken får inte vara mer än 60 tecken."); 

and without:

$a_desc = secure_string($_POST['a_desc']); 

A code example would be helpful here as well, specifically the error() function and the code that calls it.

My error() looks like this:

function error($msg) 
{
    ?>
    <SCRIPT language="JavaScript">
    <!--
    alert("<?=$msg?>");
    history.back();
    -->
    </SCRIPT>
    <?php
    exit;
}

In my secure_string() which looks like this, it doesn't work:

function secure_string($unsafe_string, $max_length = -1, $errormessage = "Du har skrivit för många tecken.") 
{
    // verify that string isn't longer then $max_length, if $max_length is set
    if ($max_length > -1)
    {
        if (!is_int($max_length))
        {
            error("Variabeln max_length är inte en siffra.");
        }
        if (strlen($unsafe_string) > $max_length)
        {
            error($errormessage);
        }
    }
    
    // create array containing bad words
    $badwords = array(";","--","select","drop","insert","xp_","delete");
    $goodwords = array(":","-","choose","leave","add"," ","remove");
    
    // check for occurences of $badwords
    for($i=0; $i<7; $i++) 
    {
        $unsafe_string = str_replace("$badwords[$i]", "$goodwords[$i]","$unsafe_string");
    }
    $unsafe_string = AddSlashes($unsafe_string);
    $unsafe_string = htmlentities($unsafe_string);
    $unsafe_string = strip_tags($unsafe_string);
    $unsafe_string = trim($unsafe_string);
    Return $unsafe_string;
} 

But in validate_email it works:

// validate entered email address

function validate_email($unchecked_email, $errortype = 1, $errormessage = "Du har inte skrivit in en giltlig e-postadress.") 
{
        if(!ereg("(^[a-zA-Z0-9_\.\-]+@[a-zA-Z0-9\-]+\.[a-zA-Z]{2,3}$)", $unchecked_email)) 
        {
            if($errortype == 1) 
            {
                error($errormessage);    
            }
            Return 1;
        }
} 

And by the way, are the last steps in secure_string needed or not (to make it secure for mysql, or could I trim it?

// create array containing bad words
    $badwords = array(";","--","select","drop","insert","xp_","delete");
    $goodwords = array(":","-","choose","leave","add"," ","remove");
    
    // check for occurences of $badwords
    for($i=0; $i<7; $i++) 
    {
        $unsafe_string = str_replace("$badwords[$i]", "$goodwords[$i]","$unsafe_string");
    }
    $unsafe_string = AddSlashes($unsafe_string);
    $unsafe_string = htmlentities($unsafe_string);
    $unsafe_string = strip_tags($unsafe_string);
    $unsafe_string = trim($unsafe_string);
    Return $unsafe_string; 

Thanks.

[brainpipe]

I'm not getting the same problem you are with that code. I'd suggest changing the $max_length parameter so it defaults to null:

function secure_string($unsafe_string, $max_length=null, $errormessage = "Du har skrivit för många tecken." ) 

In my secure_string() which looks like this, it doesn't work:

But in validate_email it works:

I'm at a loss here, I can't see anything that would cause this. Which version of PHP are you running?

And by the way, are the last steps in secure_string needed or not (to make it secure for mysql, or could I trim it?

Depends on what you're trying to do. I don't really see a need for strip_tags() here, but if you want to prevent any html from getting into the database (for whatever reason), then use it. You might play with the order in which you're calling those functions just to make sure you're getting the output you want.