[ticket/11832] Create phpbb_symfony_request to handle initiating symfony_request

Now symfony_request is also a service (removed the function
phpbb_create_symfony_request).

Inject symfony request into filesystem

Cleanup for the tests

PHPBB3-11832

phpBB/common.php

@@ -101,7 +101,6 @@ $cache = $phpbb_container->get('cache');
 
 // Instantiate some basic classes
 $phpbb_dispatcher = $phpbb_container->get('dispatcher');
-$phpbb_filesystem = $phpbb_container->get('filesystem');
 $request	= $phpbb_container->get('request');
 $user		= $phpbb_container->get('user');
 $auth		= $phpbb_container->get('auth');
@@ -111,7 +110,8 @@ $db			= $phpbb_container->get('dbal.conn');
 request_var('', 0, false, false, $request); // "dependency injection" for a function
 
 // Create a Symfony Request object from our phpbb_request object
-$symfony_request = phpbb_create_symfony_request($request);
+$symfony_request = $phpbb_container->get('symfony_request');
+$phpbb_filesystem = $phpbb_container->get('filesystem');
 
 // Grab global variables, re-cache if necessary
 $config = $phpbb_container->get('config');

phpBB/config/services.yml

@@ -170,6 +170,7 @@ services:
     filesystem:
         class: phpbb_filesystem
         arguments:
+            - @symfony_request
             - %core.root_path%
 
     groupposition.legend:
@@ -254,6 +255,11 @@ services:
     request:
         class: phpbb_request
 
+    symfony_request:
+        class: phpbb_symfony_request
+        arguments:
+            - @request
+
     template:
         class: phpbb_template_twig
         arguments:

phpBB/includes/functions.php

@@ -5708,44 +5708,3 @@ function phpbb_convert_30_dbms_to_31($dbms)
 
 	throw new \RuntimeException("You have specified an invalid dbms driver: $dbms");
 }
-
-/**
-* Create a Symfony Request object from phpbb_request object
-*
-* @param phpbb_request $request Request object
-* @return Request A Symfony Request object
-*/
-function phpbb_create_symfony_request(phpbb_request $request)
-{
-	// If we have already gotten it, don't go back through all the trouble of
-	// creating it again; instead, just return it. This allows multiple calls
-	// of this method so we don't have to globalize $symfony_request in other
-	// functions.
-	static $symfony_request;
-	if (null !== $symfony_request)
-	{
-		return $symfony_request;
-	}
-
-	// This function is meant to sanitize the global input arrays
-	$sanitizer = function(&$value, $key) {
-		$type_cast_helper = new phpbb_request_type_cast_helper();
-		$type_cast_helper->set_var($value, $value, gettype($value), true);
-	};
-
-	// We need to re-enable the super globals so we can access them here
-	$request->enable_super_globals();
-	$get_parameters = $_GET;
-	$post_parameters = $_POST;
-	$server_parameters = $_SERVER;
-	$files_parameters = $_FILES;
-	$cookie_parameters = $_COOKIE;
-	// And now disable them again for security
-	$request->disable_super_globals();
-
-	array_walk_recursive($get_parameters, $sanitizer);
-	array_walk_recursive($post_parameters, $sanitizer);
-
-	$symfony_request = new Symfony\Component\HttpFoundation\Request($get_parameters, $post_parameters, array(), $cookie_parameters, $files_parameters, $server_parameters);
-	return $symfony_request;
-}

phpBB/phpbb/filesystem.php

@@ -7,8 +7,6 @@
 *
 */
 
-use Symfony\Component\HttpFoundation\Request;
-
 /**
 * @ignore
 */
