Merge pull request #8178 from owncloud/fix-request-parameters

Correctly process request parameters other than GET or POST, dont use gl...

lib/private/appframework/http/request.php

@@ -60,7 +60,14 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	 * @param string|false 'requesttoken' the requesttoken or false when not available
 	 * @see http://www.php.net/manual/en/reserved.variables.php
 	 */
-	public function __construct(array $vars=array()) {
+	public function __construct(array $vars=array(), $stream='php://input') {
+
+		$this->inputStream = $stream;
+		$this->items['params'] = array();
+
+		if(!array_key_exists('method', $vars)) {
+			$vars['method'] = 'GET';			
+		}
 
 		foreach($this->allowedKeys as $name) {
 			$this->items[$name] = isset($vars[$name])
@@ -68,25 +75,32 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 				: array();
 		}
 
-		if (defined('PHPUNIT_RUN') && PHPUNIT_RUN
+		// 'application/json' must be decoded manually.
-			&& in_array('fakeinput', stream_get_wrappers())) {
+		if (strpos($this->getHeader('Content-Type'), 'application/json') !== false) {
-			$this->inputStream = 'fakeinput://data';
+			$params = json_decode(file_get_contents($this->inputStream), true);
-		} else {
+			if(count($params) > 0) {
-			$this->inputStream = 'php://input';
+				$this->items['params'] = $params;
-		}
+				if($vars['method'] === 'POST') {
-
+					$this->items['post'] = $params;					
-		// Only 'application/x-www-form-urlencoded' requests are automatically
+				}
-		// transformed by PHP, 'application/json' must be decoded manually.
+			}
-		if ($this->method === 'POST'
+		// Handle application/x-www-form-urlencoded for methods other than GET
-			&& strpos($this->getHeader('Content-Type'), 'application/json') !== false
+		// or post correctly
-		) {
+		} elseif($vars['method'] !== 'GET'
-			$this->items['params'] = $this->items['post'] = json_decode(file_get_contents($this->inputStream), true);
+				&& $vars['method'] !== 'POST'
-		}
+				&& strpos($this->getHeader('Content-Type'), 'application/x-www-form-urlencoded') !== false) {
+			
+			parse_str(file_get_contents($this->inputStream), $params);
+			if(is_array($params)) {
+				$this->items['params'] = $params;
+			}
+		} 
 
 		$this->items['parameters'] = array_merge(
 			$this->items['get'],
 			$this->items['post'],
-			$this->items['urlParams']
+			$this->items['urlParams'],
+			$this->items['params']
 		);
 
 	}
@@ -313,47 +327,22 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	 * @throws \LogicException
 	 */
 	protected function getContent() {
-		if ($this->content === false && $this->method === 'PUT') {
-			throw new \LogicException(
-				'"put" can only be accessed once if not '
-				. 'application/x-www-form-urlencoded or application/json.'
-			);
-		}
-
 		// If the content can't be parsed into an array then return a stream resource.
 		if ($this->method === 'PUT'
 			&& strpos($this->getHeader('Content-Type'), 'application/x-www-form-urlencoded') === false
 			&& strpos($this->getHeader('Content-Type'), 'application/json') === false
 		) {
+			if ($this->content === false) {
+				throw new \LogicException(
+					'"put" can only be accessed once if not '
+					. 'application/x-www-form-urlencoded or application/json.'
+				);
+			}
 			$this->content = false;
 			return fopen($this->inputStream, 'rb');
+		} else {
+			return $this->parameters;
 		}
-
-		if (is_null($this->content)) {
-			$this->content = file_get_contents($this->inputStream);
-
-			/*
-			* Normal jquery ajax requests are sent as application/x-www-form-urlencoded
-			* and in $_GET and $_POST PHP transformes the data into an array.
-			* The first condition mimics this.
-			* The second condition allows for sending raw application/json data while
-			* still getting the result as an array.
-			*
-			*/
-			if (strpos($this->getHeader('Content-Type'), 'application/x-www-form-urlencoded') !== false) {
-				parse_str($this->content, $content);
-				if(is_array($content)) {
-					$this->content = $content;
-				}
-			} elseif (strpos($this->getHeader('Content-Type'), 'application/json') !== false) {
-				$content = json_decode($this->content, true);
-				if(is_array($content)) {
-					$this->content = $content;
-				}
-			}
-		}
-
-		return $this->content;
 	}
 
 	/**

lib/private/server.php

@@ -35,6 +35,13 @@ class Server extends SimpleContainer implements IServerContainer {
 				$requesttoken = false;
 			}
 
+			if (defined('PHPUNIT_RUN') && PHPUNIT_RUN
+			&& in_array('fakeinput', stream_get_wrappers())) {
+				$stream = 'fakeinput://data';
+			} else {
+				$stream = 'php://input';
+			}
+
 			return new Request(
 				array(
 					'get' => $_GET,
@@ -48,7 +55,7 @@ class Server extends SimpleContainer implements IServerContainer {
 						: null,
 					'urlParams' => $urlParams,
 					'requesttoken' => $requesttoken,
-				)
+				), $stream
 			);
 		});
 		$this->registerService('PreviewManager', function($c) {

tests/lib/appframework/dependencyinjection/DIContainerTest.php

@@ -70,7 +70,7 @@ class DIContainerTest extends \PHPUnit_Framework_TestCase {
 
 
 	public function testMiddlewareDispatcherIncludesSecurityMiddleware(){
-		$this->container['Request'] = new Request();
+		$this->container['Request'] = new Request(array('method' => 'GET'));
 		$security = $this->container['SecurityMiddleware'];
 		$dispatcher = $this->container['MiddlewareDispatcher'];
 

tests/lib/appframework/http/RequestTest.php

@@ -18,6 +18,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			stream_wrapper_unregister('fakeinput');
 		}
 		stream_wrapper_register('fakeinput', 'RequestStream');
+		$this->stream = 'fakeinput://data';
 	}
 
 	public function tearDown() {
@@ -30,7 +31,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'method' => 'GET',
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 
 		// Countable
 		$this->assertEquals(2, count($request));
@@ -54,9 +55,10 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'get' => array('name' => 'John Q. Public', 'nickname' => 'Joey'),
 			'post' => array('name' => 'Jane Doe', 'nickname' => 'Janey'),
 			'urlParams' => array('user' => 'jw', 'name' => 'Johnny Weissmüller'),
+			'method' => 'GET'
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 
 		$this->assertEquals(3, count($request));
 		$this->assertEquals('Janey', $request->{'nickname'});
@@ -70,9 +72,10 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 	public function testImmutableArrayAccess() {
 		$vars = array(
 			'get' => array('name' => 'John Q. Public', 'nickname' => 'Joey'),
+			'method' => 'GET'
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 		$request['nickname'] = 'Janey';
 	}
 
@@ -82,9 +85,10 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 	public function testImmutableMagicAccess() {
 		$vars = array(
 			'get' => array('name' => 'John Q. Public', 'nickname' => 'Joey'),
+			'method' => 'GET'
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 		$request->{'nickname'} = 'Janey';
 	}
 
@@ -97,7 +101,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'method' => 'GET',
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 		$result = $request->post;
 	}
 
@@ -107,7 +111,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'method' => 'GET',
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 		$this->assertEquals('GET', $request->method);
 		$result = $request->get;
 		$this->assertEquals('John Q. Public', $result['name']);
@@ -119,10 +123,10 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 		$data = '{"name": "John Q. Public", "nickname": "Joey"}';
 		$vars = array(
 			'method' => 'POST',
-			'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
+			'server' => array('CONTENT_TYPE' => 'application/json; utf-8')
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 		$this->assertEquals('POST', $request->method);
 		$result = $request->post;
 		$this->assertEquals('John Q. Public', $result['name']);
@@ -140,7 +144,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/x-www-form-urlencoded'),
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 
 		$this->assertEquals('PATCH', $request->method);
 		$result = $request->patch;
@@ -159,7 +163,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 
 		$this->assertEquals('PUT', $request->method);
 		$result = $request->put;
@@ -174,7 +178,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 
 		$this->assertEquals('PATCH', $request->method);
 		$result = $request->patch;
@@ -193,7 +197,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase {
 			'server' => array('CONTENT_TYPE' => 'image/png'),
 		);
 
-		$request = new Request($vars);
+		$request = new Request($vars, $this->stream);
 		$this->assertEquals('PUT', $request->method);
 		$resource = $request->put;
 		$contents = stream_get_contents($resource);

tests/lib/appframework/middleware/MiddlewareDispatcherTest.php

@@ -132,7 +132,7 @@ class MiddlewareDispatcherTest extends \PHPUnit_Framework_TestCase {
 
 	private function getControllerMock(){
 		return $this->getMock('OCP\AppFramework\Controller', array('method'),
-			array($this->getAPIMock(), new Request()));
+			array($this->getAPIMock(), new Request(array('method' => 'GET'))));
 	}