Skip to content

Detect correct type on request when there are multiple media types mapped to the same content type #1335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: 1.2
Choose a base branch
from
Open

Detect correct type on request when there are multiple media types mapped to the same content type #1335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
98051c9
In the case where multiple types are registered to `lithium\net\http\…
gscherer Jul 19, 2018
67448ea
Add 'name' to config array when passing data to Media::match
gscherer Jul 19, 2018
07d6206
Make return of lithium\net\http\Request::type the same as its parent …
gscherer Jul 19, 2018
b7a515f
Add unit test for request content type detection with multiple choices.
gscherer Jul 19, 2018
875ea22
Clean up line length
gscherer Jul 19, 2018
5ac829c
Move type override to `lithium\action\Request, since it depends on
gscherer Jul 20, 2018
56151e5
Don't reset \lithium\net\http\Response::$_type from Content-Type head…
gscherer Jul 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions action/Request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,17 +620,40 @@ 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.
*/
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);
}

/**
Expand Down Expand Up @@ -803,4 +826,4 @@ protected function _parseFiles($data) {
}
}

?>
?>
2 changes: 1 addition & 1 deletion net/http/Request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,4 @@ public function __toString() {
}
}

?>
?>
4 changes: 2 additions & 2 deletions net/http/Response.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand Down Expand Up @@ -437,4 +437,4 @@ public function __toString() {
}
}

?>
?>
32 changes: 31 additions & 1 deletion tests/cases/action/RequestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use lithium\core\Libraries;
use lithium\action\Request;
use lithium\net\http\Media;

class RequestTest extends \lithium\test\Unit {

Expand Down Expand Up @@ -83,6 +84,7 @@ public function tearDown() {
foreach ($this->_superglobals as $varname) {
$GLOBALS[$varname] = $this->_env[$varname];
}
Media::reset();
}

public function testInitData() {
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -1596,4 +1626,4 @@ public function testOverridingOfEnvVariables() {
}
}

?>
?>