From 7d5594fe1057d327b0ca2fadc009ef0082bdd211 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 25 Jul 2017 17:17:41 +0300 Subject: [PATCH 1/3] Handle single field request with empty value --- src/Controller/AbstractRestfulController.php | 15 +++++++++--- test/Controller/RestfulControllerTest.php | 25 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/Controller/AbstractRestfulController.php b/src/Controller/AbstractRestfulController.php index 454b55ed3..81552efcb 100644 --- a/src/Controller/AbstractRestfulController.php +++ b/src/Controller/AbstractRestfulController.php @@ -22,6 +22,8 @@ abstract class AbstractRestfulController extends AbstractController { const CONTENT_TYPE_JSON = 'json'; + const CONTENT_TYPE_X_WWW_FORM_URL_ENCODED = 'x-www-form-urlencoded'; + /** * {@inheritDoc} */ @@ -34,6 +36,9 @@ abstract class AbstractRestfulController extends AbstractController self::CONTENT_TYPE_JSON => [ 'application/hal+json', 'application/json' + ], + self::CONTENT_TYPE_X_WWW_FORM_URL_ENCODED => [ + 'application/x-www-form-urlencoded' ] ]; @@ -594,9 +599,13 @@ protected function processBodyContent($request) parse_str($content, $parsedParams); // If parse_str fails to decode, or we have a single element with empty value - if (! is_array($parsedParams) || empty($parsedParams) - || (1 == count($parsedParams) && '' === reset($parsedParams)) - ) { + if (! is_array($parsedParams) || empty($parsedParams)) { + return $content; + } + + // If have a single element with empty value + if (1 == count($parsedParams) && '' === reset($parsedParams) + && ! $this->requestHasContentType($request, self::CONTENT_TYPE_X_WWW_FORM_URL_ENCODED)) { return $content; } diff --git a/test/Controller/RestfulControllerTest.php b/test/Controller/RestfulControllerTest.php index 18fdf6833..46abcb9b5 100644 --- a/test/Controller/RestfulControllerTest.php +++ b/test/Controller/RestfulControllerTest.php @@ -561,4 +561,29 @@ public function providerNotImplementedMethodSets504HttpCodeProvider() ['PUT', json_encode(['foo' => 1]), []], // AbstractRestfulController::replaceList() ]; } + + /** + * @dataProvider providerParseSingleFieldObjectWithEmptyValueProvider + */ + public function testParseSingleFieldObjectWithEmptyValue($method) + { + $entity = ['name' => '']; + $string = http_build_query($entity); + $this->request->setMethod($method) + ->setContent($string); + $this->request->getHeaders()->addHeaderLine('Content-type', 'application/x-www-form-urlencoded'); + $this->routeMatch->setParam('id', 1); + $result = $this->controller->dispatch($this->request, $this->response); + $test = $result['entity']; + $this->assertArrayHasKey('name', $test); + $this->assertEquals('', $test['name']); + } + + public function providerParseSingleFieldObjectWithEmptyValueProvider() + { + return [ + ['PUT'], + ['PATCH'] + ]; + } } From bcdabf397ee6c35977a892f07aaa26b2a83949a7 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 25 Jul 2017 17:31:27 +0300 Subject: [PATCH 2/3] Fix comments --- src/Controller/AbstractRestfulController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controller/AbstractRestfulController.php b/src/Controller/AbstractRestfulController.php index 81552efcb..1380e8b0f 100644 --- a/src/Controller/AbstractRestfulController.php +++ b/src/Controller/AbstractRestfulController.php @@ -598,12 +598,12 @@ protected function processBodyContent($request) parse_str($content, $parsedParams); - // If parse_str fails to decode, or we have a single element with empty value + // If parse_str fails to decode if (! is_array($parsedParams) || empty($parsedParams)) { return $content; } - // If have a single element with empty value + // If we have a single element with empty value if (1 == count($parsedParams) && '' === reset($parsedParams) && ! $this->requestHasContentType($request, self::CONTENT_TYPE_X_WWW_FORM_URL_ENCODED)) { return $content; From 11ca56d9e8459b290602ea226cbc826e8d5e5ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Bundyra?= Date: Sun, 8 Dec 2019 10:52:49 +0000 Subject: [PATCH 3/3] Fixed parsing request content string - one field with empty value To keep BC we need to process as follows: - `foo=` -> `['foo' => '']` - `foo` -> `foo` - `foo&bar` -> `['foo' => '', 'bar' => '']` This is no longer dependent on content-type provided, as it was before. --- src/Controller/AbstractRestfulController.php | 8 +-- test/Controller/RestfulControllerTest.php | 67 +++++++++---------- .../RestfulContentTypeTestController.php | 15 +++++ 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/Controller/AbstractRestfulController.php b/src/Controller/AbstractRestfulController.php index 1380e8b0f..b1bd53050 100644 --- a/src/Controller/AbstractRestfulController.php +++ b/src/Controller/AbstractRestfulController.php @@ -22,8 +22,6 @@ abstract class AbstractRestfulController extends AbstractController { const CONTENT_TYPE_JSON = 'json'; - const CONTENT_TYPE_X_WWW_FORM_URL_ENCODED = 'x-www-form-urlencoded'; - /** * {@inheritDoc} */ @@ -36,9 +34,6 @@ abstract class AbstractRestfulController extends AbstractController self::CONTENT_TYPE_JSON => [ 'application/hal+json', 'application/json' - ], - self::CONTENT_TYPE_X_WWW_FORM_URL_ENCODED => [ - 'application/x-www-form-urlencoded' ] ]; @@ -604,8 +599,7 @@ protected function processBodyContent($request) } // If we have a single element with empty value - if (1 == count($parsedParams) && '' === reset($parsedParams) - && ! $this->requestHasContentType($request, self::CONTENT_TYPE_X_WWW_FORM_URL_ENCODED)) { + if (count($parsedParams) === 1 && reset($parsedParams) === '' && substr($content, -1) !== '=') { return $content; } diff --git a/test/Controller/RestfulControllerTest.php b/test/Controller/RestfulControllerTest.php index 46abcb9b5..4e4c1336b 100644 --- a/test/Controller/RestfulControllerTest.php +++ b/test/Controller/RestfulControllerTest.php @@ -100,22 +100,6 @@ public function testDispatchInvokesCreateMethodWhenNoActionPresentAndPostInvoked $this->assertEquals('create', $this->routeMatch->getParam('action')); } - public function testCanReceiveStringAsRequestContent() - { - $string = "any content"; - $this->request->setMethod('PUT'); - $this->request->setContent($string); - $this->routeMatch->setParam('id', $id = 1); - - $controller = new RestfulContentTypeTestController(); - $controller->setEvent($this->event); - $result = $controller->dispatch($this->request, $this->response); - - $this->assertEquals($id, $result['id']); - $this->assertEquals($string, $result['data']); - $this->assertEquals('update', $this->routeMatch->getParam('action')); - } - public function testDispatchInvokesUpdateMethodWhenNoActionPresentAndPutInvokedWithIdentifier() { $entity = ['name' => __FUNCTION__]; @@ -563,27 +547,42 @@ public function providerNotImplementedMethodSets504HttpCodeProvider() } /** - * @dataProvider providerParseSingleFieldObjectWithEmptyValueProvider + * @dataProvider nonJsonRequestContent + * + * @param string $action + * @param string $method + * @param string $content + * @param array|string $expectedResult */ - public function testParseSingleFieldObjectWithEmptyValue($method) + public function testParseNonJsonRequestContent($action, $method, $content, $expectedResult) { - $entity = ['name' => '']; - $string = http_build_query($entity); - $this->request->setMethod($method) - ->setContent($string); - $this->request->getHeaders()->addHeaderLine('Content-type', 'application/x-www-form-urlencoded'); - $this->routeMatch->setParam('id', 1); - $result = $this->controller->dispatch($this->request, $this->response); - $test = $result['entity']; - $this->assertArrayHasKey('name', $test); - $this->assertEquals('', $test['name']); + $this->request->setMethod($method); + $this->request->setContent($content); + $this->routeMatch->setParam('id', $id = 1); + + $controller = new RestfulContentTypeTestController(); + $controller->setEvent($this->event); + $result = $controller->dispatch($this->request, $this->response); + + $this->assertSame($id, $result['id']); + $this->assertSame($expectedResult, $result['data']); + $this->assertSame($action, $this->routeMatch->getParam('action')); } - public function providerParseSingleFieldObjectWithEmptyValueProvider() - { - return [ - ['PUT'], - ['PATCH'] - ]; + /** + * @return string[]|mixed[] + */ + public function nonJsonRequestContent() + { + yield ['update', 'PUT', 'foo=', ['foo' => '']]; + yield ['update', 'PUT', 'foo', 'foo']; + yield ['patch', 'PATCH', 'foo=', ['foo' => '']]; + yield ['patch', 'PATCH', 'foo', 'foo']; + yield ['update', 'PUT', 'foo&bar', ['foo' => '', 'bar' => '']]; + yield ['patch', 'PATCH', 'foo&bar', ['foo' => '', 'bar' => '']]; + yield ['update', 'PUT', 'foo=bar', ['foo' => 'bar']]; + yield ['patch', 'PATCH', 'foo=bar', ['foo' => 'bar']]; + yield ['update', 'PUT', 'any content', 'any content']; + yield ['patch', 'PATCH', 'any content', 'any content']; } } diff --git a/test/Controller/TestAsset/RestfulContentTypeTestController.php b/test/Controller/TestAsset/RestfulContentTypeTestController.php index 1def9dcd2..b415eff91 100644 --- a/test/Controller/TestAsset/RestfulContentTypeTestController.php +++ b/test/Controller/TestAsset/RestfulContentTypeTestController.php @@ -27,4 +27,19 @@ public function update($id, $data) 'data' => $data, ]; } + + /** + * Patch an existing resource + * + * @param mixed $id + * @param mixed $data + * @return array + */ + public function patch($id, $data) + { + return [ + 'id' => $id, + 'data' => $data, + ]; + } }