From 965ce5719f1e936c731871360c89e459e9a20bd9 Mon Sep 17 00:00:00 2001 From: Thomas Tanghus Date: Wed, 2 Oct 2013 22:13:40 +0200 Subject: [PATCH] Modified PUT behaviour Now only non-parable PUT requests return a stream resource. --- lib/private/appframework/http/request.php | 56 +++++++++++---------- tests/lib/appframework/http/RequestTest.php | 56 ++++++++++++--------- 2 files changed, 61 insertions(+), 51 deletions(-) diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php index 8c1c2f8ec8..b6832735fa 100644 --- a/lib/private/appframework/http/request.php +++ b/lib/private/appframework/http/request.php @@ -36,7 +36,6 @@ class Request implements \ArrayAccess, \Countable, IRequest { protected $allowedKeys = array( 'get', 'post', - 'patch', 'files', 'server', 'env', @@ -69,7 +68,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { // Only 'application/x-www-form-urlencoded' requests are automatically // transformed by PHP, 'application/json' must be decoded manually. - if (isset($this->items['post']) + if ($this->method === 'POST' && strpos($this->getHeader('Content-Type'), 'application/json') !== false && is_string($this->items['post'])) { $this->items['params'] = $this->items['post'] = json_decode($this->items['post'], true); @@ -296,9 +295,9 @@ class Request implements \ArrayAccess, \Countable, IRequest { /** * Returns the request body content. * - * If the HTTP request method is PUT a stream resource is returned, otherwise an - * array or a string depending on the Content-Type. For "normal" use an array - * will be returned. + * If the HTTP request method is PUT and the body + * not application/x-www-form-urlencoded or application/json a stream + * resource is returned, otherwise an array. * * @return array|string|resource The request body content or a resource to read the body stream. * @@ -306,7 +305,10 @@ class Request implements \ArrayAccess, \Countable, IRequest { */ protected function getContent() { if ($this->content === false && $this->method === 'PUT') { - throw new \LogicException('"put" can only be accessed once.'); + throw new \LogicException( + '"put" can only be accessed once if not ' + . 'application/x-www-form-urlencoded or application/json.' + ); } if (defined('PHPUNIT_RUN') && PHPUNIT_RUN @@ -316,7 +318,11 @@ class Request implements \ArrayAccess, \Countable, IRequest { $stream = 'php://input'; } - if ($this->method === 'PUT') { + // 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 + ) { $this->content = false; return fopen($stream, 'rb'); } @@ -324,25 +330,23 @@ class Request implements \ArrayAccess, \Countable, IRequest { if (is_null($this->content)) { $this->content = file_get_contents($stream); - if ($this->method === 'PATCH') { - /* - * 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; - } + /* + * 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; } } } diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php index aaa2328be1..acd61e7091 100644 --- a/tests/lib/appframework/http/RequestTest.php +++ b/tests/lib/appframework/http/RequestTest.php @@ -12,6 +12,18 @@ global $data; class RequestTest extends \PHPUnit_Framework_TestCase { + public function setUp() { + require_once __DIR__ . '/requeststream.php'; + if (in_array('fakeinput', stream_get_wrappers())) { + stream_wrapper_unregister('fakeinput'); + } + stream_wrapper_register('fakeinput', 'RequestStream'); + } + + public function tearDown() { + stream_wrapper_unregister('fakeinput'); + } + public function testRequestAccessors() { $vars = array( 'get' => array('name' => 'John Q. Public', 'nickname' => 'Joey'), @@ -34,7 +46,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase { // Always returns null if variable not set. $this->assertEquals(null, $request->{'flickname'}); - require_once __DIR__ . '/requeststream.php'; } // urlParams has precedence over POST which has precedence over GET @@ -123,11 +134,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase { global $data; $data = http_build_query(array('name' => 'John Q. Public', 'nickname' => 'Joey'), '', '&'); - if (in_array('fakeinput', stream_get_wrappers())) { - stream_wrapper_unregister('fakeinput'); - } - stream_wrapper_register('fakeinput', 'RequestStream'); - $vars = array( 'patch' => $data, 'method' => 'PATCH', @@ -141,21 +147,29 @@ class RequestTest extends \PHPUnit_Framework_TestCase { $this->assertEquals('John Q. Public', $result['name']); $this->assertEquals('Joey', $result['nickname']); - - stream_wrapper_unregister('fakeinput'); } - public function testJsonPatch() { + public function testJsonPatchAndPut() { global $data; - $data = '{"name": "John Q. Public", "nickname": null}'; - - if (in_array('fakeinput', stream_get_wrappers())) { - stream_wrapper_unregister('fakeinput'); - } - stream_wrapper_register('fakeinput', 'RequestStream'); + // PUT content + $data = '{"name": "John Q. Public", "nickname": "Joey"}'; + $vars = array( + 'method' => 'PUT', + 'server' => array('CONTENT_TYPE' => 'application/json; utf-8'), + ); + + $request = new Request($vars); + + $this->assertEquals('PUT', $request->method); + $result = $request->put; + + $this->assertEquals('John Q. Public', $result['name']); + $this->assertEquals('Joey', $result['nickname']); + + // PATCH content + $data = '{"name": "John Q. Public", "nickname": null}'; $vars = array( - 'patch' => $data, 'method' => 'PATCH', 'server' => array('CONTENT_TYPE' => 'application/json; utf-8'), ); @@ -167,19 +181,12 @@ class RequestTest extends \PHPUnit_Framework_TestCase { $this->assertEquals('John Q. Public', $result['name']); $this->assertEquals(null, $result['nickname']); - - stream_wrapper_unregister('fakeinput'); } - public function testPutSteam() { + public function testPutStream() { global $data; $data = file_get_contents(__DIR__ . '/../../../data/testimage.png'); - if (in_array('fakeinput', stream_get_wrappers())) { - stream_wrapper_unregister('fakeinput'); - } - stream_wrapper_register('fakeinput', 'RequestStream'); - $vars = array( 'put' => $data, 'method' => 'PUT', @@ -195,7 +202,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase { try { $resource = $request->put; } catch(\LogicException $e) { - stream_wrapper_unregister('fakeinput'); return; } $this->fail('Expected LogicException.');