is urlencode() save enough for a mysql insert?

[OctoberWind]

switch($_GET['form_type']) {
case "make_model":
$header = urlencode($_GET['header']);
$make = urlencode($_GET['make']);
$model = urlencode($_GET['model']);
        
$query = "INSERT INTO `table` (`id`, `timeDate`, `ip`, `formType`, `header`, `make`, `model`, `equipmentType`, `location`) 
                VALUES ('', NOW(), '{$_SERVER['REMOTE_ADDR']}', '{$_GET['form_type']}', '$header', '$make', '$model', '', '')";
                
//echo $query;
$result = mysql_query($query, $link) or die();
header("Location: http://www.website.com/results.aspx?header={$header}&make={$make}&model={$model}");
        break;
// three other cases: 
default:
header("Location: http://www.website.com/"); 

Ignore the aspx file. All I need to do is rebuild the url string and re-forward the url to website.com/results.aspx with the specific GET variables.

The form is generated via javascript, all I'm doing is intercepting the process to store the data that's being sent. I need urlencode() to take care of the spaces and commas when I resent the header().

What's happening is, the data is being written as "Freightliner+CENTURY" or "Phoenix%2C+AZ" in the database. Do I need (or rather, should I also) mysql_real_escape_string() before/during the INSERT statement?

[phoenixbytes]

here's what i'd do:

switch($_GET['form_type']) { 
case "make_model": 
// get them in their purest form
//
$header = $_GET['header']; 
$make = $_GET['make']; 
$model = $_GET['model']; 
         


// throw them into the database
//
$query = "INSERT INTO `table` (`id`, `timeDate`, `ip`, `formType`, `header`, `make`, `model`, `equipmentType`, `location`) 
                VALUES ('', NOW(), '{$_SERVER['REMOTE_ADDR']}', '{$_GET['form_type']}', '$header', '$make', '$model', '', '')"; 
          


// then, and only then, would i urlencode them
//
$header = urlencode("$header");
$make = urlencode("$make");
$model = urlencode("$model");
//
// then send them on their merry way
//
//
//

// actually processing the query here shouldn't make a difference,
// if it does though, just move it to a place above where we called urlencode()

$result = mysql_query($query, $link) or die(); 
header("Location: http://www.website.com/results.aspx?header={$header}&make={$make}&model={$model}"); 
        break; 
// three other cases: 
default: 
header("Location: http://www.website.com/"); 

good luck!

[NogDog]

It will not hurt to run the encoded string through mysql_real_escape_string(), so I would to be on the safe side.

[phoenixbytes]

ok, try this then:

switch($_GET['form_type']) { 
case "make_model": 
// get them in their purest form 
// 
$header = mysql_real_escape_string($_GET['header']); 
$make = mysql_real_escape_string($_GET['make']); 
$model = mysql_real_escape_string($_GET['model']); 
          


// throw them into the database 
// 
$query = "INSERT INTO `table` (`id`, `timeDate`, `ip`, `formType`, `header`, `make`, `model`, `equipmentType`, `location`) 
                VALUES ('', NOW(), '{$_SERVER['REMOTE_ADDR']}', '{$_GET['form_type']}', '$header', '$make', '$model', '', '')"; 
           
// do query
//
$result = mysql_query($query, $link) or die(); 




// then, and only then, would i urlencode them 
// 
$header = urlencode("$header"); 
$make = urlencode("$make"); 
$model = urlencode("$model"); 
// 
// then send them on their merry way 
// 
// 
// 
header("Location: http://www.website.com/results.aspx?header={$header}&make={$make}&model={$model}"); 
        break; 
// three other cases: 
default: 
header("Location: http://www.website.com/"); 

that should work lovely.

[NogDog]

@phoenixbytes: I was responding more to the original post than yours, but I would probably do essentially as you suggest: store the raw data in the database, remembering to always escape user input (mysql_real_escape_string()), then urlencode() it as and when necessary as it is being output, making the actual data more portable and easier to search, with the added bonus of taking up a bit less space in the DB.

[Mindzai]

Bear in mind that you should treat ALL external data as suspicious, and that includes the $_SERVER superglobal. While it is admittedly unlikely that anyone will be able to tamper with it to any great extent, especially in this case, it is better to be safe than sorry.

[OctoberWind]

Thanks guys, and Mindzai for the tip/reminder about the $_SERVER data.

revised:

case "make_model":
        $header = mysql_real_escape_string($_GET['header']);
        $make = mysql_real_escape_string($_GET['make']);
        $model = mysql_real_escape_string($_GET['model']);
        
        $query = "INSERT INTO `table` (`id`, `timeDate`, `ip`, `formType`, `header`, `make`, `model`, `equipmentType`, `location` ) 
                VALUES ('', NOW(), '".mysql_real_escape_string($_SERVER['REMOTE_ADDR'])."', '".mysql_real_escape_string($_GET['form_type'])."', '$header', '$make', '$model', '', '' )";
        $result = mysql_query($query, $link) or die();
        
        $header = urlencode($_GET['header']);
        $make = urlencode($_GET['make']);
        $model = urlencode($_GET['model']);
        
        header("Location: http://www.website.com/docs/search.results.aspx?header={$header}&make={$make}&model={$model}");
        break; 

Next Question: someone mentioned to me that I could/should be incorporating a 301 on the header(Location), but because it's essentially a form submission and it's a 'dynamic' link with GET variable, does that rule not apply?

[phoenixbytes]

Thanks guys, and Mindzai for the tip/reminder about the $_SERVER data.

revised:

case "make_model":
        $header = mysql_real_escape_string($_GET['header']);
        $make = mysql_real_escape_string($_GET['make']);
        $model = mysql_real_escape_string($_GET['model']);
        
        $query = "INSERT INTO `table` (`id`, `timeDate`, `ip`, `formType`, `header`, `make`, `model`, `equipmentType`, `location` ) 
                VALUES ('', NOW(), '".mysql_real_escape_string($_SERVER['REMOTE_ADDR'])."', '".mysql_real_escape_string($_GET['form_type'])."', '$header', '$make', '$model', '', '' )";
        $result = mysql_query($query, $link) or die();
        
        $header = urlencode($_GET['header']);
        $make = urlencode($_GET['make']);
        $model = urlencode($_GET['model']);
        
        header("Location: http://www.website.com/docs/search.results.aspx?header={$header}&make={$make}&model={$model}");
        break; 

Next Question: someone mentioned to me that I could/should be incorporating a 301 on the header(Location), but because it's essentially a form submission and it's a 'dynamic' link with GET variable, does that rule not apply?

i'd always declare the header for completeness/compatibility reasons, and i'd always

exit; 

afterwards.

[Mindzai]

You should not use a 301 status code unless you want to indicate that the resource has been permanently moved, which I assume in this case it has not.