From 4090b3e8c60441702c9ddf1565c5497071ce2aad Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 12 Oct 2011 23:07:02 -0400 Subject: [PATCH] Fix content-type detection to accomodate jQuery. Add tests for jQuery content type strings. Refactor tests, add in missing assertions and missing parent calls. The new behavior is more lenient and allows for a single requested content type to switch the view type. Fixes #2088 --- .../Component/RequestHandlerComponent.php | 49 ++++++++++----- .../Component/RequestHandlerComponentTest.php | 60 ++++++++++++++++--- 2 files changed, 86 insertions(+), 23 deletions(-) diff --git a/lib/Cake/Controller/Component/RequestHandlerComponent.php b/lib/Cake/Controller/Component/RequestHandlerComponent.php index df9603cf85c..b69c4784341 100644 --- a/lib/Cake/Controller/Component/RequestHandlerComponent.php +++ b/lib/Cake/Controller/Component/RequestHandlerComponent.php @@ -96,10 +96,10 @@ public function __construct(ComponentCollection $collection, $settings = array() } /** - * Initializes the component, gets a reference to Controller::$parameters, and - * checks to see if a file extension has been parsed by the Router. Or if the - * HTTP_ACCEPT_TYPE is set to a single value that is a supported extension and mapped type. - * If yes, RequestHandler::$ext is set to that value + * Checks to see if a file extension has been parsed by the Router, or if the + * HTTP_ACCEPT_TYPE has matches only one content type with the supported extensions. + * If there is only one matching type between the supported content types & extensions, + * and the requested mime-types, RequestHandler::$ext is set to that value. * * @param Controller $controller A reference to the controller * @param array $settings Array of settings to _set(). @@ -113,19 +113,37 @@ public function initialize($controller, $settings = array()) { $this->ext = $this->request->params['ext']; } if (empty($this->ext) || $this->ext == 'html') { - $accepts = $this->request->accepts(); - $extensions = Router::extensions(); - if (count($accepts) == 1) { - $mapped = $this->mapType($accepts[0]); - if (in_array($mapped, $extensions)) { - $this->ext = $mapped; - } - } + $this->_setExtension(); } $this->params = $controller->params; $this->_set($settings); } +/** + * Set the extension based on the accept headers. + * Compares the accepted types and configured extensions. + * If there is one common type, that is assigned as the ext/content type + * for the response. + * + * If html is one of the preferred types, no content type will be set, this + * is to avoid issues with browsers that prefer html and several other content types. + * + * @return void + */ + protected function _setExtension() { + $accept = $this->request->parseAccept(); + if (empty($accept)) { + return; + } + $extensions = Router::extensions(); + $preferred = array_shift($accept); + $preferredTypes = $this->mapType($preferred); + $similarTypes = array_intersect($extensions, $preferredTypes); + if (count($similarTypes) === 1 && !in_array('html', $preferredTypes)) { + $this->ext = $similarTypes[0]; + } + } + /** * The startup method of the RequestHandler enables several automatic behaviors * related to the detection of certain properties of the HTTP request, including: @@ -455,7 +473,7 @@ public function requestedWith($type = null) { /** * Determines which content-types the client prefers. If no parameters are given, - * the content-type that the client most likely prefers is returned. If $type is + * the single content-type that the client most likely prefers is returned. If $type is * an array, the first item in the array that the client accepts is returned. * Preference is determined primarily by the file extension parsed by the Router * if provided, and secondarily by the list of content-types provided in @@ -464,7 +482,10 @@ public function requestedWith($type = null) { * @param mixed $type An optional array of 'friendly' content-type names, i.e. * 'html', 'xml', 'js', etc. * @return mixed If $type is null or not provided, the first content-type in the - * list, based on preference, is returned. + * list, based on preference, is returned. If a single type is provided + * a boolean will be returnend if that type is preferred. + * If an array of types are provided then the first preferred type is returned. + * If no type is provided the first preferred type is returned. * @see RequestHandlerComponent::setContent() */ public function prefers($type = null) { diff --git a/lib/Cake/Test/Case/Controller/Component/RequestHandlerComponentTest.php b/lib/Cake/Test/Case/Controller/Component/RequestHandlerComponentTest.php index 1d0303785d3..bf27661c9eb 100644 --- a/lib/Cake/Test/Case/Controller/Component/RequestHandlerComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/RequestHandlerComponentTest.php @@ -103,6 +103,7 @@ class RequestHandlerComponentTest extends CakeTestCase { * @return void */ public function setUp() { + parent::setUp(); $this->_server = $_SERVER; $this->_init(); } @@ -119,6 +120,7 @@ function _init() { $this->RequestHandler = new RequestHandlerComponent($this->Controller->Components); $this->RequestHandler->request = $request; $this->RequestHandler->response = $response; + $this->_extensions = Router::extensions(); } /** @@ -127,13 +129,13 @@ function _init() { * @return void */ public function tearDown() { - unset($this->RequestHandler); - unset($this->Controller); + parent::tearDown(); + unset($this->RequestHandler, $this->Controller); if (!headers_sent()) { header('Content-type: text/html'); //reset content type. } $_SERVER = $this->_server; - App::build(); + call_user_func_array('Router::parseExtensions', $this->_extensions); } /** @@ -169,16 +171,53 @@ public function testInitializeCallback() { */ public function testInitializeContentTypeSettingExt() { $this->assertNull($this->RequestHandler->ext); - $extensions = Router::extensions(); + + $_SERVER['HTTP_ACCEPT'] = 'application/json'; Router::parseExtensions('json'); - $this->Controller->request = $this->getMock('CakeRequest'); - $this->Controller->request->expects($this->any())->method('accepts') - ->will($this->returnValue(array('application/json'))); $this->RequestHandler->initialize($this->Controller); $this->assertEquals('json', $this->RequestHandler->ext); + } - call_user_func_array(array('Router', 'parseExtensions'), $extensions); +/** + * Test that RequestHandler sets $this->ext when jQuery sends its wonky-ish headers. + * + * @return void + */ + public function testInitializeContentTypeWithjQueryAccept() { + $_SERVER['HTTP_ACCEPT'] = 'application/json, text/javascript, */*; q=0.01'; + $this->assertNull($this->RequestHandler->ext); + Router::parseExtensions('json'); + + $this->RequestHandler->initialize($this->Controller); + $this->assertEquals('json', $this->RequestHandler->ext); + } + +/** + * Test that RequestHandler does not set $this->ext when multple accepts are sent. + * + * @return void + */ + public function testInitializeNoContentTypeWithSingleAccept() { + $_SERVER['HTTP_ACCEPT'] = 'application/json, text/html, */*; q=0.01'; + $this->assertNull($this->RequestHandler->ext); + Router::parseExtensions('json'); + + $this->RequestHandler->initialize($this->Controller); + $this->assertNull($this->RequestHandler->ext); + } +/** + * Test that ext is not set with multiple accepted content types. + * + * @return void + */ + public function testInitializeNoContentTypeWithMultipleAcceptedTypes() { + $_SERVER['HTTP_ACCEPT'] = 'application/json, text/javascript, application/xml, */*; q=0.01'; + $this->assertNull($this->RequestHandler->ext); + Router::parseExtensions('xml', 'json'); + + $this->RequestHandler->initialize($this->Controller); + $this->assertNull($this->RequestHandler->ext); } /** @@ -192,7 +231,8 @@ public function testInitializeContentTypeAndExtensionMismatch() { Router::parseExtensions('xml'); $this->Controller->request = $this->getMock('CakeRequest'); - $this->Controller->request->expects($this->any())->method('accepts') + $this->Controller->request->expects($this->any()) + ->method('accepts') ->will($this->returnValue(array('application/json'))); $this->RequestHandler->initialize($this->Controller); @@ -602,6 +642,8 @@ public function testPrefers() { $this->assertEqual($this->RequestHandler->prefers(array('js', 'xml', 'xhtml')), 'xml'); $this->assertFalse($this->RequestHandler->prefers(array('red', 'blue'))); $this->assertEqual($this->RequestHandler->prefers(array('js', 'json', 'xhtml')), 'xhtml'); + $this->assertTrue($this->RequestHandler->prefers(array('rss')), 'Should return true if input matches ext.'); + $this->assertFalse($this->RequestHandler->prefers(array('html')), 'No match with ext, return false.'); $_SERVER['HTTP_ACCEPT'] = 'text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5'; $this->_init();