@@ -23,6 +21,9 @@ if (!defined('IN_PHPBB'))
 */
 class phpbb_filesystem
 {
+	/** @var phpbb_symfony_request */
+	protected $symfony_request;
+
 	/** @var string */
 	protected $phpbb_root_path;
 
@@ -32,10 +33,12 @@ class phpbb_filesystem
 	/**
 	* Constructor
 	*
+	* @param phpbb_symfony_request $symfony_request
 	* @param string $phpbb_root_path
 	*/
-	public function __construct($phpbb_root_path)
+	public function __construct(phpbb_symfony_request $symfony_request, $phpbb_root_path)
 	{
+		$this->symfony_request = $symfony_request;
 		$this->phpbb_root_path = $phpbb_root_path;
 	}
 
@@ -57,12 +60,12 @@ class phpbb_filesystem
 	*	is not at the beginning of $path, just prepends the
 	*	web root path
 	*
-	* @param Request $symfony_request Symfony Request object
+	* @param string $path The path to be updated
 	* @return string
 	*/
-	public function update_web_root_path($path, Request $symfony_request = null)
+	public function update_web_root_path($path)
 	{
-		$web_root_path = $this->get_web_root_path($symfony_request);
+		$web_root_path = $this->get_web_root_path($this->symfony_request);
 
 		if (strpos($path, $this->phpbb_root_path) === 0)
 		{
@@ -75,12 +78,11 @@ class phpbb_filesystem
 	/**
 	* Get a relative root path from the current URL
 	*
-	* @param Request $symfony_request Symfony Request object
 	* @return string
 	*/
-	public function get_web_root_path(Request $symfony_request = null)
+	public function get_web_root_path()
 	{
-		if ($symfony_request === null)
+		if ($this->symfony_request === null)
 		{
 			return $this->phpbb_root_path;
 		}
@@ -91,13 +93,13 @@ class phpbb_filesystem
 		}
 
 		// Path info (e.g. /foo/bar)
-		$path_info = $this->clean_path($symfony_request->getPathInfo());
+		$path_info = $this->clean_path($this->symfony_request->getPathInfo());
 
 		// Full request URI (e.g. phpBB/app.php/foo/bar)
-		$request_uri = $symfony_request->getRequestUri();
+		$request_uri = $this->symfony_request->getRequestUri();
 
 		// Script name URI (e.g. phpBB/app.php)
-		$script_name = $symfony_request->getScriptName();
+		$script_name = $this->symfony_request->getScriptName();
 
 		/*
 		* If the path info is empty (single /), then we're not using

phpBB/phpbb/symfony_request.php

@@ -0,0 +1,46 @@
+<?php
+/**
+*
+* @package phpBB3
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+	exit;
+}
+
+class phpbb_symfony_request extends Request
+{
+	/**
+	* Constructor
+	*
+	* @param phpbb_request_interface $phpbb_request
+	*/
+	public function __construct(phpbb_request_interface $phpbb_request)
+	{
+		// This function is meant to sanitize the global input arrays
+		$sanitizer = function(&$value, $key) {
+			$type_cast_helper = new phpbb_request_type_cast_helper();
+			$type_cast_helper->set_var($value, $value, gettype($value), true);
+		};
+
+		$get_parameters = $phpbb_request->get_super_global(phpbb_request_interface::GET);
+		$post_parameters = $phpbb_request->get_super_global(phpbb_request_interface::POST);
+		$server_parameters = $phpbb_request->get_super_global(phpbb_request_interface::SERVER);
+		$files_parameters = $phpbb_request->get_super_global(phpbb_request_interface::FILES);
+		$cookie_parameters = $phpbb_request->get_super_global(phpbb_request_interface::COOKIE);
+
+		array_walk_recursive($get_parameters, $sanitizer);
+		array_walk_recursive($post_parameters, $sanitizer);
+
+		parent::__construct($get_parameters, $post_parameters, array(), $cookie_parameters, $files_parameters, $server_parameters);
+	}
+}

tests/filesystem/clean_path_test.php

@@ -14,7 +14,12 @@ class phpbb_filesystem_clean_path_test extends phpbb_test_case
 	public function setUp()
 	{
 		parent::setUp();
-		$this->filesystem = new phpbb_filesystem(__DIR__ . './../../phpBB/');
+		$this->filesystem = new phpbb_filesystem(
+			new phpbb_symfony_request(
+				new phpbb_mock_request()
+			),
+			dirname(__FILE__) . './../../phpBB/'
+		);
 	}
 
 	public function clean_path_data()

tests/filesystem/web_root_path_test.php

@@ -18,7 +18,8 @@ class phpbb_filesystem_web_root_path_test extends phpbb_test_case
 
 		$this->set_phpbb_root_path();
 
-		$this->filesystem = new phpbb_filesystem($this->phpbb_root_path);
+		$symfony_request = new phpbb_symfony_request(new phpbb_mock_request());
+		$this->filesystem = new phpbb_filesystem($symfony_request, $this->phpbb_root_path);
 	}
 
 	/**
@@ -40,13 +41,14 @@ class phpbb_filesystem_web_root_path_test extends phpbb_test_case
 		$this->assertEquals($this->phpbb_root_path, $this->filesystem->get_web_root_path());
 	}
 
-	public function update_web_root_path_data()
+	public function basic_update_web_root_path_data()
 	{
 		$this->set_phpbb_root_path();
 
 		return array(
 			array(
 				$this->phpbb_root_path . 'test.php',
+				$this->phpbb_root_path . 'test.php',
 			),
 			array(
 				'test.php',
@@ -54,7 +56,24 @@ class phpbb_filesystem_web_root_path_test extends phpbb_test_case
 			),
 			array(
 				$this->phpbb_root_path . $this->phpbb_root_path . 'test.php',
+				$this->phpbb_root_path . $this->phpbb_root_path . 'test.php',
 			),
+		);
+	}
+
+	/**
+	* @dataProvider basic_update_web_root_path_data
+	*/
+	public function test_basic_update_web_root_path($input, $expected)
+	{
+		$this->assertEquals($expected, $this->filesystem->update_web_root_path($input, $symfony_request));
+	}
+
+	public function update_web_root_path_data()
+	{
+		$this->set_phpbb_root_path();
+
+		return array(
 			array(
 				$this->phpbb_root_path . 'test.php',
 				$this->phpbb_root_path . 'test.php',
@@ -92,25 +111,23 @@ class phpbb_filesystem_web_root_path_test extends phpbb_test_case
 	/**
 	* @dataProvider update_web_root_path_data
 	*/
-	public function test_update_web_root_path($input, $expected = null, $getPathInfo = null, $getRequestUri = null, $getScriptName = null)
+	public function test_update_web_root_path($input, $expected, $getPathInfo, $getRequestUri = null, $getScriptName = null)
 	{
-		$expected = ($expected === null) ? $input : $expected;
+		$symfony_request = $this->getMock("phpbb_symfony_request", array(), array(
+			new phpbb_mock_request(),
+		));
+		$symfony_request->expects($this->any())
+			->method('getPathInfo')
+			->will($this->returnValue($getPathInfo));
+		$symfony_request->expects($this->any())
+			->method('getRequestUri')
+			->will($this->returnValue($getRequestUri));
+		$symfony_request->expects($this->any())
+			->method('getScriptName')
+			->will($this->returnValue($getScriptName));
 
-		$symfony_request = null;
+		$filesystem = new phpbb_filesystem($symfony_request, $this->phpbb_root_path);
-		if ($getPathInfo !== null)
-		{
-			$symfony_request = $this->getMock("Symfony\Component\HttpFoundation\Request");
-			$symfony_request->expects($this->any())
-				->method('getPathInfo')
-				->will($this->returnValue($getPathInfo));
-			$symfony_request->expects($this->any())
-				->method('getRequestUri')
-				->will($this->returnValue($getRequestUri));
-			$symfony_request->expects($this->any())
-				->method('getScriptName')
-				->will($this->returnValue($getScriptName));
-		}
 
-		$this->assertEquals($expected, $this->filesystem->update_web_root_path($input, $symfony_request));
+		$this->assertEquals($expected, $filesystem->update_web_root_path($input, $symfony_request));
 	}
 }