From 98051c9a53a5818af969d314fae2a0b605d3bf70 Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Thu, 19 Jul 2018 13:21:06 -0600 Subject: [PATCH 1/7] In the case where multiple types are registered to `lithium\net\http\Media` with the same content types, even when distinguished by `conditions`, an instance of `lithium\net\http\Request` will fail to determine the correct type to use to decode the request body. For example, if we register another type which use 'application/json' as a content type like this: ```php use lithium\net\http\Media; Media::type('json_base64', ['application/json'], [ 'encode' => function ($data) { return base64_encode(json_encode($data)); }, 'decode' => function ($data) { return json_decode(base64_decode($data), true); }, 'cast' => true, 'conditions' => [ 'http:content_transfer_encoding' => 'base64' ] ]); ``` In this case, a call to `Media::type('application/json')` will return an array like: ```php [ 'json_base64', 'json' ] ``` This case is not handled by `lithium\net\http\Message::type`, which causes `lithium\net\http\Request::$_type to be assigned as the content type of 'application/json'. Futhermore, when `Request::body` is called to decode the request body, the Media class fails to find a handler for 'application/json' (it expects a short name like 'json'). To fix this, `lithium\net\http\Message::type` must be extended by `lithium\net\http\Request`. In the case that the type is still a full content type like 'application/json', we can loop over each short name returned by `Media::type` and attempt to match the request like this: ```php public function type($type = null) { $type = parent::type($type); $media = $this->_classes['media']; if (strpos($type, '/') !== false) { $data = $media::type($type); if (is_array($data) && !isset($data['content'])) { foreach ($data as $short_type) { $conf = $media::type($short_type); if ($media::match($this, $conf)) { $type = $short_type; break; } } } } return $this->_type = $type; } ``` This will correctly determine a single short name to use for the type of the request data, and it can now correctly decode the request body. --- net/http/Request.php | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/net/http/Request.php b/net/http/Request.php index 695a1709a1..306813a4b9 100644 --- a/net/http/Request.php +++ b/net/http/Request.php @@ -139,6 +139,35 @@ public function __construct(array $config = []) { ]; } + /** + * Handle the case where multiple types are registered with the Media class + * with the same Content-Type where conditions distinguish them. In these + * cases, to correctly determine the "short type" from the full type + * (e.g. 'json' from 'application/json') we must loop over each short type + * returned from Media::type and attempt to match the request to it. + * + * @param string $type A full content type i.e. `'application/json'` or simple name `'json'` + * @return string A simple content type name, i.e. `'html'`, `'xml'`, `'json'`, etc., depending + * on the content type of the request. + */ + public function type($type = null) { + $type = parent::type($type); + $media = $this->_classes['media']; + if (strpos($type, '/') !== false) { + $data = $media::type($type); + if (is_array($data) && !isset($data['content'])) { + foreach ($data as $short_type) { + $conf = $media::type($short_type); + if ($media::match($this, $conf)) { + $type = $short_type; + break; + } + } + } + } + return $this->_type = $type; + } + /** * Compile the HTTP message body, optionally encoding its parts according to content type. * @@ -355,4 +384,4 @@ public function __toString() { } } -?> \ No newline at end of file +?> From 67448eadd16674b86ca76e38c4063704657e6f1d Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Thu, 19 Jul 2018 15:35:27 -0600 Subject: [PATCH 2/7] Add 'name' to config array when passing data to Media::match --- net/http/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/http/Request.php b/net/http/Request.php index 306813a4b9..bc4dba963c 100644 --- a/net/http/Request.php +++ b/net/http/Request.php @@ -158,7 +158,7 @@ public function type($type = null) { if (is_array($data) && !isset($data['content'])) { foreach ($data as $short_type) { $conf = $media::type($short_type); - if ($media::match($this, $conf)) { + if ($media::match($this, ['name' => $short_type] + $conf)) { $type = $short_type; break; } From 07d620610c9746770353e75b48f86a10805c4394 Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Thu, 19 Jul 2018 16:08:31 -0600 Subject: [PATCH 3/7] Make return of lithium\net\http\Request::type the same as its parent class. --- net/http/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/http/Request.php b/net/http/Request.php index bc4dba963c..b7b7e98e02 100644 --- a/net/http/Request.php +++ b/net/http/Request.php @@ -165,7 +165,7 @@ public function type($type = null) { } } } - return $this->_type = $type; + return ($this->_type = $type); } /** From b7a515fcd8ea212e2ed6e330b0bfed6eb3d2b551 Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Thu, 19 Jul 2018 17:00:01 -0600 Subject: [PATCH 4/7] Add unit test for request content type detection with multiple choices. --- tests/cases/action/RequestTest.php | 32 +++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/cases/action/RequestTest.php b/tests/cases/action/RequestTest.php index d9c22f6294..c1a41ee5e1 100644 --- a/tests/cases/action/RequestTest.php +++ b/tests/cases/action/RequestTest.php @@ -11,6 +11,7 @@ use lithium\core\Libraries; use lithium\action\Request; +use lithium\net\http\Media; class RequestTest extends \lithium\test\Unit { @@ -83,6 +84,7 @@ public function tearDown() { foreach ($this->_superglobals as $varname) { $GLOBALS[$varname] = $this->_env[$varname]; } + Media::reset(); } public function testInitData() { @@ -488,6 +490,34 @@ public function testContentTypeDetection() { $this->assertFalse($request->is('foo')); } + public function testContentTypeDetectionWithMultipleChoices() { + Media::type('json_base64', ['application/json'], [ + 'encode' => function ($data) { + return base64_encode(json_encode($data)); + }, + 'decode' => function ($data) { + return json_decode(base64_decode($data), true); + }, + 'cast' => true, + 'conditions' => [ + 'http:content_transfer_encoding' => 'base64' + ] + ]); + $request = new Request(['env' => [ + 'CONTENT_TYPE' => 'application/json; charset=UTF-8', + 'REQUEST_METHOD' => 'POST' + ]]); + $this->assertTrue($request->is('json')); + $this->assertFalse($request->is('json_base64')); + $request = new Request(['env' => [ + 'CONTENT_TYPE' => 'application/json; charset=UTF-8', + 'REQUEST_METHOD' => 'POST', + 'HTTP_CONTENT_TRANSFER_ENCODING' => 'base64' + ]]); + $this->assertTrue($request->is('json_base64')); + $this->assertFalse($request->is('json')); + } + public function testIsMobile() { $iPhone = 'Mozilla/5.0 (iPhone; U; CPU like Mac OS X; en) AppleWebKit/420+ (KHTML, like '; $iPhone .= 'Gecko) Version/3.0 Mobile/1A535b Safari/419.3'; @@ -1596,4 +1626,4 @@ public function testOverridingOfEnvVariables() { } } -?> \ No newline at end of file +?> From 875ea2257c7f515e6b4d36b945767582aa004893 Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Thu, 19 Jul 2018 17:52:12 -0600 Subject: [PATCH 5/7] Clean up line length --- net/http/Request.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/http/Request.php b/net/http/Request.php index b7b7e98e02..f0dae5d422 100644 --- a/net/http/Request.php +++ b/net/http/Request.php @@ -152,13 +152,14 @@ public function __construct(array $config = []) { */ public function type($type = null) { $type = parent::type($type); - $media = $this->_classes['media']; if (strpos($type, '/') !== false) { + $media = $this->_classes['media']; $data = $media::type($type); if (is_array($data) && !isset($data['content'])) { foreach ($data as $short_type) { $conf = $media::type($short_type); - if ($media::match($this, ['name' => $short_type] + $conf)) { + $conf['name'] = $short_type; + if ($media::match($this, $conf)) { $type = $short_type; break; } From 5ac829cb8ef262e6cc515a8945c50be41b61c92c Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Fri, 20 Jul 2018 00:32:38 -0600 Subject: [PATCH 6/7] Move type override to `lithium\action\Request, since it depends on `lithium\net\http\Media::match`, which expects the Request::get() method to be available. --- action/Request.php | 31 +++++++++++++++++++++++++++---- net/http/Request.php | 30 ------------------------------ 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/action/Request.php b/action/Request.php index b427e0e37e..690ee7a2f3 100644 --- a/action/Request.php +++ b/action/Request.php @@ -620,9 +620,17 @@ public function is($flag) { /** * Sets/Gets the content type. If `'type'` is null, the method will attempt to determine the - * type from the params, then from the environment setting + * type from the params, then from the environment setting. * - * @param string $type a full content type i.e. `'application/json'` or simple name `'json'` + * Handle the case where a single type could not be determined by a call to + * `lithium\net\http\Media::type` in `lithium\net\nttp\Message::type`. + * In that case, the value of `$type` is returned from the parent as it + * was passed in. Attempt to use `lithium\net\http\Media::match` to + * distinguish which type to use for the request. The type is later used to + * decode the request body. Not handling this case leads to the correct + * type's decoder not being invoked on the body of the request. + * + * @param string $type A full content type i.e. `'application/json'` or simple name `'json'` * @return string A simple content type name, i.e. `'html'`, `'xml'`, `'json'`, etc., depending * on the content type of the request. */ @@ -630,7 +638,22 @@ public function type($type = null) { if (!$type && !empty($this->params['type'])) { $type = $this->params['type']; } - return parent::type($type); + $_type = parent::type($type); + if (is_string($type) && $_type === $type) { + $media = $this->_classes['media']; + $content = $media::type($type); + if (is_array($content) && !isset($content['content'])) { + foreach ($content as $short_type) { + $conf = $media::type($short_type); + $conf['name'] = $short_type; + if ($media::match($this, $conf)) { + $_type = $short_type; + break; + } + } + } + } + return ($this->_type = $_type); } /** @@ -803,4 +826,4 @@ protected function _parseFiles($data) { } } -?> \ No newline at end of file +?> diff --git a/net/http/Request.php b/net/http/Request.php index f0dae5d422..7fa35098d9 100644 --- a/net/http/Request.php +++ b/net/http/Request.php @@ -139,36 +139,6 @@ public function __construct(array $config = []) { ]; } - /** - * Handle the case where multiple types are registered with the Media class - * with the same Content-Type where conditions distinguish them. In these - * cases, to correctly determine the "short type" from the full type - * (e.g. 'json' from 'application/json') we must loop over each short type - * returned from Media::type and attempt to match the request to it. - * - * @param string $type A full content type i.e. `'application/json'` or simple name `'json'` - * @return string A simple content type name, i.e. `'html'`, `'xml'`, `'json'`, etc., depending - * on the content type of the request. - */ - public function type($type = null) { - $type = parent::type($type); - if (strpos($type, '/') !== false) { - $media = $this->_classes['media']; - $data = $media::type($type); - if (is_array($data) && !isset($data['content'])) { - foreach ($data as $short_type) { - $conf = $media::type($short_type); - $conf['name'] = $short_type; - if ($media::match($this, $conf)) { - $type = $short_type; - break; - } - } - } - } - return ($this->_type = $type); - } - /** * Compile the HTTP message body, optionally encoding its parts according to content type. * From 56151e5bbd6080ffadc63da4069507d56745d5e8 Mon Sep 17 00:00:00 2001 From: Grayson Scherer Date: Wed, 25 Jul 2018 10:47:29 -0600 Subject: [PATCH 7/7] Don't reset \lithium\net\http\Response::$_type from Content-Type header if it is already set via configuration --- net/http/Response.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/http/Response.php b/net/http/Response.php index bd2f01a288..b4f5a3e679 100644 --- a/net/http/Response.php +++ b/net/http/Response.php @@ -147,7 +147,7 @@ public function __construct(array $config = []) { $header = is_array($header) ? end($header) : $header; preg_match('/([-\w\/\.+]+)(;\s*?charset=(.+))?/i', $header, $match); - if (isset($match[1])) { + if (!$this->_type && isset($match[1])) { $this->type(trim($match[1])); } if (isset($match[3])) { @@ -437,4 +437,4 @@ public function __toString() { } } -?> \ No newline at end of file +?>