From d7e52ee0f8fb876161ac9705f4347d6431657358 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 6 Mar 2010 05:40:38 +0100 Subject: [PATCH 01/21] [feature/request-class] Adding a request class based on ascraeus-experiment. The well known request_var function is now a wrapper that calls a method on a phpbb_request object. The class provides additional functionality. It can replace all super globals with special objects that throw errors when being accessed. They still allow isset operations to keep backward compatibility with isset($_POST['var']) checks. The phpbb_request class implements the phpbb_request_interface which is available for easy mocking of input in tests. PHPBB3-9716 --- phpBB/includes/functions.php | 134 ++---- .../request/deactivated_super_global.php | 121 +++++ phpBB/includes/request/request.php | 412 ++++++++++++++++++ phpBB/includes/request/request_interface.php | 103 +++++ tests/request/all_tests.php | 4 + tests/request/deactivated_super_global.php | 25 ++ tests/request/request.php | 89 ++++ tests/request/request_class.php | 74 ---- tests/request/request_var.php | 3 + 9 files changed, 790 insertions(+), 175 deletions(-) create mode 100644 phpBB/includes/request/deactivated_super_global.php create mode 100644 phpBB/includes/request/request.php create mode 100644 phpBB/includes/request/request_interface.php create mode 100644 tests/request/deactivated_super_global.php create mode 100644 tests/request/request.php delete mode 100644 tests/request/request_class.php diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 1c5e0b63a1..485f5487cc 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -19,121 +19,53 @@ if (!defined('IN_PHPBB')) // Common global functions /** -* set_var +* Wrapper function of phpbb_request::variable which exists for backwards compatability. +* See {@link phpbb_request_interface::variable phpbb_request_interface::variable} for +* documentation of this function's use. * -* Set variable, used by {@link request_var the request_var function} +* @param mixed $var_name The form variable's name from which data shall be retrieved. +* If the value is an array this may be an array of indizes which will give +* direct access to a value at any depth. E.g. if the value of "var" is array(1 => "a") +* then specifying array("var", 1) as the name will return "a". +* If you pass an instance of {@link phpbb_request_interface phpbb_request_interface} +* as this parameter it will overwrite the current request class instance. If you do +* not do so, it will create its own instance (but leave superglobals enabled). +* @param mixed $default A default value that is returned if the variable was not set. +* This function will always return a value of the same type as the default. +* @param bool $multibyte If $default is a string this paramater has to be true if the variable may contain any UTF-8 characters +* Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks +* @param bool $cookie This param is mapped to phpbb_request_interface::COOKIE as the last param for +* phpbb_request_interface::variable for backwards compatability reasons. * -* @access private -*/ -function set_var(&$result, $var, $type, $multibyte = false) -{ - settype($var, $type); - $result = $var; - - if ($type == 'string') - { - $result = trim(htmlspecialchars(str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result), ENT_COMPAT, 'UTF-8')); - - if (!empty($result)) - { - // Make sure multibyte characters are wellformed - if ($multibyte) - { - if (!preg_match('/^./u', $result)) - { - $result = ''; - } - } - else - { - // no multibyte, allow only ASCII (0-127) - $result = preg_replace('/[\x80-\xFF]/', '?', $result); - } - } - - $result = (STRIP) ? stripslashes($result) : $result; - } -} - -/** -* request_var -* -* Used to get passed variable +* @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the +* the same as that of $default. If the variable is not set $default is returned. */ function request_var($var_name, $default, $multibyte = false, $cookie = false) { - if (!$cookie && isset($_COOKIE[$var_name])) + // This is all just an ugly hack to add "Dependency Injection" to a function + // the only real code is the function call which maps this function to a method. + static $request = null; + + if ($var_name instanceof phpbb_request_interface) { - if (!isset($_GET[$var_name]) && !isset($_POST[$var_name])) - { - return (is_array($default)) ? array() : $default; - } - $_REQUEST[$var_name] = isset($_POST[$var_name]) ? $_POST[$var_name] : $_GET[$var_name]; + $request = $var_name; } - $super_global = ($cookie) ? '_COOKIE' : '_REQUEST'; - if (!isset($GLOBALS[$super_global][$var_name]) || is_array($GLOBALS[$super_global][$var_name]) != is_array($default)) + // no request class set, create a temporary one ourselves to keep backwards compatability + if ($request === null) { - return (is_array($default)) ? array() : $default; - } - - $var = $GLOBALS[$super_global][$var_name]; - if (!is_array($default)) - { - $type = gettype($default); + $tmp_request = new phpbb_request(); + // enable super globals, so the magically created request class does not + // make super globals inaccessible everywhere outside this function. + $tmp_request->enable_super_globals(); } else { - list($key_type, $type) = each($default); - $type = gettype($type); - $key_type = gettype($key_type); - if ($type == 'array') - { - reset($default); - $default = current($default); - list($sub_key_type, $sub_type) = each($default); - $sub_type = gettype($sub_type); - $sub_type = ($sub_type == 'array') ? 'NULL' : $sub_type; - $sub_key_type = gettype($sub_key_type); - } + // otherwise use the static injected instance + $tmp_request = $request; } - if (is_array($var)) - { - $_var = $var; - $var = array(); - - foreach ($_var as $k => $v) - { - set_var($k, $k, $key_type); - if ($type == 'array' && is_array($v)) - { - foreach ($v as $_k => $_v) - { - if (is_array($_v)) - { - $_v = null; - } - set_var($_k, $_k, $sub_key_type, $multibyte); - set_var($var[$k][$_k], $_v, $sub_type, $multibyte); - } - } - else - { - if ($type == 'array' || is_array($v)) - { - $v = null; - } - set_var($var[$k], $v, $type, $multibyte); - } - } - } - else - { - set_var($var, $var, $type, $multibyte); - } - - return $var; + return $tmp_request->variable($var_name, $default, $multibyte, ($cookie) ? phpbb_request_interface::COOKIE : phpbb_request_interface::REQUEST); } /** diff --git a/phpBB/includes/request/deactivated_super_global.php b/phpBB/includes/request/deactivated_super_global.php new file mode 100644 index 0000000000..9152d10811 --- /dev/null +++ b/phpBB/includes/request/deactivated_super_global.php @@ -0,0 +1,121 @@ +request = $request; + $this->name = $name; + $this->super_global = $super_global; + } + + /** + * Calls trigger_error with the file and line number the super global was used in. + */ + private function error() + { + $file = ''; + $line = 0; + + $message = 'Illegal use of $' . $this->name . '. You must use the request class or request_var() to access input data. Found in %s on line %d. This error message was generated'; + + $backtrace = debug_backtrace(); + if (isset($backtrace[1])) + { + $file = $backtrace[1]['file']; + $line = $backtrace[1]['line']; + } + trigger_error(sprintf($message, $file, $line), E_USER_ERROR); + } + + /** + * Redirects isset to the correct request class call. + * + * @param string $offset The key of the super global being accessed. + * + * @return bool Whether the key on the super global exists. + */ + public function offsetExists($offset) + { + return $this->request->is_set($offset, $this->super_global); + } + + /**#@+ + * Part of the ArrayAccess implementation, will always result in a FATAL error. + */ + public function offsetGet($offset) + { + $this->error(); + } + + public function offsetSet($offset, $value) + { + $this->error(); + } + + public function offsetUnset($offset) + { + $this->error(); + } + /**#@-*/ + + /** + * Part of the Countable implementation, will always result in a FATAL error + */ + public function count() + { + $this->error(); + } + + /** + * Part of the Traversable/IteratorAggregate implementation, will always result in a FATAL error + */ + public function getIterator() + { + $this->error(); + } +} + diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php new file mode 100644 index 0000000000..dbed95546f --- /dev/null +++ b/phpBB/includes/request/request.php @@ -0,0 +1,412 @@ + '_POST', + phpbb_request_interface::GET => '_GET', + phpbb_request_interface::REQUEST => '_REQUEST', + phpbb_request_interface::COOKIE => '_COOKIE' + ); + + /** + * @var array Stores original contents of $_REQUEST array. + */ + protected $original_request = null; + + /** + * @var + */ + protected $super_globals_disabled = false; + + /** + * @var array An associative array that has the value of super global constants as keys and holds their data as values. + */ + protected $input; + + /** + * @var string Whether slashes need to be stripped from input + */ + protected $strip; + + /** + * Initialises the request class, that means it stores all input data in {@link $input input} + * and then calls {@link phpbb_deactivated_super_global phpbb_deactivated_super_global} + */ + public function __construct() + { + if (version_compare(PHP_VERSION, '6.0.0-dev', '>=')) + { + $this->strip = false; + } + else + { + $this->strip = (@get_magic_quotes_gpc()) ? true : false; + } + + foreach (self::$super_globals as $const => $super_global) + { + $this->input[$const] = isset($GLOBALS[$super_global]) ? $GLOBALS[$super_global] : array(); + } + + // simulate request_order = GP + $this->original_request = $this->input[phpbb_request_interface::REQUEST]; + $this->input[phpbb_request_interface::REQUEST] = $this->input[phpbb_request_interface::POST] + $this->input[phpbb_request_interface::GET]; + + $this->disable_super_globals(); + } + + /** + * Getter for $super_globals_disabled + * + * @return bool Whether super globals are disabled or not. + */ + public function super_globals_disabled() + { + return $this->super_globals_disabled; + } + + /** + * Disables access of super globals specified in $super_globals. + * This is achieved by overwriting the super globals with instances of {@link phpbb_deactivated_super_global phpbb_deactivated_super_global} + */ + public function disable_super_globals() + { + if (!$this->super_globals_disabled) + { + foreach (self::$super_globals as $const => $super_global) + { + unset($GLOBALS[$super_global]); + $GLOBALS[$super_global] = new phpbb_deactivated_super_global($this, $super_global, $const); + } + + $this->super_globals_disabled = true; + } + } + + /** + * Enables access of super globals specified in $super_globals if they were disabled by {@link disable_super_globals disable_super_globals}. + * This is achieved by making the super globals point to the data stored within this class in {@link $input input}. + */ + public function enable_super_globals() + { + if ($this->super_globals_disabled) + { + foreach (self::$super_globals as $const => $super_global) + { + $GLOBALS[$super_global] = $this->input[$const]; + } + + $GLOBALS['_REQUEST'] = $this->original_request; + + $this->super_globals_disabled = false; + } + } + + /** + * Recursively applies addslashes to a variable. + * + * @param mixed &$var Variable passed by reference to which slashes will be added. + */ + public static function addslashes_recursively(&$var) + { + if (is_string($var)) + { + $var = addslashes($var); + } + else if (is_array($var)) + { + $var_copy = $var; + $var = array(); + foreach ($var_copy as $key => $value) + { + if (is_string($key)) + { + $key = addslashes($key); + } + $var[$key] = $value; + + self::addslashes_recursively($var[$key]); + } + } + } + + /** + * This function allows overwriting or setting a value in one of the super global arrays. + * + * Changes which are performed on the super globals directly will not have any effect on the results of + * other methods this class provides. Using this function should be avoided if possible! It will + * consume twice the the amount of memory of the value + * + * @param string $var_name The name of the variable that shall be overwritten + * @param mixed $value The value which the variable shall contain. + * If this is null the variable will be unset. + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies which super global shall be changed + */ + public function overwrite($var_name, $value, $super_global = phpbb_request_interface::REQUEST) + { + if (!isset(self::$super_globals[$super_global])) + { + return; + } + + if ($this->strip) + { + self::addslashes_recursively($value); + } + + // setting to null means unsetting + if ($value === null) + { + unset($this->input[$super_global][$var_name]); + if (!$this->super_globals_disabled()) + { + unset($GLOBALS[self::$super_globals[$super_global]][$var_name]); + } + } + else + { + $this->input[$super_global][$var_name] = $value; + if (!self::super_globals_disabled()) + { + $GLOBALS[self::$super_globals[$super_global]][$var_name] = $value; + } + } + + if (!self::super_globals_disabled()) + { + unset($GLOBALS[self::$super_globals[$super_global]][$var_name]); + $GLOBALS[self::$super_globals[$super_global]][$var_name] = $value; + } + } + + /** + * Set variable $result to a particular type. + * + * @param mixed &$result The variable to fill + * @param mixed $var The contents to fill with + * @param mixed $type The variable type. Will be used with {@link settype()} + * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. + * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks. + */ + public function set_var(&$result, $var, $type, $multibyte = false) + { + settype($var, $type); + $result = $var; + + if ($type == 'string') + { + $result = trim(htmlspecialchars(str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result), ENT_COMPAT, 'UTF-8')); + + if (!empty($result)) + { + // Make sure multibyte characters are wellformed + if ($multibyte) + { + if (!preg_match('/^./u', $result)) + { + $result = ''; + } + } + else + { + // no multibyte, allow only ASCII (0-127) + $result = preg_replace('/[\x80-\xFF]/', '?', $result); + } + } + + $result = ($this->strip) ? stripslashes($result) : $result; + } + } + + /** + * Recursively sets a variable to a given type using {@link set_var set_var} + * This function is only used from within {@link phpbb_request::variable phpbb_request::variable}. + * + * @param string $var The value which shall be sanitised (passed by reference). + * @param mixed $default Specifies the type $var shall have. + * If it is an array and $var is not one, then an empty array is returned. + * Otherwise var is cast to the same type, and if $default is an array all + * keys and values are cast recursively using this function too. + * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. + * Default is false, causing all bytes outside the ASCII range (0-127) to + * be replaced with question marks. + */ + protected function recursive_set_var(&$var, $default, $multibyte) + { + if (is_array($var) !== is_array($default)) + { + $var = (is_array($default)) ? array() : $default; + return; + } + + if (!is_array($default)) + { + $type = gettype($default); + $this->set_var($var, $var, $type, $multibyte); + } + else + { + // make sure there is at least one key/value pair to use get the + // types from + if (empty($default)) + { + $var = array(); + return; + } + + list($default_key, $default_value) = each($default); + $value_type = gettype($default_value); + $key_type = gettype($default_key); + + $_var = $var; + $var = array(); + + foreach ($_var as $k => $v) + { + $this->set_var($k, $k, $key_type, $multibyte, $multibyte); + + $this->recursive_set_var($v, $default_value, $multibyte); + $this->set_var($var[$k], $v, $value_type, $multibyte); + } + } + } + + /** + * Central type safe input handling function. + * All variables in GET or POST requests should be retrieved through this function to maximise security. + * + * @param string|array $var_name The form variable's name from which data shall be retrieved. + * If the value is an array this may be an array of indizes which will give + * direct access to a value at any depth. E.g. if the value of "var" is array(1 => "a") + * then specifying array("var", 1) as the name will return "a". + * @param mixed $default A default value that is returned if the variable was not set. + * This function will always return a value of the same type as the default. + * @param bool $multibyte If $default is a string this paramater has to be true if the variable may contain any UTF-8 characters + * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies which super global should be used + * + * @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the + * the same as that of $default. If the variable is not set $default is returned. + */ + public function variable($var_name, $default, $multibyte = false, $super_global = phpbb_request_interface::REQUEST) + { + $path = false; + + // deep direct access to multi dimensional arrays + if (is_array($var_name)) + { + $path = $var_name; + // make sure at least the variable name is specified + if (empty($path)) + { + return (is_array($default)) ? array() : $default; + } + // the variable name is the first element on the path + $var_name = array_shift($path); + } + + if (!isset($this->input[$super_global][$var_name])) + { + return (is_array($default)) ? array() : $default; + } + $var = $this->input[$super_global][$var_name]; + + if ($path) + { + // walk through the array structure and find the element we are looking for + foreach ($path as $key) + { + if (is_array($var) && isset($var[$key])) + { + $var = $var[$key]; + } + else + { + return (is_array($default)) ? array() : $default; + } + } + } + + self::recursive_set_var($var, $default, $multibyte); + + return $var; + } + + /** + * Checks whether a certain variable was sent via POST. + * To make sure that a request was sent using POST you should call this function + * on at least one variable. + * + * @param string $name The name of the form variable which should have a + * _p suffix to indicate the check in the code that creates the form too. + * + * @return bool True if the variable was set in a POST request, false otherwise. + */ + public function is_set_post($name) + { + return $this->is_set($name, phpbb_request_interface::POST); + } + + /** + * Checks whether a certain variable is set in one of the super global + * arrays. + * + * @param string $var Name of the variable + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies the super global which shall be checked + * + * @return bool True if the variable was sent as input + */ + public function is_set($var, $super_global = phpbb_request_interface::REQUEST) + { + return isset($this->input[$super_global][$var]); + } + + /** + * Returns all variable names for a given super global + * + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * The super global from which names shall be taken + * + * @return array All variable names that are set for the super global. + * Pay attention when using these, they are unsanitised! + */ + public function variable_names($super_global = phpbb_request_interface::REQUEST) + { + if (!isset($this->input[$super_global])) + { + return array(); + } + + return array_keys($this->input[$super_global]); + } +} diff --git a/phpBB/includes/request/request_interface.php b/phpBB/includes/request/request_interface.php new file mode 100644 index 0000000000..7b5b600100 --- /dev/null +++ b/phpBB/includes/request/request_interface.php @@ -0,0 +1,103 @@ + "a") + * then specifying array("var", 1) as the name will return "a". + * @param mixed $default A default value that is returned if the variable was not set. + * This function will always return a value of the same type as the default. + * @param bool $multibyte If $default is a string this paramater has to be true if the variable may contain any UTF-8 characters + * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies which super global should be used + * + * @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the + * the same as that of $default. If the variable is not set $default is returned. + */ + public function variable($var_name, $default, $multibyte = false, $super_global = phpbb_request_interface::REQUEST); + + /** + * Checks whether a certain variable was sent via POST. + * To make sure that a request was sent using POST you should call this function + * on at least one variable. + * + * @param string $name The name of the form variable which should have a + * _p suffix to indicate the check in the code that creates the form too. + * + * @return bool True if the variable was set in a POST request, false otherwise. + */ + public function is_set_post($name); + + /** + * Checks whether a certain variable is set in one of the super global + * arrays. + * + * @param string $var Name of the variable + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies the super global which shall be checked + * + * @return bool True if the variable was sent as input + */ + public function is_set($var, $super_global = phpbb_request_interface::REQUEST); + + /** + * Returns all variable names for a given super global + * + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * The super global from which names shall be taken + * + * @return array All variable names that are set for the super global. + * Pay attention when using these, they are unsanitised! + */ + public function variable_names($super_global = phpbb_request_interface::REQUEST); +} diff --git a/tests/request/all_tests.php b/tests/request/all_tests.php index 1ee3029b36..6757d463c5 100644 --- a/tests/request/all_tests.php +++ b/tests/request/all_tests.php @@ -15,6 +15,8 @@ if (!defined('PHPUnit_MAIN_METHOD')) require_once 'test_framework/framework.php'; require_once 'PHPUnit/TextUI/TestRunner.php'; +require_once 'request/deactivated_super_global.php'; +require_once 'request/request.php'; require_once 'request/request_var.php'; class phpbb_request_all_tests @@ -28,6 +30,8 @@ class phpbb_request_all_tests { $suite = new PHPUnit_Framework_TestSuite('phpBB Request Parameter Handling'); + $suite->addTestSuite('phpbb_request_deactivated_super_global_test'); + $suite->addTestSuite('phpbb_request_test'); $suite->addTestSuite('phpbb_request_request_var_test'); return $suite; diff --git a/tests/request/deactivated_super_global.php b/tests/request/deactivated_super_global.php new file mode 100644 index 0000000000..dcf17b0a0e --- /dev/null +++ b/tests/request/deactivated_super_global.php @@ -0,0 +1,25 @@ +setExpectedTriggerError(E_USER_ERROR); + $obj = new phpbb_deactivated_super_global($this->getMock('phpbb_request_interface'), 'obj', phpbb_request_interface::POST); + $obj->offsetSet(0, 0); + } +} diff --git a/tests/request/request.php b/tests/request/request.php new file mode 100644 index 0000000000..1376d0665a --- /dev/null +++ b/tests/request/request.php @@ -0,0 +1,89 @@ +request = new phpbb_request(); + } + + public function test_toggle_super_globals() + { + $this->assertTrue($this->request->super_globals_disabled(), 'Superglobals were not disabled'); + + $this->request->enable_super_globals(); + + $this->assertFalse($this->request->super_globals_disabled(), 'Superglobals were not enabled'); + + $this->assertEquals(1, $_POST['test'], 'Checking $_POST after enable_super_globals'); + $this->assertEquals(2, $_GET['test'], 'Checking $_GET after enable_super_globals'); + $this->assertEquals(3, $_COOKIE['test'], 'Checking $_COOKIE after enable_super_globals'); + $this->assertEquals(3, $_REQUEST['test'], 'Checking $_REQUEST after enable_super_globals'); + + $_POST['x'] = 2; + $this->assertEquals($_POST, $GLOBALS['_POST'], 'Checking whether $_POST can still be accessed via $GLOBALS[\'_POST\']'); + } + + /** + * Checks that directly accessing $_POST will trigger + * an error. + */ + public function test_disable_post_super_global() + { + $this->setExpectedTriggerError(E_USER_ERROR); + $_POST['test'] = 3; + } + + public function test_is_set_post() + { + $this->assertTrue($this->request->is_set_post('test')); + $this->assertFalse($this->request->is_set_post('unset')); + } + + public function test_addslashes_recursively() + { + $data = array('some"string' => array('that"' => 'really"', 'needs"' => '"escaping')); + $expected = array('some\\"string' => array('that\\"' => 'really\\"', 'needs\\"' => '\\"escaping')); + + phpbb_request::addslashes_recursively($data); + + $this->assertEquals($expected, $data); + } + + public function test_variable_names() + { + $expected = array('test', 'unset'); + $result = $this->request->variable_names(); + $this->assertEquals($expected, $result); + } + + /** + * Makes sure super globals work properly after these tests + */ + protected function tearDown() + { + $this->request->enable_super_globals(); + } +} diff --git a/tests/request/request_class.php b/tests/request/request_class.php deleted file mode 100644 index e8c2154bab..0000000000 --- a/tests/request/request_class.php +++ /dev/null @@ -1,74 +0,0 @@ -assertEquals(1, $_POST['test'], 'Checking $_POST toggling via request::dis/enable_super_globals'); - $this->assertEquals(2, $_GET['test'], 'Checking $_GET toggling via request::dis/enable_super_globals'); - $this->assertEquals(3, $_COOKIE['test'], 'Checking $_COOKIE toggling via request::dis/enable_super_globals'); - $this->assertEquals(3, $_REQUEST['test'], 'Checking $_REQUEST toggling via request::dis/enable_super_globals'); - - $_POST['x'] = 2; - $this->assertEquals($_POST, $GLOBALS['_POST'], 'Checking whether $_POST can still be accessed via $GLOBALS[\'_POST\']'); - } - - /** - * Checks that directly accessing $_POST will trigger - * an error. - */ - public function test_disable_post_super_global() - { - request::disable_super_globals(); - - $this->setExpectedTriggerError(E_USER_ERROR); - $_POST['test'] = 3; - } - - public function test_is_set_post() - { - $_GET['unset'] = ''; - request::reset(); - - $this->assertTrue(request::is_set_post('test')); - $this->assertFalse(request::is_set_post('unset')); - } - - /** - * Makes sure super globals work properly after these tests - */ - protected function tearDown() - { - request::enable_super_globals(); - request::reset(); - } -} \ No newline at end of file diff --git a/tests/request/request_var.php b/tests/request/request_var.php index 0f24d77034..50c6ffd44b 100644 --- a/tests/request/request_var.php +++ b/tests/request/request_var.php @@ -8,6 +8,9 @@ */ require_once 'test_framework/framework.php'; +require_once '../phpBB/includes/request/request_interface.php'; +require_once '../phpBB/includes/request/deactivated_super_global.php'; +require_once '../phpBB/includes/request/request.php'; require_once '../phpBB/includes/functions.php'; class phpbb_request_request_var_test extends phpbb_test_case From cf3f0f825af846da6e7b36a84328475b164ade64 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 6 Mar 2010 05:49:57 +0100 Subject: [PATCH 02/21] [feature/request-class] New request class supports recursive arrays. So we can enable this old 3 level deep array input entry in the request_var data provider, it is now also supported! PHPBB3-9716 --- tests/request/request_var.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/request/request_var.php b/tests/request/request_var.php index 50c6ffd44b..ca764a6481 100644 --- a/tests/request/request_var.php +++ b/tests/request/request_var.php @@ -215,7 +215,7 @@ class phpbb_request_request_var_test extends phpbb_test_case 'abc' => array() ) ), - /* 3-dimensional (not supported atm! + // 3-dimensional (not supported atm! array( // input: array( @@ -260,7 +260,6 @@ class phpbb_request_request_var_test extends phpbb_test_case 'ä' => array(4 => array('a' => 2, 'ö' => 3)), ) ), - */ ); } From 99a3adfba791baabd27f82429fddfbcb82625523 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 10 Mar 2010 11:09:36 +0100 Subject: [PATCH 03/21] [feature/request-class] Instantiate a global request class instance. It should at all cost be avoided to rely on this global variable. Instead either use the request_var method (deprecated) or pass the instance to your function as a parameter or to your object as a contructor argument or through a setter function. PHPBB3-9716 --- phpBB/common.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/phpBB/common.php b/phpBB/common.php index 6cc7abf118..a4be46f987 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -195,6 +195,9 @@ require($phpbb_root_path . 'includes/template.' . $phpEx); require($phpbb_root_path . 'includes/session.' . $phpEx); require($phpbb_root_path . 'includes/auth.' . $phpEx); +require($phpbb_root_path . 'includes/request/deactivated_super_global.' . $phpEx); +require($phpbb_root_path . 'includes/request/request_interface.' . $phpEx); +require($phpbb_root_path . 'includes/request/request.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); require($phpbb_root_path . 'includes/functions_content.' . $phpEx); @@ -206,6 +209,7 @@ require($phpbb_root_path . 'includes/utf/utf_tools.' . $phpEx); set_error_handler(defined('PHPBB_MSG_HANDLER') ? PHPBB_MSG_HANDLER : 'msg_handler'); // Instantiate some basic classes +$request = new phpbb_request(); $user = new user(); $auth = new auth(); $template = new template(); @@ -215,6 +219,9 @@ $db = new $sql_db(); $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); $class_loader->register(); +// make sure request_var uses this request instance +request_var($request, 0); // "dependency injection" for a function + // Connect to DB $db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, defined('PHPBB_DB_NEW_LINK') ? PHPBB_DB_NEW_LINK : false); @@ -233,4 +240,4 @@ foreach ($cache->obtain_hooks() as $hook) @include($phpbb_root_path . 'includes/hooks/' . $hook . '.' . $phpEx); } -?> \ No newline at end of file +?> From d87d9d96b2de67480084f0626986dadaeeda7806 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 11 Mar 2010 15:42:24 +0100 Subject: [PATCH 04/21] [feature/request-class] request_var should return after setting the request object. If the "dependency injection" mechanism is used there should not be any regular computation of a result value. request_var has to return immediately. PHPBB3-9716 --- phpBB/includes/functions.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 485f5487cc..d5132d218e 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -49,6 +49,7 @@ function request_var($var_name, $default, $multibyte = false, $cookie = false) if ($var_name instanceof phpbb_request_interface) { $request = $var_name; + return; } // no request class set, create a temporary one ourselves to keep backwards compatability From 76e530196bb99d02b3d6d7736fde027fa5e2bae8 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 11 Mar 2010 15:46:14 +0100 Subject: [PATCH 05/21] [feature/request-class] Use the request class in the installer & updater. Just like common.php database_update.php and install/index.php need to include the request class files and create an instance for use in request_var. PHPBB3-9716 --- phpBB/install/database_update.php | 13 ++++++++++--- phpBB/install/index.php | 6 +++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index ca4ef817be..d571d41be4 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -66,6 +66,9 @@ require($phpbb_root_path . 'includes/template.' . $phpEx); require($phpbb_root_path . 'includes/session.' . $phpEx); require($phpbb_root_path . 'includes/auth.' . $phpEx); +require($phpbb_root_path . 'includes/request/deactivated_super_global.' . $phpEx); +require($phpbb_root_path . 'includes/request/request_interface.' . $phpEx); +require($phpbb_root_path . 'includes/request/request.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); if (file_exists($phpbb_root_path . 'includes/functions_content.' . $phpEx)) @@ -92,10 +95,14 @@ else define('STRIP', (get_magic_quotes_gpc()) ? true : false); } +$request = new phpbb_request(); $user = new user(); $cache = new cache(); $db = new $sql_db(); +// make sure request_var uses this request instance +request_var($request, 0); // "dependency injection" for a function + // Add own hook handler, if present. :o if (file_exists($phpbb_root_path . 'includes/hooks/index.' . $phpEx)) { @@ -1947,7 +1954,7 @@ class updater_db_tools 'VCHAR_CI' => '[varchar] (255)', 'VARBINARY' => '[varchar] (255)', ), - + 'mssqlnative' => array( 'INT:' => '[int]', 'BINT' => '[float]', @@ -1977,7 +1984,7 @@ class updater_db_tools 'VCHAR_CI' => '[varchar] (255)', 'VARBINARY' => '[varchar] (255)', ), - + 'oracle' => array( 'INT:' => 'number(%d)', 'BINT' => 'number(20)', @@ -2124,7 +2131,7 @@ class updater_db_tools case 'mssql_odbc': $this->sql_layer = 'mssql'; break; - + case 'mssqlnative': $this->sql_layer = 'mssqlnative'; break; diff --git a/phpBB/install/index.php b/phpBB/install/index.php index 03b19d1c12..3e4331cde5 100644 --- a/phpBB/install/index.php +++ b/phpBB/install/index.php @@ -171,6 +171,10 @@ require($phpbb_root_path . 'includes/functions_install.' . $phpEx); $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx); $class_loader->register(); +$request = new phpbb_request(); + +// make sure request_var uses this request instance +request_var($request, 0); // "dependency injection" for a function // Try and load an appropriate language if required $language = basename(request_var('language', '')); @@ -813,4 +817,4 @@ class module } } -?> \ No newline at end of file +?> From 6beeda79eb5a001b589e987d832acf4ea0ae5b4f Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 11 Mar 2010 16:08:19 +0100 Subject: [PATCH 06/21] [feature/request-class] Replace direct use of GET/REQUEST with request_var. Now with $_VARs causing fatal errors we should really be able to find and delete all of these occurances. PHPBB3-9716 --- phpBB/includes/session.php | 9 +++++---- phpBB/viewtopic.php | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/phpBB/includes/session.php b/phpBB/includes/session.php index cc216392b8..0e6a23762e 100644 --- a/phpBB/includes/session.php +++ b/phpBB/includes/session.php @@ -130,7 +130,7 @@ class session 'root_script_path' => str_replace(' ', '%20', htmlspecialchars($root_script_path)), 'page' => $page, - 'forum' => (isset($_REQUEST['f']) && $_REQUEST['f'] > 0) ? (int) $_REQUEST['f'] : 0, + 'forum' => request_var('f', 0), ); return $page_array; @@ -318,7 +318,7 @@ class session } // Is session_id is set or session_id is set and matches the url param if required - if (!empty($this->session_id) && (!defined('NEED_SID') || (isset($_GET['sid']) && $this->session_id === $_GET['sid']))) + if (!empty($this->session_id) && (!defined('NEED_SID') || (isset($_GET['sid']) && $this->session_id === request_var('sid', '')))) { $sql = 'SELECT u.*, s.* FROM ' . SESSIONS_TABLE . ' s, ' . USERS_TABLE . " u @@ -1591,11 +1591,12 @@ class user extends session $this->add_lang($lang_set); unset($lang_set); - if (!empty($_GET['style']) && $auth->acl_get('a_styles') && !defined('ADMIN_START')) + $style_request = request_var('style', 0); + if ($style_request && $auth->acl_get('a_styles') && !defined('ADMIN_START')) { global $SID, $_EXTRA_URL; - $style = request_var('style', 0); + $style = $style_request; $SID .= '&style=' . $style; $_EXTRA_URL = array('style=' . $style); } diff --git a/phpBB/viewtopic.php b/phpBB/viewtopic.php index 498088c5c8..fecd87bbc1 100644 --- a/phpBB/viewtopic.php +++ b/phpBB/viewtopic.php @@ -1732,15 +1732,15 @@ if ($s_can_vote || $s_quick_reply) // We overwrite $_REQUEST['f'] if there is no forum specified // to be able to display the correct online list. // One downside is that the user currently viewing this topic/post is not taken into account. -if (empty($_REQUEST['f'])) +if (!request_var('f', 0)) { - $_REQUEST['f'] = $forum_id; + $request->overwrite('f', $forum_id); } // We need to do the same with the topic_id. See #53025. -if (empty($_REQUEST['t']) && !empty($topic_id)) +if (!request_var('t', 0) && !empty($topic_id)) { - $_REQUEST['t'] = $topic_id; + $request->overwrite('t', $topic_id); } // Output the page From 85b6d3b9a1b346f36232d98bcf308f5635eb7f49 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 13 Mar 2010 11:08:12 +0100 Subject: [PATCH 07/21] [feature/request-class] Extracted type casting helpers from the request class. These methods should be available without having to instantiate a request class object, better separation of concerns. A set_var wrapper around this class no longer requires a request object at all. PHPBB3-9716 --- phpBB/includes/request/type_cast_helper.php | 178 ++++++++++++++++++ .../request/type_cast_helper_interface.php | 63 +++++++ tests/request/type_cast_helper.php | 33 ++++ 3 files changed, 274 insertions(+) create mode 100644 phpBB/includes/request/type_cast_helper.php create mode 100644 phpBB/includes/request/type_cast_helper_interface.php create mode 100644 tests/request/type_cast_helper.php diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php new file mode 100644 index 0000000000..ff392fbf8f --- /dev/null +++ b/phpBB/includes/request/type_cast_helper.php @@ -0,0 +1,178 @@ +=')) + { + $this->strip = false; + } + else + { + $this->strip = (@get_magic_quotes_gpc()) ? true : false; + } + } + + /** + * Recursively applies addslashes to a variable. + * + * @param mixed &$var Variable passed by reference to which slashes will be added. + */ + public function addslashes_recursively(&$var) + { + if (is_string($var)) + { + $var = addslashes($var); + } + else if (is_array($var)) + { + $var_copy = $var; + $var = array(); + foreach ($var_copy as $key => $value) + { + if (is_string($key)) + { + $key = addslashes($key); + } + $var[$key] = $value; + + $this->addslashes_recursively($var[$key]); + } + } + } + + /** + * Recursively applies addslashes to a variable if magic quotes are turned on. + * + * @param mixed &$var Variable passed by reference to which slashes will be added. + */ + public function add_magic_quotes(&$var) + { + if ($this->strip) + { + $this->addslashes_recursively($var); + } + } + + /** + * Set variable $result to a particular type. + * + * @param mixed &$result The variable to fill + * @param mixed $var The contents to fill with + * @param mixed $type The variable type. Will be used with {@link settype()} + * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. + * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks. + */ + public function set_var(&$result, $var, $type, $multibyte = false) + { + settype($var, $type); + $result = $var; + + if ($type == 'string') + { + $result = trim(htmlspecialchars(str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result), ENT_COMPAT, 'UTF-8')); + + if (!empty($result)) + { + // Make sure multibyte characters are wellformed + if ($multibyte) + { + if (!preg_match('/^./u', $result)) + { + $result = ''; + } + } + else + { + // no multibyte, allow only ASCII (0-127) + $result = preg_replace('/[\x80-\xFF]/', '?', $result); + } + } + + $result = ($this->strip) ? stripslashes($result) : $result; + } + } + + /** + * Recursively sets a variable to a given type using {@link set_var set_var} + * + * @param string $var The value which shall be sanitised (passed by reference). + * @param mixed $default Specifies the type $var shall have. + * If it is an array and $var is not one, then an empty array is returned. + * Otherwise var is cast to the same type, and if $default is an array all + * keys and values are cast recursively using this function too. + * @param bool $multibyte Indicates whether string keys and values may contain UTF-8 characters. + * Default is false, causing all bytes outside the ASCII range (0-127) to + * be replaced with question marks. + */ + public function recursive_set_var(&$var, $default, $multibyte) + { + if (is_array($var) !== is_array($default)) + { + $var = (is_array($default)) ? array() : $default; + return; + } + + if (!is_array($default)) + { + $type = gettype($default); + $this->set_var($var, $var, $type, $multibyte); + } + else + { + // make sure there is at least one key/value pair to use get the + // types from + if (empty($default)) + { + $var = array(); + return; + } + + list($default_key, $default_value) = each($default); + $value_type = gettype($default_value); + $key_type = gettype($default_key); + + $_var = $var; + $var = array(); + + foreach ($_var as $k => $v) + { + $this->set_var($k, $k, $key_type, $multibyte, $multibyte); + + $this->recursive_set_var($v, $default_value, $multibyte); + $this->set_var($var[$k], $v, $value_type, $multibyte); + } + } + } +} diff --git a/phpBB/includes/request/type_cast_helper_interface.php b/phpBB/includes/request/type_cast_helper_interface.php new file mode 100644 index 0000000000..96e984003c --- /dev/null +++ b/phpBB/includes/request/type_cast_helper_interface.php @@ -0,0 +1,63 @@ +type_cast_helper = new phpbb_type_cast_helper(); + } + + public function test_addslashes_recursively() + { + $data = array('some"string' => array('that"' => 'really"', 'needs"' => '"escaping')); + $expected = array('some\\"string' => array('that\\"' => 'really\\"', 'needs\\"' => '\\"escaping')); + + $this->type_cast_helper->addslashes_recursively($data); + + $this->assertEquals($expected, $data); + } +} From ea919ad8b276c78207ec33d1fc34f1f0ef15bc0d Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 13 Mar 2010 11:19:28 +0100 Subject: [PATCH 08/21] [feature/request-class] Refactored request class and wrapper functions. The request class - now makes use of the new type cast helper (dependency injection) - has no static methods anymore. - now has a constructor argument to leave super globals turned on Brought back the set_var function in functions.php. It is now a wrapper around the type cast helper. It creates an instance on the fly. The request_var wrapper function now has an optional last argument to inject the request class instance, rather than abusing the $var_name. PHPBB3-9716 --- phpBB/includes/functions.php | 35 +++-- phpBB/includes/request/request.php | 166 +++------------------ tests/request/all_tests.php | 6 +- tests/request/deactivated_super_global.php | 4 +- tests/request/request.php | 12 +- tests/request/request_var.php | 6 +- 6 files changed, 56 insertions(+), 173 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index d5132d218e..81c5c40fd1 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -18,6 +18,13 @@ if (!defined('IN_PHPBB')) // Common global functions +function set_var(&$result, $var, $type, $multibyte = false) +{ + // no need for dependency injection here, if you have the object, call the method yourself! + $type_cast_helper = new phpbb_type_cast_helper(); + $type_cast_helper->set_var($result, $var, $type, $multibyte); +} + /** * Wrapper function of phpbb_request::variable which exists for backwards compatability. * See {@link phpbb_request_interface::variable phpbb_request_interface::variable} for @@ -40,30 +47,30 @@ if (!defined('IN_PHPBB')) * @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the * the same as that of $default. If the variable is not set $default is returned. */ -function request_var($var_name, $default, $multibyte = false, $cookie = false) +function request_var($var_name, $default, $multibyte = false, $cookie = false, phpbb_request_interface $request = null) { // This is all just an ugly hack to add "Dependency Injection" to a function // the only real code is the function call which maps this function to a method. - static $request = null; + static $static_request = null; - if ($var_name instanceof phpbb_request_interface) + if ($request instanceof phpbb_request_interface) { - $request = $var_name; - return; + $static_request = $request; + + if (empty($var_name)) + { + return; + } } + $tmp_request = $static_request; + // no request class set, create a temporary one ourselves to keep backwards compatability - if ($request === null) + if ($tmp_request === null) { - $tmp_request = new phpbb_request(); - // enable super globals, so the magically created request class does not + // false param: enable super globals, so the created request class does not // make super globals inaccessible everywhere outside this function. - $tmp_request->enable_super_globals(); - } - else - { - // otherwise use the static injected instance - $tmp_request = $request; + $tmp_request = new phpbb_request(new phpbb_type_cast_helper(), false); } return $tmp_request->variable($var_name, $default, $multibyte, ($cookie) ? phpbb_request_interface::COOKIE : phpbb_request_interface::REQUEST); diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php index dbed95546f..a473e40426 100644 --- a/phpBB/includes/request/request.php +++ b/phpBB/includes/request/request.php @@ -28,7 +28,7 @@ class phpbb_request implements phpbb_request_interface /** * @var array The names of super global variables that this class should protect if super globals are disabled. */ - protected static $super_globals = array( + protected $super_globals = array( phpbb_request_interface::POST => '_POST', phpbb_request_interface::GET => '_GET', phpbb_request_interface::REQUEST => '_REQUEST', @@ -51,26 +51,26 @@ class phpbb_request implements phpbb_request_interface protected $input; /** - * @var string Whether slashes need to be stripped from input + * @var phpbb_type_cast_helper_interface An instance of a type cast helper providing convenience methods for type conversions. */ - protected $strip; + protected $type_cast_helper; /** * Initialises the request class, that means it stores all input data in {@link $input input} * and then calls {@link phpbb_deactivated_super_global phpbb_deactivated_super_global} */ - public function __construct() + public function __construct(phpbb_type_cast_helper_interface $type_cast_helper = null, $disable_super_globals = true) { - if (version_compare(PHP_VERSION, '6.0.0-dev', '>=')) + if ($type_cast_helper) { - $this->strip = false; + $this->type_cast_helper = $type_cast_helper; } else { - $this->strip = (@get_magic_quotes_gpc()) ? true : false; + $this->type_cast_helper = new phpbb_type_cast_helper(); } - foreach (self::$super_globals as $const => $super_global) + foreach ($this->super_globals as $const => $super_global) { $this->input[$const] = isset($GLOBALS[$super_global]) ? $GLOBALS[$super_global] : array(); } @@ -79,7 +79,10 @@ class phpbb_request implements phpbb_request_interface $this->original_request = $this->input[phpbb_request_interface::REQUEST]; $this->input[phpbb_request_interface::REQUEST] = $this->input[phpbb_request_interface::POST] + $this->input[phpbb_request_interface::GET]; - $this->disable_super_globals(); + if ($disable_super_globals) + { + $this->disable_super_globals(); + } } /** @@ -100,7 +103,7 @@ class phpbb_request implements phpbb_request_interface { if (!$this->super_globals_disabled) { - foreach (self::$super_globals as $const => $super_global) + foreach ($this->super_globals as $const => $super_global) { unset($GLOBALS[$super_global]); $GLOBALS[$super_global] = new phpbb_deactivated_super_global($this, $super_global, $const); @@ -118,7 +121,7 @@ class phpbb_request implements phpbb_request_interface { if ($this->super_globals_disabled) { - foreach (self::$super_globals as $const => $super_global) + foreach ($this->super_globals as $const => $super_global) { $GLOBALS[$super_global] = $this->input[$const]; } @@ -129,34 +132,6 @@ class phpbb_request implements phpbb_request_interface } } - /** - * Recursively applies addslashes to a variable. - * - * @param mixed &$var Variable passed by reference to which slashes will be added. - */ - public static function addslashes_recursively(&$var) - { - if (is_string($var)) - { - $var = addslashes($var); - } - else if (is_array($var)) - { - $var_copy = $var; - $var = array(); - foreach ($var_copy as $key => $value) - { - if (is_string($key)) - { - $key = addslashes($key); - } - $var[$key] = $value; - - self::addslashes_recursively($var[$key]); - } - } - } - /** * This function allows overwriting or setting a value in one of the super global arrays. * @@ -172,15 +147,12 @@ class phpbb_request implements phpbb_request_interface */ public function overwrite($var_name, $value, $super_global = phpbb_request_interface::REQUEST) { - if (!isset(self::$super_globals[$super_global])) + if (!isset($this->super_globals[$super_global])) { return; } - if ($this->strip) - { - self::addslashes_recursively($value); - } + $this->type_cast_helper->add_magic_quotes($value); // setting to null means unsetting if ($value === null) @@ -188,114 +160,22 @@ class phpbb_request implements phpbb_request_interface unset($this->input[$super_global][$var_name]); if (!$this->super_globals_disabled()) { - unset($GLOBALS[self::$super_globals[$super_global]][$var_name]); + unset($GLOBALS[$this->super_globals[$super_global]][$var_name]); } } else { $this->input[$super_global][$var_name] = $value; - if (!self::super_globals_disabled()) + if (!$this->super_globals_disabled()) { - $GLOBALS[self::$super_globals[$super_global]][$var_name] = $value; + $GLOBALS[$this->super_globals[$super_global]][$var_name] = $value; } } - if (!self::super_globals_disabled()) + if (!$this->super_globals_disabled()) { - unset($GLOBALS[self::$super_globals[$super_global]][$var_name]); - $GLOBALS[self::$super_globals[$super_global]][$var_name] = $value; - } - } - - /** - * Set variable $result to a particular type. - * - * @param mixed &$result The variable to fill - * @param mixed $var The contents to fill with - * @param mixed $type The variable type. Will be used with {@link settype()} - * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. - * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks. - */ - public function set_var(&$result, $var, $type, $multibyte = false) - { - settype($var, $type); - $result = $var; - - if ($type == 'string') - { - $result = trim(htmlspecialchars(str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result), ENT_COMPAT, 'UTF-8')); - - if (!empty($result)) - { - // Make sure multibyte characters are wellformed - if ($multibyte) - { - if (!preg_match('/^./u', $result)) - { - $result = ''; - } - } - else - { - // no multibyte, allow only ASCII (0-127) - $result = preg_replace('/[\x80-\xFF]/', '?', $result); - } - } - - $result = ($this->strip) ? stripslashes($result) : $result; - } - } - - /** - * Recursively sets a variable to a given type using {@link set_var set_var} - * This function is only used from within {@link phpbb_request::variable phpbb_request::variable}. - * - * @param string $var The value which shall be sanitised (passed by reference). - * @param mixed $default Specifies the type $var shall have. - * If it is an array and $var is not one, then an empty array is returned. - * Otherwise var is cast to the same type, and if $default is an array all - * keys and values are cast recursively using this function too. - * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. - * Default is false, causing all bytes outside the ASCII range (0-127) to - * be replaced with question marks. - */ - protected function recursive_set_var(&$var, $default, $multibyte) - { - if (is_array($var) !== is_array($default)) - { - $var = (is_array($default)) ? array() : $default; - return; - } - - if (!is_array($default)) - { - $type = gettype($default); - $this->set_var($var, $var, $type, $multibyte); - } - else - { - // make sure there is at least one key/value pair to use get the - // types from - if (empty($default)) - { - $var = array(); - return; - } - - list($default_key, $default_value) = each($default); - $value_type = gettype($default_value); - $key_type = gettype($default_key); - - $_var = $var; - $var = array(); - - foreach ($_var as $k => $v) - { - $this->set_var($k, $k, $key_type, $multibyte, $multibyte); - - $this->recursive_set_var($v, $default_value, $multibyte); - $this->set_var($var[$k], $v, $value_type, $multibyte); - } + unset($GLOBALS[$this->super_globals[$super_global]][$var_name]); + $GLOBALS[$this->super_globals[$super_global]][$var_name] = $value; } } @@ -356,7 +236,7 @@ class phpbb_request implements phpbb_request_interface } } - self::recursive_set_var($var, $default, $multibyte); + $this->type_cast_helper->recursive_set_var($var, $default, $multibyte); return $var; } diff --git a/tests/request/all_tests.php b/tests/request/all_tests.php index 6757d463c5..f1633309fd 100644 --- a/tests/request/all_tests.php +++ b/tests/request/all_tests.php @@ -15,6 +15,7 @@ if (!defined('PHPUnit_MAIN_METHOD')) require_once 'test_framework/framework.php'; require_once 'PHPUnit/TextUI/TestRunner.php'; +require_once 'request/type_cast_helper.php'; require_once 'request/deactivated_super_global.php'; require_once 'request/request.php'; require_once 'request/request_var.php'; @@ -30,9 +31,10 @@ class phpbb_request_all_tests { $suite = new PHPUnit_Framework_TestSuite('phpBB Request Parameter Handling'); - $suite->addTestSuite('phpbb_request_deactivated_super_global_test'); + $suite->addTestSuite('phpbb_type_cast_helper_test'); + $suite->addTestSuite('phpbb_deactivated_super_global_test'); $suite->addTestSuite('phpbb_request_test'); - $suite->addTestSuite('phpbb_request_request_var_test'); + $suite->addTestSuite('phpbb_request_var_test'); return $suite; } diff --git a/tests/request/deactivated_super_global.php b/tests/request/deactivated_super_global.php index dcf17b0a0e..2991badd1a 100644 --- a/tests/request/deactivated_super_global.php +++ b/tests/request/deactivated_super_global.php @@ -11,12 +11,12 @@ require_once 'test_framework/framework.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; -class phpbb_request_deactivated_super_global_test extends phpbb_test_case +class phpbb_deactivated_super_global_test extends phpbb_test_case { /** * Checks that on write access the correct error is thrown */ - public function test_write_results_in_error() + public function test_write_triggers_error() { $this->setExpectedTriggerError(E_USER_ERROR); $obj = new phpbb_deactivated_super_global($this->getMock('phpbb_request_interface'), 'obj', phpbb_request_interface::POST); diff --git a/tests/request/request.php b/tests/request/request.php index 1376d0665a..df71d783ed 100644 --- a/tests/request/request.php +++ b/tests/request/request.php @@ -9,6 +9,8 @@ */ require_once 'test_framework/framework.php'; +require_once '../phpBB/includes/request/type_cast_helper_interface.php'; +require_once '../phpBB/includes/request/type_cast_helper.php'; require_once '../phpBB/includes/request/request_interface.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; require_once '../phpBB/includes/request/request.php'; @@ -62,16 +64,6 @@ class phpbb_request_test extends phpbb_test_case $this->assertFalse($this->request->is_set_post('unset')); } - public function test_addslashes_recursively() - { - $data = array('some"string' => array('that"' => 'really"', 'needs"' => '"escaping')); - $expected = array('some\\"string' => array('that\\"' => 'really\\"', 'needs\\"' => '\\"escaping')); - - phpbb_request::addslashes_recursively($data); - - $this->assertEquals($expected, $data); - } - public function test_variable_names() { $expected = array('test', 'unset'); diff --git a/tests/request/request_var.php b/tests/request/request_var.php index ca764a6481..5bdcb5d4e7 100644 --- a/tests/request/request_var.php +++ b/tests/request/request_var.php @@ -8,12 +8,14 @@ */ require_once 'test_framework/framework.php'; -require_once '../phpBB/includes/request/request_interface.php'; +require_once '../phpBB/includes/request/type_cast_helper_interface.php'; +require_once '../phpBB/includes/request/type_cast_helper.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; +require_once '../phpBB/includes/request/request_interface.php'; require_once '../phpBB/includes/request/request.php'; require_once '../phpBB/includes/functions.php'; -class phpbb_request_request_var_test extends phpbb_test_case +class phpbb_request_var_test extends phpbb_test_case { /** * @dataProvider request_variables From 0ae7df8a51129f2ece86f959e2e155d988feb081 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 13 Mar 2010 11:28:41 +0100 Subject: [PATCH 09/21] [feature/request-class] Request class test now uses a type cast helper mock. Removed the dependency of the request class test on having an actual phpbb_type_cast_helper instance, by replacing it with an object mocking the phpbb_type_cast_helper_interface. PHPBB3-9716 --- phpBB/common.php | 6 ++++-- phpBB/install/database_update.php | 6 ++++-- phpBB/install/index.php | 5 +++-- tests/request/request.php | 6 ++++-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/phpBB/common.php b/phpBB/common.php index a4be46f987..57a1384ee4 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -195,6 +195,8 @@ require($phpbb_root_path . 'includes/template.' . $phpEx); require($phpbb_root_path . 'includes/session.' . $phpEx); require($phpbb_root_path . 'includes/auth.' . $phpEx); +require($phpbb_root_path . 'includes/request/type_cast_helper_interface.' . $phpEx); +require($phpbb_root_path . 'includes/request/type_cast_helper.' . $phpEx); require($phpbb_root_path . 'includes/request/deactivated_super_global.' . $phpEx); require($phpbb_root_path . 'includes/request/request_interface.' . $phpEx); require($phpbb_root_path . 'includes/request/request.' . $phpEx); @@ -209,7 +211,7 @@ require($phpbb_root_path . 'includes/utf/utf_tools.' . $phpEx); set_error_handler(defined('PHPBB_MSG_HANDLER') ? PHPBB_MSG_HANDLER : 'msg_handler'); // Instantiate some basic classes -$request = new phpbb_request(); +$request = new phpbb_request(new phpbb_type_cast_helper()); $user = new user(); $auth = new auth(); $template = new template(); @@ -220,7 +222,7 @@ $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); $class_loader->register(); // make sure request_var uses this request instance -request_var($request, 0); // "dependency injection" for a function +request_var('', 0, false, false, $request); // "dependency injection" for a function // Connect to DB $db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, defined('PHPBB_DB_NEW_LINK') ? PHPBB_DB_NEW_LINK : false); diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index d571d41be4..483f24f3e5 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -66,6 +66,8 @@ require($phpbb_root_path . 'includes/template.' . $phpEx); require($phpbb_root_path . 'includes/session.' . $phpEx); require($phpbb_root_path . 'includes/auth.' . $phpEx); +require($phpbb_root_path . 'includes/request/type_cast_helper_interface.' . $phpEx); +require($phpbb_root_path . 'includes/request/type_cast_helper.' . $phpEx); require($phpbb_root_path . 'includes/request/deactivated_super_global.' . $phpEx); require($phpbb_root_path . 'includes/request/request_interface.' . $phpEx); require($phpbb_root_path . 'includes/request/request.' . $phpEx); @@ -95,13 +97,13 @@ else define('STRIP', (get_magic_quotes_gpc()) ? true : false); } -$request = new phpbb_request(); +$request = new phpbb_request(new phpbb_type_cast_helper()); $user = new user(); $cache = new cache(); $db = new $sql_db(); // make sure request_var uses this request instance -request_var($request, 0); // "dependency injection" for a function +request_var('', 0, false, false, $request); // "dependency injection" for a function // Add own hook handler, if present. :o if (file_exists($phpbb_root_path . 'includes/hooks/index.' . $phpEx)) diff --git a/phpBB/install/index.php b/phpBB/install/index.php index 3e4331cde5..5862e2e8d2 100644 --- a/phpBB/install/index.php +++ b/phpBB/install/index.php @@ -171,10 +171,11 @@ require($phpbb_root_path . 'includes/functions_install.' . $phpEx); $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx); $class_loader->register(); -$request = new phpbb_request(); + +$request = new phpbb_request(new phpbb_type_cast_helper()); // make sure request_var uses this request instance -request_var($request, 0); // "dependency injection" for a function +request_var('', 0, false, false, $request); // "dependency injection" for a function // Try and load an appropriate language if required $language = basename(request_var('language', '')); diff --git a/tests/request/request.php b/tests/request/request.php index df71d783ed..ebfc3ba2b0 100644 --- a/tests/request/request.php +++ b/tests/request/request.php @@ -10,13 +10,13 @@ require_once 'test_framework/framework.php'; require_once '../phpBB/includes/request/type_cast_helper_interface.php'; -require_once '../phpBB/includes/request/type_cast_helper.php'; require_once '../phpBB/includes/request/request_interface.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; require_once '../phpBB/includes/request/request.php'; class phpbb_request_test extends phpbb_test_case { + private $type_cast_helper; private $request; protected function setUp() @@ -28,7 +28,9 @@ class phpbb_request_test extends phpbb_test_case $_REQUEST['test'] = 3; $_GET['unset'] = ''; - $this->request = new phpbb_request(); + $this->type_cast_helper = $this->getMock('phpbb_type_cast_helper_interface'); + + $this->request = new phpbb_request($this->type_cast_helper); } public function test_toggle_super_globals() From b3558b50786dd8a66e4c4011647c048612bbfa7f Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Sat, 24 Apr 2010 15:28:50 +0200 Subject: [PATCH 10/21] [feature/request-class] Automatically normalize multibyte data in request_var To save users from having to run everything through utf8_normalize_nfc(), a call is done automatically from within set_var, which is called by request_var. PHPBB3-9716 --- phpBB/includes/request/type_cast_helper.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php index ff392fbf8f..7b8aacc5a9 100644 --- a/phpBB/includes/request/type_cast_helper.php +++ b/phpBB/includes/request/type_cast_helper.php @@ -103,6 +103,11 @@ class phpbb_type_cast_helper implements phpbb_type_cast_helper_interface { $result = trim(htmlspecialchars(str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result), ENT_COMPAT, 'UTF-8')); + if ($multibyte) + { + $result = utf8_normalize_nfc($result); + } + if (!empty($result)) { // Make sure multibyte characters are wellformed From 456de639123ae3da6320bed6140ab69ac9925e74 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Tue, 31 Aug 2010 21:26:50 +0200 Subject: [PATCH 11/21] [feature/request-class] Refactor request classes to use autoloading All class names have been adjusted to use a phpbb_request prefix, allowing them to be autoloaded. Also introduces some improvements to autoloading in general. PHPBB3-9716 --- phpBB/common.php | 15 ++++++++------- phpBB/includes/functions.php | 4 ++-- .../includes/request/deactivated_super_global.php | 2 +- .../{request_interface.php => interface.php} | 0 phpBB/includes/request/request.php | 12 ++++++------ phpBB/includes/request/type_cast_helper.php | 2 +- .../request/type_cast_helper_interface.php | 2 +- phpBB/install/database_update.php | 13 ++++++------- phpBB/install/index.php | 2 +- phpBB/style.php | 7 ++++++- tests/request/deactivated_super_global.php | 3 ++- tests/request/request.php | 4 ++-- tests/request/request_var.php | 2 +- tests/request/type_cast_helper.php | 2 +- 14 files changed, 38 insertions(+), 32 deletions(-) rename phpBB/includes/request/{request_interface.php => interface.php} (100%) diff --git a/phpBB/common.php b/phpBB/common.php index 57a1384ee4..41df3049a1 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -195,11 +195,6 @@ require($phpbb_root_path . 'includes/template.' . $phpEx); require($phpbb_root_path . 'includes/session.' . $phpEx); require($phpbb_root_path . 'includes/auth.' . $phpEx); -require($phpbb_root_path . 'includes/request/type_cast_helper_interface.' . $phpEx); -require($phpbb_root_path . 'includes/request/type_cast_helper.' . $phpEx); -require($phpbb_root_path . 'includes/request/deactivated_super_global.' . $phpEx); -require($phpbb_root_path . 'includes/request/request_interface.' . $phpEx); -require($phpbb_root_path . 'includes/request/request.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); require($phpbb_root_path . 'includes/functions_content.' . $phpEx); @@ -210,12 +205,18 @@ require($phpbb_root_path . 'includes/utf/utf_tools.' . $phpEx); // Set PHP error handler to ours set_error_handler(defined('PHPBB_MSG_HANDLER') ? PHPBB_MSG_HANDLER : 'msg_handler'); +// Cache must be loaded before class loader +$cache = new cache(); + +// Setup class loader first +$class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); +$class_loader->register(); + // Instantiate some basic classes -$request = new phpbb_request(new phpbb_type_cast_helper()); +$request = new phpbb_request(); $user = new user(); $auth = new auth(); $template = new template(); -$cache = new cache(); $db = new $sql_db(); $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 81c5c40fd1..029281ee84 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -21,7 +21,7 @@ if (!defined('IN_PHPBB')) function set_var(&$result, $var, $type, $multibyte = false) { // no need for dependency injection here, if you have the object, call the method yourself! - $type_cast_helper = new phpbb_type_cast_helper(); + $type_cast_helper = new phpbb_request_type_cast_helper(); $type_cast_helper->set_var($result, $var, $type, $multibyte); } @@ -70,7 +70,7 @@ function request_var($var_name, $default, $multibyte = false, $cookie = false, p { // false param: enable super globals, so the created request class does not // make super globals inaccessible everywhere outside this function. - $tmp_request = new phpbb_request(new phpbb_type_cast_helper(), false); + $tmp_request = new phpbb_request(new phpbb_request_type_cast_helper(), false); } return $tmp_request->variable($var_name, $default, $multibyte, ($cookie) ? phpbb_request_interface::COOKIE : phpbb_request_interface::REQUEST); diff --git a/phpBB/includes/request/deactivated_super_global.php b/phpBB/includes/request/deactivated_super_global.php index 9152d10811..d7a5b3145f 100644 --- a/phpBB/includes/request/deactivated_super_global.php +++ b/phpBB/includes/request/deactivated_super_global.php @@ -21,7 +21,7 @@ if (!defined('IN_PHPBB')) * * @package phpbb_request */ -class phpbb_deactivated_super_global implements ArrayAccess, Countable, IteratorAggregate +class phpbb_request_deactivated_super_global implements ArrayAccess, Countable, IteratorAggregate { /** * @var string Holds the name of the superglobal this is replacing. diff --git a/phpBB/includes/request/request_interface.php b/phpBB/includes/request/interface.php similarity index 100% rename from phpBB/includes/request/request_interface.php rename to phpBB/includes/request/interface.php diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php index a473e40426..7d284a9bf7 100644 --- a/phpBB/includes/request/request.php +++ b/phpBB/includes/request/request.php @@ -51,15 +51,15 @@ class phpbb_request implements phpbb_request_interface protected $input; /** - * @var phpbb_type_cast_helper_interface An instance of a type cast helper providing convenience methods for type conversions. + * @var phpbb_request_type_cast_helper_interface An instance of a type cast helper providing convenience methods for type conversions. */ protected $type_cast_helper; /** * Initialises the request class, that means it stores all input data in {@link $input input} - * and then calls {@link phpbb_deactivated_super_global phpbb_deactivated_super_global} + * and then calls {@link phpbb_request_deactivated_super_global phpbb_request_deactivated_super_global} */ - public function __construct(phpbb_type_cast_helper_interface $type_cast_helper = null, $disable_super_globals = true) + public function __construct(phpbb_request_type_cast_helper_interface $type_cast_helper = null, $disable_super_globals = true) { if ($type_cast_helper) { @@ -67,7 +67,7 @@ class phpbb_request implements phpbb_request_interface } else { - $this->type_cast_helper = new phpbb_type_cast_helper(); + $this->type_cast_helper = new phpbb_request_type_cast_helper(); } foreach ($this->super_globals as $const => $super_global) @@ -97,7 +97,7 @@ class phpbb_request implements phpbb_request_interface /** * Disables access of super globals specified in $super_globals. - * This is achieved by overwriting the super globals with instances of {@link phpbb_deactivated_super_global phpbb_deactivated_super_global} + * This is achieved by overwriting the super globals with instances of {@link phpbb_request_deactivated_super_global phpbb_request_deactivated_super_global} */ public function disable_super_globals() { @@ -106,7 +106,7 @@ class phpbb_request implements phpbb_request_interface foreach ($this->super_globals as $const => $super_global) { unset($GLOBALS[$super_global]); - $GLOBALS[$super_global] = new phpbb_deactivated_super_global($this, $super_global, $const); + $GLOBALS[$super_global] = new phpbb_request_deactivated_super_global($this, $super_global, $const); } $this->super_globals_disabled = true; diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php index 7b8aacc5a9..c39bbebe01 100644 --- a/phpBB/includes/request/type_cast_helper.php +++ b/phpBB/includes/request/type_cast_helper.php @@ -20,7 +20,7 @@ if (!defined('IN_PHPBB')) * * @package phpbb_request */ -class phpbb_type_cast_helper implements phpbb_type_cast_helper_interface +class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_interface { /** diff --git a/phpBB/includes/request/type_cast_helper_interface.php b/phpBB/includes/request/type_cast_helper_interface.php index 96e984003c..366bd2e6ce 100644 --- a/phpBB/includes/request/type_cast_helper_interface.php +++ b/phpBB/includes/request/type_cast_helper_interface.php @@ -20,7 +20,7 @@ if (!defined('IN_PHPBB')) * * @package phpbb_request */ -interface phpbb_type_cast_helper_interface +interface phpbb_request_type_cast_helper_interface { /** * Recursively applies addslashes to a variable. diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 483f24f3e5..69f5f58563 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -66,11 +66,6 @@ require($phpbb_root_path . 'includes/template.' . $phpEx); require($phpbb_root_path . 'includes/session.' . $phpEx); require($phpbb_root_path . 'includes/auth.' . $phpEx); -require($phpbb_root_path . 'includes/request/type_cast_helper_interface.' . $phpEx); -require($phpbb_root_path . 'includes/request/type_cast_helper.' . $phpEx); -require($phpbb_root_path . 'includes/request/deactivated_super_global.' . $phpEx); -require($phpbb_root_path . 'includes/request/request_interface.' . $phpEx); -require($phpbb_root_path . 'includes/request/request.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); if (file_exists($phpbb_root_path . 'includes/functions_content.' . $phpEx)) @@ -97,9 +92,13 @@ else define('STRIP', (get_magic_quotes_gpc()) ? true : false); } -$request = new phpbb_request(new phpbb_type_cast_helper()); -$user = new user(); $cache = new cache(); + +$class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); +$class_loader->register(); + +$request = new phpbb_request(); +$user = new user(); $db = new $sql_db(); // make sure request_var uses this request instance diff --git a/phpBB/install/index.php b/phpBB/install/index.php index 5862e2e8d2..50f0a049f2 100644 --- a/phpBB/install/index.php +++ b/phpBB/install/index.php @@ -172,7 +172,7 @@ require($phpbb_root_path . 'includes/functions_install.' . $phpEx); $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx); $class_loader->register(); -$request = new phpbb_request(new phpbb_type_cast_helper()); +$request = new phpbb_request(); // make sure request_var uses this request instance request_var('', 0, false, false, $request); // "dependency injection" for a function diff --git a/phpBB/style.php b/phpBB/style.php index 8ca1751391..026b5eecfc 100644 --- a/phpBB/style.php +++ b/phpBB/style.php @@ -55,15 +55,20 @@ $id = (isset($_GET['id'])) ? intval($_GET['id']) : 0; if ($id) { // Include files + require($phpbb_root_path . 'includes/class_loader.' . $phpEx); require($phpbb_root_path . 'includes/acm/acm_' . $acm_type . '.' . $phpEx); require($phpbb_root_path . 'includes/cache.' . $phpEx); require($phpbb_root_path . 'includes/db/' . $dbms . '.' . $phpEx); require($phpbb_root_path . 'includes/constants.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); - $db = new $sql_db(); $cache = new cache(); + $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); + $class_loader->register(); + + $db = new $sql_db(); + // Connect to DB if (!@$db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, false)) { diff --git a/tests/request/deactivated_super_global.php b/tests/request/deactivated_super_global.php index 2991badd1a..3c7a638e38 100644 --- a/tests/request/deactivated_super_global.php +++ b/tests/request/deactivated_super_global.php @@ -9,6 +9,7 @@ */ require_once 'test_framework/framework.php'; +require_once '../phpBB/includes/request/interface.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; class phpbb_deactivated_super_global_test extends phpbb_test_case @@ -19,7 +20,7 @@ class phpbb_deactivated_super_global_test extends phpbb_test_case public function test_write_triggers_error() { $this->setExpectedTriggerError(E_USER_ERROR); - $obj = new phpbb_deactivated_super_global($this->getMock('phpbb_request_interface'), 'obj', phpbb_request_interface::POST); + $obj = new phpbb_request_deactivated_super_global($this->getMock('phpbb_request_interface'), 'obj', phpbb_request_interface::POST); $obj->offsetSet(0, 0); } } diff --git a/tests/request/request.php b/tests/request/request.php index ebfc3ba2b0..cf275e763c 100644 --- a/tests/request/request.php +++ b/tests/request/request.php @@ -10,7 +10,7 @@ require_once 'test_framework/framework.php'; require_once '../phpBB/includes/request/type_cast_helper_interface.php'; -require_once '../phpBB/includes/request/request_interface.php'; +require_once '../phpBB/includes/request/interface.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; require_once '../phpBB/includes/request/request.php'; @@ -28,7 +28,7 @@ class phpbb_request_test extends phpbb_test_case $_REQUEST['test'] = 3; $_GET['unset'] = ''; - $this->type_cast_helper = $this->getMock('phpbb_type_cast_helper_interface'); + $this->type_cast_helper = $this->getMock('phpbb_request_type_cast_helper_interface'); $this->request = new phpbb_request($this->type_cast_helper); } diff --git a/tests/request/request_var.php b/tests/request/request_var.php index 5bdcb5d4e7..857dbe55a5 100644 --- a/tests/request/request_var.php +++ b/tests/request/request_var.php @@ -11,7 +11,7 @@ require_once 'test_framework/framework.php'; require_once '../phpBB/includes/request/type_cast_helper_interface.php'; require_once '../phpBB/includes/request/type_cast_helper.php'; require_once '../phpBB/includes/request/deactivated_super_global.php'; -require_once '../phpBB/includes/request/request_interface.php'; +require_once '../phpBB/includes/request/interface.php'; require_once '../phpBB/includes/request/request.php'; require_once '../phpBB/includes/functions.php'; diff --git a/tests/request/type_cast_helper.php b/tests/request/type_cast_helper.php index aba8523ec3..51687c186d 100644 --- a/tests/request/type_cast_helper.php +++ b/tests/request/type_cast_helper.php @@ -18,7 +18,7 @@ class phpbb_type_cast_helper_test extends phpbb_test_case protected function setUp() { - $this->type_cast_helper = new phpbb_type_cast_helper(); + $this->type_cast_helper = new phpbb_request_type_cast_helper(); } public function test_addslashes_recursively() From 204ee4714b2a0be1513e6a30b255477f39dac5cb Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Tue, 31 Aug 2010 21:29:11 +0200 Subject: [PATCH 12/21] [feature/request-class] Removal of direct access to some superglobals PHPBB3-9716 --- phpBB/includes/functions.php | 12 ++---------- phpBB/ucp.php | 6 +++++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 029281ee84..81e327143a 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2698,22 +2698,14 @@ function check_form_key($form_name, $timespan = false, $return_page = '', $trigg function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_body.html', $u_action = '') { global $user, $template, $db; - global $phpEx, $phpbb_root_path; + global $phpEx, $phpbb_root_path, $request; if (isset($_POST['cancel'])) { return false; } - $confirm = false; - if (isset($_POST['confirm'])) - { - // language frontier - if ($_POST['confirm'] === $user->lang['YES']) - { - $confirm = true; - } - } + $confirm = ($user->lang['YES'] === $request->variable('confirm', '', true, phpbb_request_interface::POST)); if ($check && $confirm) { diff --git a/phpBB/ucp.php b/phpBB/ucp.php index f5a2ec9648..f26f7b048e 100644 --- a/phpBB/ucp.php +++ b/phpBB/ucp.php @@ -136,13 +136,17 @@ switch ($mode) case 'delete_cookies': + global $request; + // Delete Cookies with dynamic names (do NOT delete poll cookies) if (confirm_box(true)) { $set_time = time() - 31536000; - foreach ($_COOKIE as $cookie_name => $cookie_data) + foreach ($request->variable_names(phpbb_request_interface::COOKIE) as $cookie_name) { + $cookie_data = $request->variable($cookie_name, '', true, phpbb_request_interface::COOKIE); + // Only delete board cookies, no other ones... if (strpos($cookie_name, $config['cookie_name'] . '_') !== 0) { From 55808e11c987b72b6c2d364d4f17e11dfbbebc71 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Sun, 19 Sep 2010 15:31:00 +0200 Subject: [PATCH 13/21] [feature/request-class] Prevent recursive_set_var from applying htmlspecialchars twice PHPBB3-9716 --- phpBB/includes/request/type_cast_helper.php | 2 +- tests/request/type_cast_helper.php | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php index c39bbebe01..29855a9804 100644 --- a/phpBB/includes/request/type_cast_helper.php +++ b/phpBB/includes/request/type_cast_helper.php @@ -176,7 +176,7 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i $this->set_var($k, $k, $key_type, $multibyte, $multibyte); $this->recursive_set_var($v, $default_value, $multibyte); - $this->set_var($var[$k], $v, $value_type, $multibyte); + $var[$k] = $v; } } } diff --git a/tests/request/type_cast_helper.php b/tests/request/type_cast_helper.php index 51687c186d..291b414fd3 100644 --- a/tests/request/type_cast_helper.php +++ b/tests/request/type_cast_helper.php @@ -9,6 +9,7 @@ */ require_once 'test_framework/framework.php'; +require_once '../phpBB/includes/utf/utf_tools.php'; require_once '../phpBB/includes/request/type_cast_helper_interface.php'; require_once '../phpBB/includes/request/type_cast_helper.php'; @@ -30,4 +31,24 @@ class phpbb_type_cast_helper_test extends phpbb_test_case $this->assertEquals($expected, $data); } + + public function test_simple_recursive_set_var() + { + $data = 'eviL<3'; + $expected = 'eviL<3'; + + $this->type_cast_helper->recursive_set_var($data, '', true); + + $this->assertEquals($expected, $data); + } + + public function test_nested_recursive_set_var() + { + $data = array('eviL<3'); + $expected = array('eviL<3'); + + $this->type_cast_helper->recursive_set_var($data, array(0 => ''), true); + + $this->assertEquals($expected, $data); + } } From 15883dfac22c8a5660c0400c565ba980eaf6f618 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Mon, 20 Sep 2010 18:52:08 +0200 Subject: [PATCH 14/21] [feature/request-class] Add $request to style.php, minor change Add $request instantiation to style.php to allow request_var to work properly. Also remove unneeded globalization of $request in ucp.php. PHPBB3-9716 --- phpBB/style.php | 5 +++++ phpBB/ucp.php | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/phpBB/style.php b/phpBB/style.php index 026b5eecfc..368a7132e4 100644 --- a/phpBB/style.php +++ b/phpBB/style.php @@ -45,6 +45,7 @@ if (!empty($load_extensions) && function_exists('dl')) } } +// no $request here because it is not loaded yet $id = (isset($_GET['id'])) ? intval($_GET['id']) : 0; // This is a simple script to grab and output the requested CSS data stored in the DB @@ -67,8 +68,12 @@ if ($id) $class_loader = new phpbb_class_loader($phpbb_root_path, '.' . $phpEx, $cache); $class_loader->register(); + $request = new phpbb_request(); $db = new $sql_db(); + // make sure request_var uses this request instance + request_var('', 0, false, false, $request); // "dependency injection" for a function + // Connect to DB if (!@$db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, false)) { diff --git a/phpBB/ucp.php b/phpBB/ucp.php index f26f7b048e..2d190cb1a9 100644 --- a/phpBB/ucp.php +++ b/phpBB/ucp.php @@ -136,8 +136,6 @@ switch ($mode) case 'delete_cookies': - global $request; - // Delete Cookies with dynamic names (do NOT delete poll cookies) if (confirm_box(true)) { From fccd7f0ab5ab559dc89be6af9e582a986af8bb13 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Wed, 22 Sep 2010 21:58:20 +0200 Subject: [PATCH 15/21] [feature/request-class] Convert any direct access to $_* to use $request PHPBB3-9716 --- phpBB/adm/index.php | 3 +- phpBB/includes/acp/acp_attachments.php | 3 +- phpBB/includes/acp/acp_icons.php | 5 ++- phpBB/includes/acp/acp_language.php | 47 ++++++++++++++------- phpBB/includes/acp/acp_logs.php | 5 ++- phpBB/includes/acp/acp_permissions.php | 17 ++++---- phpBB/includes/acp/acp_profile.php | 11 +++-- phpBB/includes/db/dbal.php | 3 +- phpBB/includes/functions.php | 8 ++-- phpBB/includes/functions_display.php | 7 +-- phpBB/includes/functions_module.php | 3 +- phpBB/includes/functions_profile_fields.php | 6 ++- phpBB/includes/mcp/mcp_forum.php | 6 ++- phpBB/includes/mcp/mcp_main.php | 9 ++-- phpBB/includes/mcp/mcp_queue.php | 3 +- phpBB/includes/message_parser.php | 7 +-- phpBB/includes/ucp/ucp_groups.php | 5 ++- phpBB/includes/ucp/ucp_main.php | 3 +- phpBB/includes/ucp/ucp_pm_compose.php | 22 ++++------ phpBB/includes/ucp/ucp_pm_viewmessage.php | 2 +- phpBB/includes/ucp/ucp_profile.php | 7 +-- phpBB/includes/ucp/ucp_register.php | 11 ++--- phpBB/install/install_convert.php | 5 ++- phpBB/install/install_update.php | 11 ++--- phpBB/mcp.php | 15 ++----- phpBB/memberlist.php | 2 +- phpBB/posting.php | 2 +- phpBB/ucp.php | 2 +- 28 files changed, 131 insertions(+), 99 deletions(-) diff --git a/phpBB/adm/index.php b/phpBB/adm/index.php index 92bcf90039..2e07e58d0f 100644 --- a/phpBB/adm/index.php +++ b/phpBB/adm/index.php @@ -163,6 +163,7 @@ function adm_page_footer($copyright_html = true) { global $db, $config, $template, $user, $auth, $cache; global $starttime, $phpbb_root_path, $phpbb_admin_path, $phpEx; + global $request; // Output page creation time if (defined('DEBUG')) @@ -170,7 +171,7 @@ function adm_page_footer($copyright_html = true) $mtime = explode(' ', microtime()); $totaltime = $mtime[0] + $mtime[1] - $starttime; - if (!empty($_REQUEST['explain']) && $auth->acl_get('a_') && defined('DEBUG_EXTRA') && method_exists($db, 'sql_report')) + if ($request->variable('explain', false) && $auth->acl_get('a_') && defined('DEBUG_EXTRA') && method_exists($db, 'sql_report')) { $db->sql_report('display'); } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index fc5f44e14f..68870dce54 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -1235,6 +1235,7 @@ class acp_attachments function perform_site_list() { global $db, $user; + global $request; if (isset($_REQUEST['securesubmit'])) { @@ -1243,7 +1244,7 @@ class acp_attachments $ip_list = array_unique(explode("\n", $ips)); $ip_list_log = implode(', ', $ip_list); - $ip_exclude = (!empty($_POST['ipexclude'])) ? 1 : 0; + $ip_exclude = (int) $request->variable('ipexclude', false, false, phpbb_request_interface::POST); $iplist = array(); $hostlist = array(); diff --git a/phpBB/includes/acp/acp_icons.php b/phpBB/includes/acp/acp_icons.php index 3d64a2acda..43412f3c47 100644 --- a/phpBB/includes/acp/acp_icons.php +++ b/phpBB/includes/acp/acp_icons.php @@ -28,6 +28,7 @@ class acp_icons { global $db, $user, $auth, $template, $cache; global $config, $phpbb_root_path, $phpbb_admin_path, $phpEx; + global $request; $user->add_lang('acp/posting'); @@ -338,7 +339,7 @@ class acp_icons $image_display_on_posting = (isset($_POST['display_on_posting'])) ? request_var('display_on_posting', array('' => 0)) : array(); // Ok, add the relevant bits if we are adding new codes to existing emoticons... - if (!empty($_POST['add_additional_code'])) + if ($request->variable('add_additional_code', false, false, phpbb_request_interface::POST)) { $add_image = request_var('add_image', ''); $add_code = utf8_normalize_nfc(request_var('add_code', '', true)); @@ -354,7 +355,7 @@ class acp_icons $image_width[$add_image] = request_var('add_width', 0); $image_height[$add_image] = request_var('add_height', 0); - if (!empty($_POST['add_display_on_posting'])) + if ($request->variable('add_display_on_posting', false, false, phpbb_request_interface::POST)) { $image_display_on_posting[$add_image] = 1; } diff --git a/phpBB/includes/acp/acp_language.php b/phpBB/includes/acp/acp_language.php index c2cb2f9c11..0f924dc95f 100644 --- a/phpBB/includes/acp/acp_language.php +++ b/phpBB/includes/acp/acp_language.php @@ -34,6 +34,7 @@ class acp_language global $config, $db, $user, $auth, $template, $cache; global $phpbb_root_path, $phpbb_admin_path, $phpEx, $table_prefix; global $safe_mode, $file_uploads; + global $request; include_once($phpbb_root_path . 'includes/functions_user.' . $phpEx); @@ -58,7 +59,7 @@ class acp_language if (isset($_POST['missing_file'])) { $missing_file = request_var('missing_file', array('' => 0)); - list($_REQUEST['language_file'], ) = array_keys($missing_file); + $request->overwrite('language_file', array_shift(array_keys($missing_file))); } $selected_lang_file = request_var('language_file', '|common.' . $phpEx); @@ -68,6 +69,23 @@ class acp_language $this->language_directory = basename($this->language_directory); $this->language_file = basename($this->language_file); + // detect language file type + if ($this->language_directory == 'email') + { + $language_file_type = 'email'; + $request_default = ''; + } + else if (strpos($this->language_file, 'help_') === 0) + { + $language_file_type = 'help'; + $request_default = array(0 => array(0 => '')); + } + else + { + $language_file_type = 'normal'; + $request_default = array('' => ''); + } + $user->add_lang('acp/language'); $this->tpl_name = 'acp_language'; $this->page_title = 'ACP_LANGUAGE_PACKS'; @@ -119,7 +137,7 @@ class acp_language 'DATA' => $data, 'NAME' => $user->lang[strtoupper($method . '_' . $data)], 'EXPLAIN' => $user->lang[strtoupper($method . '_' . $data) . '_EXPLAIN'], - 'DEFAULT' => (!empty($_REQUEST[$data])) ? request_var($data, '') : $default + 'DEFAULT' => $request->variable($data, (string) $default), )); } @@ -130,7 +148,7 @@ class acp_language 'method' => $method) ); - $hidden_data .= build_hidden_fields(array('entry' => $_POST['entry']), true, STRIP); + $hidden_data .= build_hidden_fields(array('entry' => $request->variable('entry', $request_default, true, phpbb_request_interface::POST))); $template->assign_vars(array( 'S_UPLOAD' => true, @@ -187,12 +205,9 @@ class acp_language trigger_error($user->lang['FORM_INVALID']. adm_back_link($this->u_action), E_USER_WARNING); } - if (!$lang_id || empty($_POST['entry'])) - { - trigger_error($user->lang['NO_LANG_ID'] . adm_back_link($this->u_action), E_USER_WARNING); - } + $entry_value = $request->variable('entry', $request_default, true, phpbb_request_interface::POST); - if ($this->language_directory != 'email' && !is_array($_POST['entry'])) + if (!$lang_id || !$entry_value) { trigger_error($user->lang['NO_LANG_ID'] . adm_back_link($this->u_action), E_USER_WARNING); } @@ -291,10 +306,10 @@ class acp_language trigger_error(sprintf($user->lang['UNABLE_TO_WRITE_FILE'], $filename) . adm_back_link($this->u_action . '&id=' . $lang_id . '&action=details&language_file=' . urlencode($selected_lang_file)), E_USER_WARNING); } - if ($this->language_directory == 'email') + if ($language_file_type == 'email') { // Email Template - $entry = $this->prepare_lang_entry($_POST['entry'], false); + $entry = $this->prepare_lang_entry(htmlspecialchars_decode($entry_value), false); fwrite($fp, $entry); } else @@ -302,13 +317,13 @@ class acp_language $name = (($this->language_directory) ? $this->language_directory . '_' : '') . $this->language_file; $header = str_replace(array('{FILENAME}', '{LANG_NAME}', '{CHANGED}', '{AUTHOR}'), array($name, $row['lang_english_name'], date('Y-m-d', time()), $row['lang_author']), $this->language_file_header); - if (strpos($this->language_file, 'help_') === 0) + if ($language_file_type == 'help') { // Help File $header .= '$help = array(' . "\n"; fwrite($fp, $header); - foreach ($_POST['entry'] as $key => $value) + foreach ($entry_value as $key => $value) { if (!is_array($value)) { @@ -319,7 +334,7 @@ class acp_language foreach ($value as $_key => $_value) { - $entry .= "\t\t" . (int) $_key . "\t=> '" . $this->prepare_lang_entry($_value) . "',\n"; + $entry .= "\t\t" . (int) $_key . "\t=> '" . $this->prepare_lang_entry(htmlspecialchars_decode($_value)) . "',\n"; } $entry .= "\t),\n"; @@ -329,15 +344,15 @@ class acp_language $footer = ");\n\n?>"; fwrite($fp, $footer); } - else + else if ($language_file_type == 'normal') { // Language File $header .= $this->lang_header; fwrite($fp, $header); - foreach ($_POST['entry'] as $key => $value) + foreach ($entry_value as $key => $value) { - $entry = $this->format_lang_array($key, $value); + $entry = $this->format_lang_array(htmlspecialchars_decode($key), htmlspecialchars_decode($value)); fwrite($fp, $entry); } diff --git a/phpBB/includes/acp/acp_logs.php b/phpBB/includes/acp/acp_logs.php index 0f4f78fcdd..e37b696873 100644 --- a/phpBB/includes/acp/acp_logs.php +++ b/phpBB/includes/acp/acp_logs.php @@ -27,6 +27,7 @@ class acp_logs { global $db, $user, $auth, $template, $cache; global $config, $phpbb_root_path, $phpbb_admin_path, $phpEx; + global $request; $user->add_lang('mcp'); @@ -35,8 +36,8 @@ class acp_logs $forum_id = request_var('f', 0); $topic_id = request_var('t', 0); $start = request_var('start', 0); - $deletemark = (!empty($_POST['delmarked'])) ? true : false; - $deleteall = (!empty($_POST['delall'])) ? true : false; + $deletemark = $request->variable('delmarked', false, false, phpbb_request_interface::POST); + $deleteall = $request->variable('delall', false, false, phpbb_request_interface::POST); $marked = request_var('mark', array(0)); // Sort keys diff --git a/phpBB/includes/acp/acp_permissions.php b/phpBB/includes/acp/acp_permissions.php index e9f0af5071..50d7357100 100644 --- a/phpBB/includes/acp/acp_permissions.php +++ b/phpBB/includes/acp/acp_permissions.php @@ -658,6 +658,7 @@ class acp_permissions function set_permissions($mode, $permission_type, &$auth_admin, &$user_id, &$group_id) { global $user, $auth; + global $request; $psubmit = request_var('psubmit', array(0 => array(0 => 0))); @@ -676,18 +677,17 @@ class acp_permissions list($ug_id, ) = each($psubmit); list($forum_id, ) = each($psubmit[$ug_id]); - if (empty($_POST['setting']) || empty($_POST['setting'][$ug_id]) || empty($_POST['setting'][$ug_id][$forum_id]) || !is_array($_POST['setting'][$ug_id][$forum_id])) + $settings = $request->variable('setting', array(0 => array(0 => array('' => 0))), false, phpbb_request_interface::POST); + if (empty($settings) || empty($settings[$ug_id]) || empty($settings[$ug_id][$forum_id])) { trigger_error('WRONG_PERMISSION_SETTING_FORMAT', E_USER_WARNING); } - // We obtain and check $_POST['setting'][$ug_id][$forum_id] directly and not using request_var() because request_var() - // currently does not support the amount of dimensions required. ;) - // $auth_settings = request_var('setting', array(0 => array(0 => array('' => 0)))); - $auth_settings = array_map('intval', $_POST['setting'][$ug_id][$forum_id]); + $auth_settings = $settings[$ug_id][$forum_id]; // Do we have a role we want to set? - $assigned_role = (isset($_POST['role'][$ug_id][$forum_id])) ? (int) $_POST['role'][$ug_id][$forum_id] : 0; + $roles = $request->variable('role', array(0 => array(0 => 0)), false, phpbb_request_interface::POST); + $assigned_role = (isset($roles[$ug_id][$forum_id])) ? (int) $roles[$ug_id][$forum_id] : 0; // Do the admin want to set these permissions to other items too? $inherit = request_var('inherit', array(0 => array(0))); @@ -747,6 +747,7 @@ class acp_permissions function set_all_permissions($mode, $permission_type, &$auth_admin, &$user_id, &$group_id) { global $user, $auth; + global $request; // User or group to be set? $ug_type = (sizeof($user_id)) ? 'user' : 'group'; @@ -757,8 +758,8 @@ class acp_permissions trigger_error($user->lang['NO_AUTH_OPERATION'] . adm_back_link($this->u_action), E_USER_WARNING); } - $auth_settings = (isset($_POST['setting'])) ? $_POST['setting'] : array(); - $auth_roles = (isset($_POST['role'])) ? $_POST['role'] : array(); + $auth_settings = $request->variable('setting', array(0 => array(0 => array('' => 0))), false, phpbb_request_interface::POST); + $auth_roles = $request->variable('role', array(0 => array(0 => 0)), false, phpbb_request_interface::POST); $ug_ids = $forum_ids = array(); // We need to go through the auth settings diff --git a/phpBB/includes/acp/acp_profile.php b/phpBB/includes/acp/acp_profile.php index 2288a0728b..776a59c8c7 100644 --- a/phpBB/includes/acp/acp_profile.php +++ b/phpBB/includes/acp/acp_profile.php @@ -30,6 +30,7 @@ class acp_profile { global $config, $db, $user, $auth, $template, $cache; global $phpbb_root_path, $phpbb_admin_path, $phpEx, $table_prefix; + global $request; include($phpbb_root_path . 'includes/functions_posting.' . $phpEx); include($phpbb_root_path . 'includes/functions_user.' . $phpEx); @@ -487,7 +488,8 @@ class acp_profile $cp->vars['field_default_value_day'] = $now['mday']; $cp->vars['field_default_value_month'] = $now['mon']; $cp->vars['field_default_value_year'] = $now['year']; - $var = $_POST['field_default_value'] = 'now'; + $var = 'now'; + $request->overwrite('field_default_value', $var, phpbb_request_interface::POST); } else { @@ -496,7 +498,8 @@ class acp_profile $cp->vars['field_default_value_day'] = request_var('field_default_value_day', 0); $cp->vars['field_default_value_month'] = request_var('field_default_value_month', 0); $cp->vars['field_default_value_year'] = request_var('field_default_value_year', 0); - $var = $_POST['field_default_value'] = sprintf('%2d-%2d-%4d', $cp->vars['field_default_value_day'], $cp->vars['field_default_value_month'], $cp->vars['field_default_value_year']); + $var = sprintf('%2d-%2d-%4d', $cp->vars['field_default_value_day'], $cp->vars['field_default_value_month'], $cp->vars['field_default_value_year']); + $request->overwrite('field_default_value', $var, phpbb_request_interface::POST); } else { @@ -688,7 +691,7 @@ class acp_profile } else { - $_new_key_ary[$key] = (is_array($_REQUEST[$key])) ? utf8_normalize_nfc(request_var($key, array(''), true)) : utf8_normalize_nfc(request_var($key, '', true)); + $_new_key_ary[$key] = ($request->is_array($key)) ? utf8_normalize_nfc(request_var($key, array(''), true)) : utf8_normalize_nfc(request_var($key, '', true)); } } } @@ -1623,4 +1626,4 @@ class acp_profile } } -?> +?> \ No newline at end of file diff --git a/phpBB/includes/db/dbal.php b/phpBB/includes/db/dbal.php index eeddf1f41b..9a0e9f1b55 100644 --- a/phpBB/includes/db/dbal.php +++ b/phpBB/includes/db/dbal.php @@ -711,8 +711,9 @@ class dbal function sql_report($mode, $query = '') { global $cache, $starttime, $phpbb_root_path, $user; + global $request; - if (empty($_REQUEST['explain'])) + if (!$request->variable('explain', false)) { return false; } diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 81e327143a..25cc1f9960 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2793,6 +2793,7 @@ function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_bo function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = false, $s_display = true) { global $db, $user, $template, $auth, $phpEx, $phpbb_root_path, $config; + global $request; if (!class_exists('phpbb_captcha_factory', false)) { @@ -2843,8 +2844,8 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa } $username = request_var('username', '', true); - $autologin = (!empty($_POST['autologin'])) ? true : false; - $viewonline = (!empty($_POST['viewonline'])) ? 0 : 1; + $autologin = $request->variable('autologin', false, false, phpbb_request_interface::POST); + $viewonline = (int) $request->variable('viewonline', false, false, phpbb_request_interface::POST); $admin = ($admin) ? 1 : 0; $viewonline = ($admin) ? $user->data['session_viewonline'] : $viewonline; @@ -4449,6 +4450,7 @@ function page_header($page_title = '', $display_online_list = true, $item_id = 0 function page_footer($run_cron = true) { global $db, $config, $template, $user, $auth, $cache, $starttime, $phpbb_root_path, $phpEx; + global $request; // Output page creation time if (defined('DEBUG')) @@ -4456,7 +4458,7 @@ function page_footer($run_cron = true) $mtime = explode(' ', microtime()); $totaltime = $mtime[0] + $mtime[1] - $starttime; - if (!empty($_REQUEST['explain']) && $auth->acl_get('a_') && defined('DEBUG_EXTRA') && method_exists($db, 'sql_report')) + if ($request->variable('explain', false) && $auth->acl_get('a_') && defined('DEBUG_EXTRA') && method_exists($db, 'sql_report')) { $db->sql_report('display'); } diff --git a/phpBB/includes/functions_display.php b/phpBB/includes/functions_display.php index 2de7e1b169..7f9070740f 100644 --- a/phpBB/includes/functions_display.php +++ b/phpBB/includes/functions_display.php @@ -1062,6 +1062,7 @@ function display_user_activity(&$userdata) function watch_topic_forum($mode, &$s_watching, $user_id, $forum_id, $topic_id, $notify_status = 'unset', $start = 0) { global $template, $db, $user, $phpEx, $start, $phpbb_root_path; + global $request; $table_sql = ($mode == 'forum') ? FORUMS_WATCH_TABLE : TOPICS_WATCH_TABLE; $where_sql = ($mode == 'forum') ? 'forum_id' : 'topic_id'; @@ -1098,7 +1099,7 @@ function watch_topic_forum($mode, &$s_watching, $user_id, $forum_id, $topic_id, $message = $user->lang['ERR_UNWATCHING'] . '

