updating code for PHP 5

[camner]

I am taking an existing codebase (that I didn't write) and updating it for use with PHP 5.x...

The old code clearly took advantage of the fact that POST and GET data were automatically inserted into variables without use of the $_POST or $_GET arrays, which no longer works in PHP 5

The code also uses an html form that is submitted via javascriptdocument.frmproductlist.submit();
Now, here are my questions:

1. When a form is submitted this way, via javascript, the form fields are inserted into the $_REQUEST array, but not the $_POST array (nor the $_GET array). I have to admit I'm confused by this; why is this the case?

2. In order to get the form fields into the appropriate variables, I'm using extract($_REQUEST); Is there a better way?

3. In the old code, no provisions were made for escaping special characters such as ', " etc. as they were inserted into a database. So, I'm presuming that magic_quotes_gpc() must have been assumed to be on! This is poor programming practice, I know. I've read posts on this forum suggesting that addslashes()/stripslashes() is a poor way to code, and that mysql_real_escape_string() is preferable. It seems cumbersome to call that function for all strings to be inserted into the database...but even if that is done, how is the data "unescaped" when it is pulled out of the database?

Thanks for looking at this...

[Shrike]

First up, $_REQUEST should contain either the POST or GET data depending on which method was used to submit the request. There should always be either POST or GET if the request is made over HTTP - are you sure it's not there?

Second, writing extract( $_REQUEST ); is a huge security risk, with the potential for all sorts of nasty behaviour, for example:

$myvar = "something thats fine to write to database";
extract( $_REQUEST );
mysql_query( "INSERT INTO sometable '$myvar'" );

If some malicious user knew that the code looked like this, they could make a request of yourfile.php?myvar=evildata and overwrite your safe data. The behaviour of extract() can be changed in a few ways, but still use it with extreme caution. From the manual:

Do not use extract() on untrusted data, like user-input ($_GET, ...). If you do, for example, if you want to run old code that relies on register_globals temporarily, make sure you use one of the non-overwriting extract_type values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini.


Third, you should always use mysql_real_escape_string(), or the other database versions to escape data. The slashes and so forth just tell the database to treat the characters literally, and not as structural characters. They are written to the database in their original form. So you need to remember to escape them again when displaying on a web page. htmlentities() usually does the trick here.

[camner]

Thank you for your clear explanations. I have read elsewhere of the perils of "extract", and your example clearly explains why.

I will have to do some more digging, and perhaps create a simpler test script that just submits a form via javascript. I AM sure in the script as presently written that if I dump $_POST (the form is submitted via POST using javascript) I don't get the form variables, but extracting from $_REQUEST I do, which I know is puzzling and doesn't really make sense.

And thank you for your explanation of the use of escaping....somehow I had gotten into my head that the text actually stored in the database was escaped, whereas what's really going on is that the escaping is for the purpose of building the query and not fouling up the syntax of the query.