On Wed, Dec 26, 2012 at 4:57 PM, Hannes Magnusson < [email protected]> wrote:
> You know, these are very scary commits you are making. > In 6months, how on earth are people supposed to realize that specific > files on masterweb no longer use MQ, while others do? > > Why can't you simply check for magic quotes and accept their existance > if they are enabled? > And use ext/filter to target and escape most things? > You're right. They look scary because they undo everything magic quotes did, but they are limited to the code I changed. Obviously there is no easy way to see that unless you take a closer look at the code. You raise a very good concern and I'm not saying it isn't a concern. The problem, however, was that at the time that I wrote the code I had no idea the server was fully relying on magic quotes, because the code I updated was using addslashes directly. I removed the use of addslashes in that code and replaced it with mysql_real_escape_string functions instead. When I later went on to add more code (parts where I don't need to use GPC variables in the database queries) I didn't need escaping for the db there. So I only used the proper escaping methods where necessary. Once it was realized that magic_quotes was supposed to be on (something I was not aware of at the time) this broke my code entirely. To go back and check each variable that was being used and then filter it specifically for what I needed it would have created a diff of hundreds of lines of code (as the entire code had literally averted from MQ entirely). Instead I thought a smaller diff that just undid everything MQ did was more concise. I hope you can understand my concern in having to take the smarter solution into consideration over the overly complex solution since this issue was limited to the script files I either added or modified which are literally just 2 scripts on master right now (manage/user-notes.php and entry/user-notes-vote.php). The only other fix would have been to revert everything I did on master and start over from scratch with the assumption that magic_quotes is always on. Also, please note that my changes won't break the code base again even if MQ goes away. The ultimate goal is to fix everything on master to not depend on magic_quotes at all. However, that is going to be a much larger undertaking and will require a lot more careful consideration before any of those changes can be committed. > > -Hannes > > > On Wed, Dec 26, 2012 at 12:34 PM, Sherif Ramadan <[email protected]> > wrote: > > Commit: 07da9a04b2e17c72f15e74fe984b601a4707b812 > > Author: Sherif Ramadan <[email protected]> Wed, 26 Dec 2012 > 15:34:22 -0500 > > Parents: c2b7b05b1c237ecfd281f7acf5f1e8f908a7205b > > Branches: master > > > > Link: > http://git.php.net/?p=web/master.git;a=commitdiff;h=07da9a04b2e17c72f15e74fe984b601a4707b812 > > > > Log: > > Clean up GPC handling and move everything to undo_magic_quotes function > for portability. > > > > Changed paths: > > M entry/user-notes-vote.php > > M include/functions.inc > > M manage/user-notes.php > > > > > > Diff: > > diff --git a/entry/user-notes-vote.php b/entry/user-notes-vote.php > > index 04cfe54..bcaa62a 100644 > > --- a/entry/user-notes-vote.php > > +++ b/entry/user-notes-vote.php > > @@ -20,21 +20,49 @@ > > { "status": false, "message": "Invalid request..." } > > */ > > > > +undo_magic_quotes(); > > + > > /* > > - - Since filter.default is 'magic_quotes' I'm reverting to > filter_input with FILTER_UNSAFE_RAW as this was the original > > - assumption underwhich this code was written. The code continues to > use mysql_real_escape_string as opposed to > > - relying on magic_quotes and the addslashes/strip dependencies were > removed entirely from this code. > > - - This remains to be portable whether magic_quotes is set as > filter.default or not. > > - - Please use hscr() as opposed to clean() and real_clean() as opposed > to escape(). > > + This function will revert the GPCRS superglobals to their raw state > if the default.filter/magic_quotes is on. > > + Please do not use this function unless your code has no dependency > on magic_quotes and is properly escaping data. > > */ > > -foreach($_GET as $key => $val) $_GET[$key] = > filter_input(INPUT_GET,$key,FILTER_UNSAFE_RAW); > > -foreach($_POST as $key => $val) $_POST[$key] = > filter_input(INPUT_POST,$key,FILTER_UNSAFE_RAW); > > -foreach($_COOKIE as $key => $val) $_COOKIE[$key] = > filter_input(INPUT_COOKIE,$key,FILTER_UNSAFE_RAW); > > -foreach($_POST as $key => $val) $_REQUEST[$key] = > filter_input(INPUT_POST,$key,FILTER_UNSAFE_RAW); > > -foreach($_GET as $key => $val) $_REQUEST[$key] = > filter_input(INPUT_GET,$key,FILTER_UNSAFE_RAW); > > -foreach($_SERVER as $key => $val) $_SERVER[$key] = > filter_input(INPUT_SERVER,$key,FILTER_UNSAFE_RAW); > > - > > - > > +function undo_magic_quotes() { > > + if (!empty($_POST)) { > > + $args = array(); > > + foreach ($_POST as $key => $val) $args[$key] = array('filter' > => FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? > > + > FILTER_REQUIRE_ARRAY : FILTER_REQUIRE_SCALAR); > > + $_POST = filter_input_array(INPUT_POST, $args); > > + $_REQUEST = filter_input_array(INPUT_POST, $args); > > + } > > + if (!empty($_GET)) { > > + $args = array(); > > + foreach ($_GET as $key => $val) $args[$key] = array('filter' => > FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? > > + > FILTER_REQUIRE_ARRAY : FILTER_REQUIRE_SCALAR); > > + $_GET = filter_input_array(INPUT_GET, $args); > > + $_REQUEST += filter_input_array(INPUT_GET, $args); > > + } > > + if (!empty($_COOKIE)) { > > + $args = array(); > > + foreach ($_COOKIE as $key => $val) $args[$key] = array('filter' > => FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? > > + > FILTER_REQUIRE_ARRAY : FILTER_REQUIRE_SCALAR); > > + $_COOKIE = filter_input_array(INPUT_COOKIE, $args); > > + $_REQUEST += filter_input_array(INPUT_COOKIE, $args); > > + } > > + if (!empty($_SERVER)) { > > + $args = array(); > > + $append = array(); > > + foreach ($_SERVER as $key => $val) { > > + if ($key == 'REQUEST_TIME' || $key == 'REQUEST_TIME_FLOAT') > { > > + $append[$key] = $val; > > + continue; > > + } > > + $args[$key] = array('filter' => FILTER_UNSAFE_RAW, 'flags' > => is_array($val) ? > > + FILTER_REQUIRE_ARRAY : > FILTER_REQUIRE_SCALAR); > > + } > > + $_SERVER = filter_input_array(INPUT_SERVER, $args); > > + $_SERVER += $append; > > + } > > +} > > > > // Validate that the request to vote on a user note is OK (ip limits, > post variables, and db info must pass validation) > > function vote_validate_request(PDO $dbh) { > > diff --git a/include/functions.inc b/include/functions.inc > > index acf3adb..7885478 100644 > > --- a/include/functions.inc > > +++ b/include/functions.inc > > @@ -252,3 +252,50 @@ function > get_extension_info($mirror_hostname,$ext=null) { > > > > // We use markdown for people profiles > > include_once dirname(__FILE__) . > '/../vendor/michelf/php-markdown-extra/markdown.php'; > > + > > + > > +// > ----------------------------------------------------------------------------------- > > + > > +/* > > + This function will revert the GPCRS superglobals to their raw state > if the default.filter/magic_quotes is on. > > + Please do not use this function unless your code has no dependency > on magic_quotes and is properly escaping data. > > +*/ > > +function undo_magic_quotes() { > > + if (!empty($_POST)) { > > + $args = array(); > > + foreach ($_POST as $key => $val) $args[$key] = array('filter' > => FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? > > + > FILTER_REQUIRE_ARRAY : FILTER_REQUIRE_SCALAR); > > + $_POST = filter_input_array(INPUT_POST, $args); > > + $_REQUEST = filter_input_array(INPUT_POST, $args); > > + } > > + if (!empty($_GET)) { > > + $args = array(); > > + foreach ($_GET as $key => $val) $args[$key] = array('filter' => > FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? > > + > FILTER_REQUIRE_ARRAY : FILTER_REQUIRE_SCALAR); > > + $_GET = filter_input_array(INPUT_GET, $args); > > + $_REQUEST += filter_input_array(INPUT_GET, $args); > > + } > > + if (!empty($_COOKIE)) { > > + $args = array(); > > + foreach ($_COOKIE as $key => $val) $args[$key] = array('filter' > => FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? > > + > FILTER_REQUIRE_ARRAY : FILTER_REQUIRE_SCALAR); > > + $_COOKIE = filter_input_array(INPUT_COOKIE, $args); > > + $_REQUEST += filter_input_array(INPUT_COOKIE, $args); > > + } > > + if (!empty($_SERVER)) { > > + $args = array(); > > + $append = array(); > > + foreach ($_SERVER as $key => $val) { > > + if ($key == 'REQUEST_TIME' || $key == 'REQUEST_TIME_FLOAT') > { > > + $append[$key] = $val; > > + continue; > > + } > > + $args[$key] = array('filter' => FILTER_UNSAFE_RAW, 'flags' > => is_array($val) ? > > + FILTER_REQUIRE_ARRAY : > FILTER_REQUIRE_SCALAR); > > + } > > + $_SERVER = filter_input_array(INPUT_SERVER, $args); > > + $_SERVER += $append; > > + } > > +} > > + > > +// > ----------------------------------------------------------------------------------- > > diff --git a/manage/user-notes.php b/manage/user-notes.php > > index b5b6272..f05511d 100644 > > --- a/manage/user-notes.php > > +++ b/manage/user-notes.php > > @@ -7,22 +7,7 @@ include '../include/email-validation.inc'; > > include '../include/note-reasons.inc'; > > //require_once 'alert_lib.inc'; // remove comment if alerts are needed > > > > -/* > > - - Since filter.default is 'magic_quotes' I'm reverting to > filter_input with FILTER_UNSAFE_RAW as this was the original > > - assumption underwhich this code was rewritten. The code continues > to use mysql_real_escape_string as opposed to > > - relying on magic_quotes and the addslashes/strip dependencies were > removed entirely from this code. > > - - This remains to be portable whether magic_quotes is set as > filter.default or not. > > - - Please use hscr() as opposed to clean() and real_clean() as opposed > to escape(). > > -*/ > > -foreach($_GET as $key => $val) $_GET[$key] = > filter_input(INPUT_GET,$key,FILTER_UNSAFE_RAW); > > -$args = array(); > > -foreach($_POST as $key => $val) $args[$key] = array('filter' => > FILTER_UNSAFE_RAW, 'flags' => is_array($val) ? FILTER_REQUIRE_ARRAY : > FILTER_REQUIRE_SCALAR); > > -$_POST = filter_input_array(INPUT_POST, $args); > > -unset($args); > > -foreach($_COOKIE as $key => $val) $_COOKIE[$key] = > filter_input(INPUT_COOKIE,$key,FILTER_UNSAFE_RAW); > > -foreach($_POST as $key => $val) $_REQUEST[$key] = > filter_input(INPUT_POST,$key,FILTER_UNSAFE_RAW); > > -foreach($_GET as $key => $val) $_REQUEST[$key] = > filter_input(INPUT_GET,$key,FILTER_UNSAFE_RAW); > > -foreach($_SERVER as $key => $val) $_SERVER[$key] = > filter_input(INPUT_SERVER,$key,FILTER_UNSAFE_RAW); > > +undo_magic_quotes(); > > > > define("NOTES_MAIL", "[email protected]"); > > define("PHP_SELF", hsc($_SERVER['PHP_SELF'])); > > > > > > -- > > PHP Webmaster List Mailing List (http://www.php.net/) > > To unsubscribe, visit: http://www.php.net/unsub.php > > >