' . sprintf($user->lang['RETURN_' . strtoupper($mode)], '', ''); trigger_error($message); } - if ($_GET['unwatch'] == $mode) + if ($request->variable('unwatch', '', false, phpbb_request_interface::GET) == $mode) { $is_watching = 0; @@ -1136,7 +1137,7 @@ function watch_topic_forum($mode, &$s_watching, $user_id, $forum_id, $topic_id, $token = request_var('hash', ''); $redirect_url = append_sid("{$phpbb_root_path}view$mode.$phpEx", "$u_url=$match_id&start=$start"); - if ($_GET['watch'] == $mode && check_link_hash($token, "{$mode}_$match_id")) + if ($request->variable('watch', '', false, phpbb_request_interface::GET) == $mode && check_link_hash($token, "{$mode}_$match_id")) { $is_watching = true; @@ -1162,7 +1163,7 @@ function watch_topic_forum($mode, &$s_watching, $user_id, $forum_id, $topic_id, } else { - if (isset($_GET['unwatch']) && $_GET['unwatch'] == $mode) + if ($request->variable('unwatch', '', false, phpbb_request_interface::GET) == $mode) { login_box(); } diff --git a/phpBB/includes/functions_module.php b/phpBB/includes/functions_module.php index d0e7c8cfc8..194cba7291 100644 --- a/phpBB/includes/functions_module.php +++ b/phpBB/includes/functions_module.php @@ -314,6 +314,7 @@ class p_master function module_auth($module_auth, $forum_id = false) { global $auth, $config; + global $request; $module_auth = trim($module_auth); @@ -361,7 +362,7 @@ class p_master $forum_id = ($forum_id === false) ? $this->acl_forum_id : $forum_id; $is_auth = false; - eval('$is_auth = (int) (' . preg_replace(array('#acl_([a-z0-9_]+)(,\$id)?#', '#\$id#', '#aclf_([a-z0-9_]+)#', '#cfg_([a-z0-9_]+)#', '#request_([a-zA-Z0-9_]+)#'), array('(int) $auth->acl_get(\'\\1\'\\2)', '(int) $forum_id', '(int) $auth->acl_getf_global(\'\\1\')', '(int) $config[\'\\1\']', '!empty($_REQUEST[\'\\1\'])'), $module_auth) . ');'); + eval('$is_auth = (int) (' . preg_replace(array('#acl_([a-z0-9_]+)(,\$id)?#', '#\$id#', '#aclf_([a-z0-9_]+)#', '#cfg_([a-z0-9_]+)#', '#request_([a-zA-Z0-9_]+)#'), array('(int) $auth->acl_get(\'\\1\'\\2)', '(int) $forum_id', '(int) $auth->acl_getf_global(\'\\1\')', '(int) $config[\'\\1\']', '$request->variable(\'\\1\', false)'), $module_auth) . ');'); return $is_auth; } diff --git a/phpBB/includes/functions_profile_fields.php b/phpBB/includes/functions_profile_fields.php index 3937cf9c21..3ad06a3383 100644 --- a/phpBB/includes/functions_profile_fields.php +++ b/phpBB/includes/functions_profile_fields.php @@ -610,6 +610,7 @@ class custom_profile function get_var($field_validation, &$profile_row, $default_value, $preview) { global $user; + global $request; $profile_row['field_ident'] = (isset($profile_row['var_name'])) ? $profile_row['var_name'] : 'pf_' . $profile_row['field_ident']; $user_ident = $profile_row['field_ident']; @@ -622,7 +623,7 @@ class custom_profile { if (isset($_REQUEST[$profile_row['field_ident']])) { - $value = ($_REQUEST[$profile_row['field_ident']] === '') ? NULL : request_var($profile_row['field_ident'], $default_value); + $value = ($request->variable($profile_row['field_ident'], '') === '') ? NULL : $request->variable($profile_row['field_ident'], $default_value); } else { @@ -894,6 +895,7 @@ class custom_profile { global $phpbb_root_path, $phpEx; global $config; + global $request; $var_name = 'pf_' . $profile_row['field_ident']; @@ -938,7 +940,7 @@ class custom_profile break; case FIELD_INT: - if (isset($_REQUEST[$var_name]) && $_REQUEST[$var_name] === '') + if (isset($_REQUEST[$var_name]) && $request->variable($var_name, '') === '') { $var = NULL; } diff --git a/phpBB/includes/mcp/mcp_forum.php b/phpBB/includes/mcp/mcp_forum.php index b70601b479..ddd13cb080 100644 --- a/phpBB/includes/mcp/mcp_forum.php +++ b/phpBB/includes/mcp/mcp_forum.php @@ -23,6 +23,7 @@ function mcp_forum_view($id, $mode, $action, $forum_info) { global $template, $db, $user, $auth, $cache, $module; global $phpEx, $phpbb_root_path, $config; + global $request; $user->add_lang(array('viewtopic', 'viewforum')); @@ -34,7 +35,10 @@ function mcp_forum_view($id, $mode, $action, $forum_info) if ($merge_select) { // Fixes a "bug" that makes forum_view use the same ordering as topic_view - unset($_POST['sk'], $_POST['sd'], $_REQUEST['sk'], $_REQUEST['sd']); + $request->overwrite('sk', null); + $request->overwrite('sd', null); + $request->overwrite('sk', null, phpbb_request_interface::POST); + $request->overwrite('sd', null, phpbb_request_interface::POST); } $forum_id = $forum_info['forum_id']; diff --git a/phpBB/includes/mcp/mcp_main.php b/phpBB/includes/mcp/mcp_main.php index d5551f5114..14286f98e9 100644 --- a/phpBB/includes/mcp/mcp_main.php +++ b/phpBB/includes/mcp/mcp_main.php @@ -532,6 +532,7 @@ function mcp_move_topic($topic_ids) { global $auth, $user, $db, $template; global $phpEx, $phpbb_root_path; + global $request; // Here we limit the operation to one forum only $forum_id = check_ids($topic_ids, TOPICS_TABLE, 'topic_id', array('m_move'), true); @@ -585,8 +586,8 @@ function mcp_move_topic($topic_ids) if (!$to_forum_id || $additional_msg) { - unset($_POST['confirm']); - unset($_REQUEST['confirm_key']); + $request->overwrite('confirm', null, phpbb_request_interface::POST); + $request->overwrite('confirm_key', null); } if (confirm_box(true)) @@ -1037,8 +1038,8 @@ function mcp_fork_topic($topic_ids) if ($additional_msg) { - unset($_POST['confirm']); - unset($_REQUEST['confirm_key']); + $request->overwrite('confirm', null, phpbb_request_interface::POST); + $request->overwrite('confirm_key', null); } if (confirm_box(true)) diff --git a/phpBB/includes/mcp/mcp_queue.php b/phpBB/includes/mcp/mcp_queue.php index c419da5574..dc34f04db4 100644 --- a/phpBB/includes/mcp/mcp_queue.php +++ b/phpBB/includes/mcp/mcp_queue.php @@ -744,6 +744,7 @@ function disapprove_post($post_id_list, $id, $mode) { global $db, $template, $user, $config; global $phpEx, $phpbb_root_path; + global $request; if (!check_ids($post_id_list, POSTS_TABLE, 'post_id', array('m_approve'))) { @@ -778,7 +779,7 @@ function disapprove_post($post_id_list, $id, $mode) if (!$row || (!$reason && strtolower($row['reason_title']) == 'other')) { $additional_msg = $user->lang['NO_REASON_DISAPPROVAL']; - unset($_POST['confirm']); + $request->overwrite('confirm', null, phpbb_request_interface::POST); } else { diff --git a/phpBB/includes/message_parser.php b/phpBB/includes/message_parser.php index 952b55cc8c..e0b2bb1496 100644 --- a/phpBB/includes/message_parser.php +++ b/phpBB/includes/message_parser.php @@ -1532,9 +1532,10 @@ class parse_message extends bbcode_firstpass function get_submitted_attachment_data($check_user_id = false) { global $user, $db, $phpbb_root_path, $phpEx, $config; + global $request; $this->filename_data['filecomment'] = utf8_normalize_nfc(request_var('filecomment', '', true)); - $attachment_data = (isset($_POST['attachment_data'])) ? $_POST['attachment_data'] : array(); + $attachment_data = $request->variable('attachment_data', array(0 => array('' => '')), true, phpbb_request_interface::POST); $this->attachment_data = array(); $check_user_id = ($check_user_id === false) ? $user->data['user_id'] : $check_user_id; @@ -1572,7 +1573,7 @@ class parse_message extends bbcode_firstpass { $pos = $not_orphan[$row['attach_id']]; $this->attachment_data[$pos] = $row; - set_var($this->attachment_data[$pos]['attach_comment'], $_POST['attachment_data'][$pos]['attach_comment'], 'string', true); + $this->attachment_data[$pos]['attach_comment'] = $attachment_data[$pos]['attach_comment']; unset($not_orphan[$row['attach_id']]); } @@ -1598,7 +1599,7 @@ class parse_message extends bbcode_firstpass { $pos = $orphan[$row['attach_id']]; $this->attachment_data[$pos] = $row; - set_var($this->attachment_data[$pos]['attach_comment'], $_POST['attachment_data'][$pos]['attach_comment'], 'string', true); + $this->attachment_data[$pos]['attach_comment'] = $attachment_data[$pos]['attach_comment']; unset($orphan[$row['attach_id']]); } diff --git a/phpBB/includes/ucp/ucp_groups.php b/phpBB/includes/ucp/ucp_groups.php index 1c055a4823..433b9af9d1 100644 --- a/phpBB/includes/ucp/ucp_groups.php +++ b/phpBB/includes/ucp/ucp_groups.php @@ -28,14 +28,15 @@ class ucp_groups { global $config, $phpbb_root_path, $phpEx; global $db, $user, $auth, $cache, $template; + global $request; $user->add_lang('groups'); $return_page = '

' . sprintf($user->lang['RETURN_PAGE'], '', ''); $mark_ary = request_var('mark', array(0)); - $submit = (!empty($_POST['submit'])) ? true : false; - $delete = (!empty($_POST['delete'])) ? true : false; + $submit = $request->variable('submit', false, false, phpbb_request_interface::POST); + $delete = $request->variable('delete', false, false, phpbb_request_interface::POST); $error = $data = array(); switch ($mode) diff --git a/phpBB/includes/ucp/ucp_main.php b/phpBB/includes/ucp/ucp_main.php index a6f71669ce..3fde308309 100644 --- a/phpBB/includes/ucp/ucp_main.php +++ b/phpBB/includes/ucp/ucp_main.php @@ -34,6 +34,7 @@ class ucp_main function main($id, $mode) { global $config, $db, $user, $auth, $template, $phpbb_root_path, $phpEx; + global $request; switch ($mode) { @@ -435,7 +436,7 @@ class ucp_main $edit = (isset($_REQUEST['edit'])) ? true : false; $submit = (isset($_POST['submit'])) ? true : false; - $draft_id = ($edit) ? intval($_REQUEST['edit']) : 0; + $draft_id = $request->variable('edit', 0); $delete = (isset($_POST['delete'])) ? true : false; $s_hidden_fields = ($edit) ? '' : ''; diff --git a/phpBB/includes/ucp/ucp_pm_compose.php b/phpBB/includes/ucp/ucp_pm_compose.php index b596e72c41..e7c0244b99 100644 --- a/phpBB/includes/ucp/ucp_pm_compose.php +++ b/phpBB/includes/ucp/ucp_pm_compose.php @@ -24,6 +24,7 @@ function compose_pm($id, $mode, $action) { global $template, $db, $auth, $user; global $phpbb_root_path, $phpEx, $config; + global $request; // Damn php and globals - i know, this is horrible // Needed for handle_message_list_actions() @@ -49,13 +50,7 @@ function compose_pm($id, $mode, $action) // Reply to all triggered (quote/reply) $reply_to_all = request_var('reply_to_all', 0); - // Do NOT use request_var or specialchars here - $address_list = isset($_REQUEST['address_list']) ? $_REQUEST['address_list'] : array(); - - if (!is_array($address_list)) - { - $address_list = array(); - } + $address_list = $request->variable('address_list', array('' => array(0 => ''))); $submit = (isset($_POST['post'])) ? true : false; $preview = (isset($_POST['preview'])) ? true : false; @@ -1029,7 +1024,7 @@ function compose_pm($id, $mode, $action) $s_hidden_fields = ''; $s_hidden_fields .= (isset($check_value)) ? '' : ''; - $s_hidden_fields .= ($draft_id || isset($_REQUEST['draft_loaded'])) ? '' : ''; + $s_hidden_fields .= ($draft_id || isset($_REQUEST['draft_loaded'])) ? '' : ''; $form_enctype = (@ini_get('file_uploads') == '0' || strtolower(@ini_get('file_uploads')) == 'off' || !$config['allow_pm_attach'] || !$auth->acl_get('u_pm_attach')) ? '' : ' enctype="multipart/form-data"'; @@ -1105,11 +1100,12 @@ function compose_pm($id, $mode, $action) function handle_message_list_actions(&$address_list, &$error, $remove_u, $remove_g, $add_to, $add_bcc) { global $auth, $db, $user; + global $request; // Delete User [TO/BCC] - if ($remove_u && !empty($_REQUEST['remove_u']) && is_array($_REQUEST['remove_u'])) + if ($remove_u && $request->variable('remove_u', array(0 => ''))) { - $remove_user_id = array_keys($_REQUEST['remove_u']); + $remove_user_id = array_keys($request->variable('remove_u', array(0 => ''))); if (isset($remove_user_id[0])) { @@ -1118,9 +1114,9 @@ function handle_message_list_actions(&$address_list, &$error, $remove_u, $remove } // Delete Group [TO/BCC] - if ($remove_g && !empty($_REQUEST['remove_g']) && is_array($_REQUEST['remove_g'])) + if ($remove_g && $request->variable('remove_g', array(0 => ''))) { - $remove_group_id = array_keys($_REQUEST['remove_g']); + $remove_group_id = array_keys($request->variable('remove_g', array(0 => ''))); if (isset($remove_group_id[0])) { @@ -1188,7 +1184,7 @@ function handle_message_list_actions(&$address_list, &$error, $remove_u, $remove } // Add Friends if specified - $friend_list = (isset($_REQUEST['add_' . $type]) && is_array($_REQUEST['add_' . $type])) ? array_map('intval', array_keys($_REQUEST['add_' . $type])) : array(); + $friend_list = array_keys($request->variable('add_' . $type, array(0))); $user_id_ary = array_merge($user_id_ary, $friend_list); foreach ($user_id_ary as $user_id) diff --git a/phpBB/includes/ucp/ucp_pm_viewmessage.php b/phpBB/includes/ucp/ucp_pm_viewmessage.php index 16700c490c..429c8688cf 100644 --- a/phpBB/includes/ucp/ucp_pm_viewmessage.php +++ b/phpBB/includes/ucp/ucp_pm_viewmessage.php @@ -245,7 +245,7 @@ function view_message($id, $mode, $folder_id, $msg_id, $folder, $message_row) } } - if (!isset($_REQUEST['view']) || $_REQUEST['view'] != 'print') + if (!isset($_REQUEST['view']) || $request->variable('view', '') != 'print') { // Message History if (message_history($msg_id, $user->data['user_id'], $message_row, $folder)) diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index c099e3b3fa..afd85c9975 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -30,12 +30,13 @@ class ucp_profile function main($id, $mode) { global $config, $db, $user, $auth, $template, $phpbb_root_path, $phpEx; + global $request; $user->add_lang('posting'); - $preview = (!empty($_POST['preview'])) ? true : false; - $submit = (!empty($_POST['submit'])) ? true : false; - $delete = (!empty($_POST['delete'])) ? true : false; + $preview = $request->variable('preview', false, false, phpbb_request_interface::POST); + $submit = $request->variable('submit', false, false, phpbb_request_interface::POST); + $delete = $request->variable('delete', false, false, phpbb_request_interface::POST); $error = $data = array(); $s_hidden_fields = ''; diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 7fd99da55a..ce682da56b 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -28,6 +28,7 @@ class ucp_register function main($id, $mode) { global $config, $db, $user, $auth, $template, $phpbb_root_path, $phpEx; + global $request; // if ($config['require_activation'] == USER_ACTIVATION_DISABLE) @@ -37,9 +38,9 @@ class ucp_register include($phpbb_root_path . 'includes/functions_profile_fields.' . $phpEx); - $coppa = (isset($_REQUEST['coppa'])) ? ((!empty($_REQUEST['coppa'])) ? 1 : 0) : false; - $agreed = (!empty($_POST['agreed'])) ? 1 : 0; - $submit = (isset($_POST['submit'])) ? true : false; + $coppa = $request->is_set('coppa') ? (int) $request->variable('coppa', false) : false; + $agreed = (int) $request->variable('agreed', false); + $submit = $request->is_set_post('submit'); $change_lang = request_var('change_lang', ''); $user_lang = request_var('lang', $user->lang_name); @@ -63,7 +64,7 @@ class ucp_register $submit = false; // Setting back agreed to let the user view the agreement in his/her language - $agreed = (empty($_GET['change_lang'])) ? 0 : $agreed; + $agreed = ($request->variable('change_lang', false)) ? 0 : $agreed; } $user->lang_name = $user_lang = $use_lang; @@ -502,4 +503,4 @@ class ucp_register } } -?> \ No newline at end of file +?> diff --git a/phpBB/install/install_convert.php b/phpBB/install/install_convert.php index 814b50cf68..03cae5f124 100644 --- a/phpBB/install/install_convert.php +++ b/phpBB/install/install_convert.php @@ -586,6 +586,7 @@ class install_convert extends module { global $template, $user, $phpbb_root_path, $phpEx, $db, $lang, $config, $cache; global $convert, $convert_row, $message_parser, $skip_rows, $language; + global $request; require($phpbb_root_path . 'config.' . $phpEx); require($phpbb_root_path . 'includes/constants.' . $phpEx); @@ -812,7 +813,7 @@ class install_convert extends module if (!$current_table && !$skip_rows) { - if (empty($_REQUEST['confirm'])) + if (!$request->variable('confirm', false)) { // If avatars / ranks / smilies folders are specified make sure they are writable $bad_folders = array(); @@ -973,7 +974,7 @@ class install_convert extends module )); return; - } // if (empty($_REQUEST['confirm'])) + } // if (!$request->variable('confirm', false))) $template->assign_block_vars('checks', array( 'S_LEGEND' => true, diff --git a/phpBB/install/install_update.php b/phpBB/install/install_update.php index 6184cbbc33..1399a99a31 100644 --- a/phpBB/install/install_update.php +++ b/phpBB/install/install_update.php @@ -73,6 +73,7 @@ class install_update extends module function main($mode, $sub) { global $template, $phpEx, $phpbb_root_path, $user, $db, $config, $cache, $auth, $language; + global $request; $this->tpl_name = 'install_update'; $this->page_title = 'UPDATE_INSTALLATION'; @@ -251,7 +252,7 @@ class install_update extends module $this->include_file('includes/diff/renderer.' . $phpEx); // Make sure we stay at the file check if checking the files again - if (!empty($_POST['check_again'])) + if ($request->variable('check_again', false, false, phpbb_request_interface::POST)) { $sub = $this->p_master->sub = 'file_check'; } @@ -358,7 +359,7 @@ class install_update extends module $action = request_var('action', ''); // We are directly within an update. To make sure our update list is correct we check its status. - $update_list = (!empty($_POST['check_again'])) ? false : $cache->get('_update_list'); + $update_list = ($request->variable('check_again', false, false, phpbb_request_interface::POST)) ? false : $cache->get('_update_list'); $modified = ($update_list !== false) ? @filemtime($cache->cache_dir . 'data_update_list.' . $phpEx) : 0; // Make sure the list is up-to-date @@ -714,7 +715,7 @@ class install_update extends module { $cache->put('_diff_files', $file_list); - if (!empty($_REQUEST['download'])) + if ($request->variable('download', false)) { $params[] = 'download=1'; } @@ -829,7 +830,7 @@ class install_update extends module $file_list['status'] = -1; $cache->put('_diff_files', $file_list); - if (!empty($_REQUEST['download'])) + if ($request->variable('download', false)) { $this->include_file('includes/functions_compress.' . $phpEx); @@ -963,7 +964,7 @@ class install_update extends module 'DATA' => $data, 'NAME' => $user->lang[strtoupper($method . '_' . $data)], 'EXPLAIN' => $user->lang[strtoupper($method . '_' . $data) . '_EXPLAIN'], - 'DEFAULT' => (!empty($_REQUEST[$data])) ? request_var($data, '') : $default + 'DEFAULT' => $request->variable($data, (string) $default), )); } diff --git a/phpBB/mcp.php b/phpBB/mcp.php index 48cd68500f..360f60fc74 100644 --- a/phpBB/mcp.php +++ b/phpBB/mcp.php @@ -31,15 +31,8 @@ $template->assign_var('S_IN_MCP', true); // Basic parameter data $id = request_var('i', ''); -if (isset($_REQUEST['mode']) && is_array($_REQUEST['mode'])) -{ - $mode = request_var('mode', array('')); - list($mode, ) = each($mode); -} -else -{ - $mode = request_var('mode', ''); -} +$mode = request_var('mode', array('')); +$mode = sizeof($mode) ? array_shift($mode) : ''; // Only Moderators can go beyond this point if (!$user->data['is_registered']) @@ -57,7 +50,7 @@ $action = request_var('action', ''); $action_ary = request_var('action', array('' => 0)); $forum_action = request_var('forum_action', ''); -if ($forum_action !== '' && !empty($_POST['sort'])) +if ($forum_action !== '' && $request->variable('sort', false, false, phpbb_request_interface::POST)) { $action = $forum_action; } @@ -174,7 +167,7 @@ if ($quickmod) // Reset start parameter if we jumped from the quickmod dropdown if (request_var('start', 0)) { - $_REQUEST['start'] = 0; + $request->overwrite('start', 0); } $module->set_active('logs', 'topic_logs'); diff --git a/phpBB/memberlist.php b/phpBB/memberlist.php index 2fa2d11ee1..4ff798cd51 100644 --- a/phpBB/memberlist.php +++ b/phpBB/memberlist.php @@ -1015,7 +1015,7 @@ switch ($mode) // We validate form and field here, only id/class allowed $form = (!preg_match('/^[a-z0-9_-]+$/i', $form)) ? '' : $form; $field = (!preg_match('/^[a-z0-9_-]+$/i', $field)) ? '' : $field; - if (($mode == 'searchuser' || sizeof(array_intersect(array_keys($_GET), $search_params)) > 0) && ($config['load_search'] || $auth->acl_get('a_'))) + if (($mode == 'searchuser' || sizeof(array_intersect($request->variable_names(phpbb_request_interface::GET), $search_params)) > 0) && ($config['load_search'] || $auth->acl_get('a_'))) { $username = request_var('username', '', true); $email = strtolower(request_var('email', '')); diff --git a/phpBB/posting.php b/phpBB/posting.php index 853ac18aad..86e953680a 100644 --- a/phpBB/posting.php +++ b/phpBB/posting.php @@ -663,7 +663,7 @@ if ($submit || $preview || $refresh) $message_parser->message = utf8_normalize_nfc(request_var('message', '', true)); $post_data['username'] = utf8_normalize_nfc(request_var('username', $post_data['username'], true)); - $post_data['post_edit_reason'] = (!empty($_POST['edit_reason']) && $mode == 'edit' && $auth->acl_get('m_edit', $forum_id)) ? utf8_normalize_nfc(request_var('edit_reason', '', true)) : ''; + $post_data['post_edit_reason'] = ($request->variable('edit_reason', false, false, phpbb_request_interface::POST) && $mode == 'edit' && $auth->acl_get('m_edit', $forum_id)) ? utf8_normalize_nfc(request_var('edit_reason', '', true)) : ''; $post_data['orig_topic_type'] = $post_data['topic_type']; $post_data['topic_type'] = request_var('topic_type', (($mode != 'post') ? (int) $post_data['topic_type'] : POST_NORMAL)); diff --git a/phpBB/ucp.php b/phpBB/ucp.php index 2d190cb1a9..bd8c47a7df 100644 --- a/phpBB/ucp.php +++ b/phpBB/ucp.php @@ -82,7 +82,7 @@ switch ($mode) break; case 'logout': - if ($user->data['user_id'] != ANONYMOUS && isset($_GET['sid']) && !is_array($_GET['sid']) && $_GET['sid'] === $user->session_id) + if ($user->data['user_id'] != ANONYMOUS && $request->is_set('sid') && $request->variable('sid', '') === $user->session_id) { $user->session_kill(); $user->session_begin(); From c09bdb6c5535e7a7164acbe04e152ce415bfce9e Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Fri, 24 Sep 2010 18:53:33 +0200 Subject: [PATCH 16/21] [feature/request-class] Remove tricky $_* is_array from acp_profile PHPBB3-9716 --- phpBB/includes/acp/acp_profile.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_profile.php b/phpBB/includes/acp/acp_profile.php index 776a59c8c7..b66737ce03 100644 --- a/phpBB/includes/acp/acp_profile.php +++ b/phpBB/includes/acp/acp_profile.php @@ -691,7 +691,7 @@ class acp_profile } else { - $_new_key_ary[$key] = ($request->is_array($key)) ? utf8_normalize_nfc(request_var($key, array(''), true)) : utf8_normalize_nfc(request_var($key, '', true)); + $_new_key_ary[$key] = ($field_type == FIELD_BOOL && $key == 'lang_options') ? utf8_normalize_nfc(request_var($key, array(''), true)) : utf8_normalize_nfc(request_var($key, '', true)); } } } From 986935c4745195f45cbff74e541ffc98ecac714e Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Mon, 27 Sep 2010 22:50:25 +0200 Subject: [PATCH 17/21] [feature/request-class] Adjust some trailing newlines PHPBB3-9716 --- phpBB/common.php | 2 +- phpBB/includes/ucp/ucp_register.php | 2 +- phpBB/install/index.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/common.php b/phpBB/common.php index 41df3049a1..2eba85383d 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -243,4 +243,4 @@ foreach ($cache->obtain_hooks() as $hook) @include($phpbb_root_path . 'includes/hooks/' . $hook . '.' . $phpEx); } -?> +?> \ No newline at end of file diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index ce682da56b..5fab865093 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -503,4 +503,4 @@ class ucp_register } } -?> +?> \ No newline at end of file diff --git a/phpBB/install/index.php b/phpBB/install/index.php index 50f0a049f2..5894a228c7 100644 --- a/phpBB/install/index.php +++ b/phpBB/install/index.php @@ -818,4 +818,4 @@ class module } } -?> +?> \ No newline at end of file From 684914e6350fff9d3b89953b313b82db247cd1f8 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Thu, 30 Sep 2010 22:17:08 +0200 Subject: [PATCH 18/21] [feature/request-class] Make additional request test cases run PHPBB3-9716 --- tests/request/request_var.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/request/request_var.php b/tests/request/request_var.php index 857dbe55a5..1c360d45d0 100644 --- a/tests/request/request_var.php +++ b/tests/request/request_var.php @@ -81,11 +81,14 @@ class phpbb_request_var_test extends phpbb_test_case /** * @dataProvider deep_access * Only possible with 3.1.x (later) + */ public function test_deep_multi_dim_array_access($path, $default, $expected) { $this->unset_variables('var'); - $_REQUEST['var'] = array( + // cannot set $_REQUEST directly because in phpbb_request implementation + // $_REQUEST = $_GET + $_POST + $_POST['var'] = array( 0 => array( 'b' => array( true => array( @@ -115,7 +118,6 @@ class phpbb_request_var_test extends phpbb_test_case array(array('var', 0, 'b', true), array(0 => ''), array(5 => 'c', 6 => 'd')), ); } -*/ public static function request_variables() { @@ -217,7 +219,6 @@ class phpbb_request_var_test extends phpbb_test_case 'abc' => array() ) ), - // 3-dimensional (not supported atm! array( // input: array( From 30b57332e3b0a07294bacba8df26b309818662f9 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Tue, 5 Oct 2010 22:53:06 +0200 Subject: [PATCH 19/21] [feature/request-class] Fix missing include in database_update install/database_update.php was missing the include for the class loader. PHPBB3-9716 --- phpBB/install/database_update.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 69f5f58563..211e194c73 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -60,6 +60,7 @@ if (!empty($load_extensions) && function_exists('dl')) } // Include files +require($phpbb_root_path . 'includes/class_loader.' . $phpEx); require($phpbb_root_path . 'includes/acm/acm_' . $acm_type . '.' . $phpEx); require($phpbb_root_path . 'includes/cache.' . $phpEx); require($phpbb_root_path . 'includes/template.' . $phpEx); From c62aa522347c6d54d5a3bdeed635de97bf26581e Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Tue, 5 Oct 2010 23:20:39 +0200 Subject: [PATCH 20/21] [feature/request-class] Fix remember and session hide on login Thanks to SA007. PHPBB3-9716 --- phpBB/includes/functions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 25cc1f9960..912d39e0f0 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2844,8 +2844,8 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa } $username = request_var('username', '', true); - $autologin = $request->variable('autologin', false, false, phpbb_request_interface::POST); - $viewonline = (int) $request->variable('viewonline', false, false, phpbb_request_interface::POST); + $autologin = $request->is_set_post('autologin'); + $viewonline = (int) !$request->is_set_post('viewonline'); $admin = ($admin) ? 1 : 0; $viewonline = ($admin) ? $user->data['session_viewonline'] : $viewonline; From c2ffa78521a656b1a183d75c8de2f88624011967 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Tue, 5 Oct 2010 23:29:57 +0200 Subject: [PATCH 21/21] [feature/request-class] Fix mcp.php mode parameter Thanks to SA007. PHPBB3-9716 --- phpBB/mcp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/mcp.php b/phpBB/mcp.php index 360f60fc74..1d294bb581 100644 --- a/phpBB/mcp.php +++ b/phpBB/mcp.php @@ -32,7 +32,7 @@ $template->assign_var('S_IN_MCP', true); $id = request_var('i', ''); $mode = request_var('mode', array('')); -$mode = sizeof($mode) ? array_shift($mode) : ''; +$mode = sizeof($mode) ? array_shift($mode) : request_var('mode', ''); // Only Moderators can go beyond this point if (!$user->data['is_registered'])