From e7fa2790f39a5c120d4f935139c1210258a1472a Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Sat, 12 Apr 2014 15:02:19 +0200 Subject: [PATCH 1/2] Correctly process request parameters other than GET or POST, dont use globals in the class but inject it --- lib/private/appframework/http/request.php | 80 +++++++++-------------- lib/private/server.php | 9 ++- 2 files changed, 39 insertions(+), 50 deletions(-) diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php index 40f47a7bd2..43631697d3 100644 --- a/lib/private/appframework/http/request.php +++ b/lib/private/appframework/http/request.php @@ -60,7 +60,10 @@ 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(); foreach($this->allowedKeys as $name) { $this->items[$name] = isset($vars[$name]) @@ -68,25 +71,29 @@ class Request implements \ArrayAccess, \Countable, IRequest { : array(); } - if (defined('PHPUNIT_RUN') && PHPUNIT_RUN - && in_array('fakeinput', stream_get_wrappers())) { - $this->inputStream = 'fakeinput://data'; - } else { - $this->inputStream = 'php://input'; - } - - // Only 'application/x-www-form-urlencoded' requests are automatically - // transformed by PHP, 'application/json' must be decoded manually. - if ($this->method === 'POST' - && strpos($this->getHeader('Content-Type'), 'application/json') !== false - ) { - $this->items['params'] = $this->items['post'] = json_decode(file_get_contents($this->inputStream), true); - } + // 'application/json' must be decoded manually. + if (strpos($this->getHeader('Content-Type'), 'application/json') !== false) { + $params = json_decode(file_get_contents($this->inputStream), true); + if(count($params) > 0) { + $this->items['params'] = $params; + } + // Handle application/x-www-form-urlencoded for methods other than GET + // or post correctly + } elseif($vars['method'] !== 'GET' + && $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 +320,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; } /** diff --git a/lib/private/server.php b/lib/private/server.php index 3517d7b354..5d90a0b19f 100644 --- a/lib/private/server.php +++ b/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) { From 62cce982bbd66c11ddb7867b516bc13f95eaf9a5 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Sat, 12 Apr 2014 16:17:49 +0200 Subject: [PATCH 2/2] default to GET request when no method is set to fix unittests, also set parsed json parameters on the post attribute --- lib/private/appframework/http/request.php | 7 +++++ .../dependencyinjection/DIContainerTest.php | 2 +- tests/lib/appframework/http/RequestTest.php | 28 +++++++++++-------- .../middleware/MiddlewareDispatcherTest.php | 2 +- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php index 43631697d3..643fa685ad 100644 --- a/lib/private/appframework/http/request.php +++ b/lib/private/appframework/http/request.php @@ -65,6 +65,10 @@ class Request implements \ArrayAccess, \Countable, IRequest { $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]) ? $vars[$name] @@ -76,6 +80,9 @@ class Request implements \ArrayAccess, \Countable, IRequest { $params = json_decode(file_get_contents($this->inputStream), true); if(count($params) > 0) { $this->items['params'] = $params; + if($vars['method'] === 'POST') { + $this->items['post'] = $params; + } } // Handle application/x-www-form-urlencoded for methods other than GET // or post correctly diff --git a/tests/lib/appframework/dependencyinjection/DIContainerTest.php b/tests/lib/appframework/dependencyinjection/DIContainerTest.php index f3ebff0207..d1bc900fb2 100644 --- a/tests/lib/appframework/dependencyinjection/DIContainerTest.php +++ b/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']; diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php index 00473a8c44..58828d17bb 100644 --- a/tests/lib/appframework/http/RequestTest.php +++ b/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); diff --git a/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php b/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php index f16b14150c..935f97d2a6 100644 --- a/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php +++ b/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')))); }