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
cakephp · Oct 13, 2011 · 4090b3e · 4090b3e
1 parent 7f3c566
commit 4090b3e
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 23 deletions.
diff --git 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
@@ -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();