From fb720c7e605fc31c90b4fbbcd2b527817f6628c8 Mon Sep 17 00:00:00 2001 From: mscherer Date: Sun, 23 Sep 2018 15:49:18 +0200 Subject: [PATCH 1/9] Fix silencing errors in extension handling. --- .../Component/RequestHandlerComponent.php | 9 ++++++++- src/Http/Response.php | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/Controller/Component/RequestHandlerComponent.php b/src/Controller/Component/RequestHandlerComponent.php index f0ad7d27938..7ddaf665444 100644 --- a/src/Controller/Component/RequestHandlerComponent.php +++ b/src/Controller/Component/RequestHandlerComponent.php @@ -21,6 +21,7 @@ use Cake\Core\Configure; use Cake\Core\Exception\Exception; use Cake\Event\Event; +use Cake\Http\Exception\NotFoundException; use Cake\Http\Response; use Cake\Routing\Router; use Cake\Utility\Exception\XmlException; @@ -332,8 +333,11 @@ public function beforeRender(Event $event) !in_array($this->ext, ['html', 'htm']) && $response->getMimeType($this->ext) ); + if ($this->ext && !$isRecognized) { + throw new NotFoundException('Invoked extension not recognized: ' . $this->ext); + } - if ($this->ext && $isRecognized) { + if ($this->ext) { $this->renderAs($controller, $this->ext); $response = $controller->response; } else { @@ -594,6 +598,9 @@ public function renderAs(Controller $controller, $type, array $options = []) $viewClass = null; if ($builder->getClassName() === null) { $viewClass = App::className($view, 'View', 'View'); + if ($viewClass === false) { + throw new RuntimeException('Configured view class can not be found: ' . $view); + } } if ($viewClass) { diff --git a/src/Http/Response.php b/src/Http/Response.php index 7777a883f33..4908d28d603 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -1111,7 +1111,7 @@ public function type($contentType = null) { deprecationWarning( 'Response::type() is deprecated. ' . - 'Use getType() or withType() instead.' + 'Use getType(), setType() or withType() instead.' ); if ($contentType === null) { @@ -1137,6 +1137,22 @@ public function type($contentType = null) return $contentType; } + /** + * Sets a content type into the map. + * + * E.g.: setType('xhtml' => ['application/xhtml+xml', 'application/xhtml', 'text/xhtml']) + * + * This is needed for RequestHandlerComponent and recognition of types. + * + * @param string $type + * @param string|array $definition + * @return void + */ + public function setType($type, $definition) + { + $this->_mimeTypes[$type] = $definition; + } + /** * Returns the current content type. * From 4bcd6f7aa6eecccf4d9650e2fde2c1dbb5814459 Mon Sep 17 00:00:00 2001 From: mscherer Date: Sun, 23 Sep 2018 15:52:49 +0200 Subject: [PATCH 2/9] Fix CS. --- src/Http/Response.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Response.php b/src/Http/Response.php index 4908d28d603..2cafaa3a216 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -1144,8 +1144,8 @@ public function type($contentType = null) * * This is needed for RequestHandlerComponent and recognition of types. * - * @param string $type - * @param string|array $definition + * @param string $type Content type. + * @param string|array $definition Definition of the content type. * @return void */ public function setType($type, $definition) From 5262d0d037e883d8291afe63475410cbe2fd1fe0 Mon Sep 17 00:00:00 2001 From: mscherer Date: Sun, 23 Sep 2018 16:22:42 +0200 Subject: [PATCH 3/9] Adjust as per review. --- src/Http/Response.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Http/Response.php b/src/Http/Response.php index 2cafaa3a216..b3aece13bc8 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -1145,12 +1145,12 @@ public function type($contentType = null) * This is needed for RequestHandlerComponent and recognition of types. * * @param string $type Content type. - * @param string|array $definition Definition of the content type. + * @param string|array $mimeType Definition of the mime type. * @return void */ - public function setType($type, $definition) + public function setType($type, $mimeType) { - $this->_mimeTypes[$type] = $definition; + $this->_mimeTypes[$type] = $mimeType; } /** From 78340f5fa64faae2a69f54dd061fbc43b73aa245 Mon Sep 17 00:00:00 2001 From: mscherer Date: Sun, 23 Sep 2018 16:43:24 +0200 Subject: [PATCH 4/9] Add tests --- src/Http/Response.php | 8 ++++---- tests/TestCase/Http/ResponseTest.php | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Http/Response.php b/src/Http/Response.php index b3aece13bc8..857a449d836 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -1111,7 +1111,7 @@ public function type($contentType = null) { deprecationWarning( 'Response::type() is deprecated. ' . - 'Use getType(), setType() or withType() instead.' + 'Use setTypeMap(), getType() or withType() instead.' ); if ($contentType === null) { @@ -1138,9 +1138,9 @@ public function type($contentType = null) } /** - * Sets a content type into the map. + * Sets a content type definition into the map. * - * E.g.: setType('xhtml' => ['application/xhtml+xml', 'application/xhtml', 'text/xhtml']) + * E.g.: setType('xhtml' => ['application/xhtml+xml', 'application/xhtml']) * * This is needed for RequestHandlerComponent and recognition of types. * @@ -1148,7 +1148,7 @@ public function type($contentType = null) * @param string|array $mimeType Definition of the mime type. * @return void */ - public function setType($type, $mimeType) + public function setTypeMap($type, $mimeType) { $this->_mimeTypes[$type] = $mimeType; } diff --git a/tests/TestCase/Http/ResponseTest.php b/tests/TestCase/Http/ResponseTest.php index 2c36667e8f9..06e6f55ec79 100644 --- a/tests/TestCase/Http/ResponseTest.php +++ b/tests/TestCase/Http/ResponseTest.php @@ -265,6 +265,30 @@ public function testGetType() ); } + /** + * @return void + */ + public function testSetTypeMap() + { + $response = new Response(); + $response->setTypeMap('ical', 'text/calendar'); + + $response = $response->withType('ical')->getType(); + $this->assertEquals('text/calendar', $response); + } + + /** + * @return void + */ + public function testSetTypeMapAsArray() + { + $response = new Response(); + $response->setTypeMap('ical', ['text/calendar']); + + $response = $response->withType('ical')->getType(); + $this->assertEquals('text/calendar', $response); + } + /** * Tests the withType method * From cd063fac7c43d4fb7ad0b1480f754a6d08cf980e Mon Sep 17 00:00:00 2001 From: Mark Sch Date: Wed, 26 Sep 2018 13:19:54 +0200 Subject: [PATCH 5/9] Revert exception for strict class name finding. --- src/Controller/Component/RequestHandlerComponent.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Controller/Component/RequestHandlerComponent.php b/src/Controller/Component/RequestHandlerComponent.php index 7ddaf665444..491ef95ce0a 100644 --- a/src/Controller/Component/RequestHandlerComponent.php +++ b/src/Controller/Component/RequestHandlerComponent.php @@ -598,9 +598,6 @@ public function renderAs(Controller $controller, $type, array $options = []) $viewClass = null; if ($builder->getClassName() === null) { $viewClass = App::className($view, 'View', 'View'); - if ($viewClass === false) { - throw new RuntimeException('Configured view class can not be found: ' . $view); - } } if ($viewClass) { From 2e95a152d8c321cd357a4975dc71a15850afc1cc Mon Sep 17 00:00:00 2001 From: mscherer Date: Thu, 27 Sep 2018 11:11:34 +0200 Subject: [PATCH 6/9] Add test case. --- .../Component/RequestHandlerComponent.php | 3 +- .../Component/RequestHandlerComponentTest.php | 38 +++++++++++-------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/Controller/Component/RequestHandlerComponent.php b/src/Controller/Component/RequestHandlerComponent.php index 491ef95ce0a..e5b3970b2e7 100644 --- a/src/Controller/Component/RequestHandlerComponent.php +++ b/src/Controller/Component/RequestHandlerComponent.php @@ -321,6 +321,7 @@ public function beforeRedirect(Event $event, $url, Response $response) * * @param \Cake\Event\Event $event The Controller.beforeRender event. * @return bool false if the render process should be aborted + * @throws \Cake\Http\Exception\NotFoundException If invoked extension is not configured. */ public function beforeRender(Event $event) { @@ -334,7 +335,7 @@ public function beforeRender(Event $event) $response->getMimeType($this->ext) ); if ($this->ext && !$isRecognized) { - throw new NotFoundException('Invoked extension not recognized: ' . $this->ext); + throw new NotFoundException('Invoked extension not recognized/configured: ' . $this->ext); } if ($this->ext) { diff --git a/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php b/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php index 0701cbce00d..6c8f945c976 100644 --- a/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php +++ b/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php @@ -20,6 +20,7 @@ use Cake\Http\Response; use Cake\Http\ServerRequest; use Cake\Routing\DispatcherFactory; +use Cake\Routing\RouteBuilder; use Cake\Routing\Router; use Cake\TestSuite\TestCase; use Cake\View\AjaxView; @@ -35,15 +36,11 @@ class RequestHandlerComponentTest extends TestCase { /** - * Controller property - * * @var RequestHandlerTestController */ public $Controller; /** - * RequestHandler property - * * @var RequestHandlerComponent */ public $RequestHandler; @@ -90,7 +87,7 @@ protected function _init() $this->RequestHandler = $this->Controller->components()->load('RequestHandler'); $this->request = $request; - Router::scope('/', function ($routes) { + Router::scope('/', function (RouteBuilder $routes) { $routes->setExtensions('json'); $routes->fallbacks('InflectedRoute'); }); @@ -383,7 +380,6 @@ public function testViewClassMapMethod() { $this->deprecated(function () { $this->RequestHandler->setConfig(['viewClassMap' => ['json' => 'CustomJson']]); - $this->RequestHandler->initialize([]); $result = $this->RequestHandler->viewClassMap(); $expected = [ 'json' => 'CustomJson', @@ -416,7 +412,6 @@ public function testIsAjaxParams() { $this->Controller->request = $this->request->withHeader('X-Requested-With', 'XMLHttpRequest'); $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->Controller->beforeFilter($event); $this->RequestHandler->startup($event); $this->assertTrue($this->Controller->request->getParam('isAjax')); @@ -432,7 +427,6 @@ public function testAutoAjaxLayout() { $event = new Event('Controller.startup', $this->Controller); $this->Controller->request = $this->request->withHeader('X-Requested-With', 'XMLHttpRequest'); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $event = new Event('Controller.beforeRender', $this->Controller); $this->RequestHandler->beforeRender($event); @@ -443,7 +437,6 @@ public function testAutoAjaxLayout() $this->_init(); $this->Controller->request = $this->Controller->request->withParam('_ext', 'js'); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $this->assertNotEquals(AjaxView::class, $this->Controller->viewBuilder()->getClassName()); } @@ -459,7 +452,6 @@ public function testJsonViewLoaded() Router::extensions(['json', 'xml', 'ajax'], false); $this->Controller->request = $this->Controller->request->withParam('_ext', 'json'); $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $event = new Event('Controller.beforeRender', $this->Controller); $this->RequestHandler->beforeRender($event); @@ -480,7 +472,6 @@ public function testXmlViewLoaded() Router::extensions(['json', 'xml', 'ajax'], false); $this->Controller->request = $this->Controller->request->withParam('_ext', 'xml'); $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $event = new Event('Controller.beforeRender', $this->Controller); $this->RequestHandler->beforeRender($event); @@ -501,7 +492,6 @@ public function testAjaxViewLoaded() Router::extensions(['json', 'xml', 'ajax'], false); $this->Controller->request = $this->Controller->request->withParam('_ext', 'ajax'); $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $event = new Event('Controller.beforeRender', $this->Controller); $this->RequestHandler->beforeRender($event); @@ -521,7 +511,6 @@ public function testNoViewClassExtension() Router::extensions(['json', 'xml', 'ajax', 'csv'], false); $this->Controller->request = $this->Controller->request->withParam('_ext', 'csv'); $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $this->Controller->getEventManager()->on('Controller.beforeRender', function () { return $this->Controller->response; @@ -531,6 +520,26 @@ public function testNoViewClassExtension() $this->assertEquals('csv', $this->Controller->viewBuilder()->getLayoutPath()); } + /** + * Tests that configured extensions that have no configured mimetype do not silently fallback to HTML. + * + * @return void + * @expectedException \Cake\Http\Exception\NotFoundException + * @expectedExceptionMessage Invoked extension not recognized/configured: foo + */ + public function testUnrecognizedExtensionFailure() + { + Router::extensions(['json', 'foo'], false); + $this->Controller->request = $this->Controller->request->withParam('_ext', 'foo'); + $event = new Event('Controller.startup', $this->Controller); + $this->RequestHandler->startup($event); + $this->Controller->getEventManager()->on('Controller.beforeRender', function () { + return $this->Controller->response; + }); + $this->Controller->render(); + $this->assertEquals('RequestHandlerTest' . DS . 'csv', $this->Controller->viewBuilder()->getTemplatePath()); + } + /** * testStartupCallback method * @@ -799,7 +808,6 @@ public function testBeforeRedirectDisabled() $this->Controller->request = $this->Controller->request->withHeader('X-Requested-With', 'XMLHttpRequest'); $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->RequestHandler->setConfig('enableBeforeRedirect', false); $this->RequestHandler->startup($event); $this->assertNull($this->RequestHandler->beforeRedirect($event, '/posts/index', $this->Controller->response)); @@ -816,7 +824,6 @@ public function testNonAjaxRedirect() { $this->deprecated(function () { $event = new Event('Controller.startup', $this->Controller); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $this->assertNull($this->RequestHandler->beforeRedirect($event, '/', $this->Controller->response)); }); @@ -839,7 +846,6 @@ public function testAjaxRedirectWithNoUrl() $this->Controller->response->expects($this->never()) ->method('body'); - $this->RequestHandler->initialize([]); $this->RequestHandler->startup($event); $this->assertNull($this->RequestHandler->beforeRedirect($event, null, $this->Controller->response)); }); From 4f174c11c9db24afb05d9065cc2a604fdb92b00e Mon Sep 17 00:00:00 2001 From: mscherer Date: Thu, 27 Sep 2018 11:41:20 +0200 Subject: [PATCH 7/9] Use setter on Controller. --- .../Controller/Component/RequestHandlerComponentTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php b/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php index 6c8f945c976..f9fc94d9fb1 100644 --- a/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php +++ b/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php @@ -530,7 +530,7 @@ public function testNoViewClassExtension() public function testUnrecognizedExtensionFailure() { Router::extensions(['json', 'foo'], false); - $this->Controller->request = $this->Controller->request->withParam('_ext', 'foo'); + $this->Controller->setRequest($this->Controller->request->withParam('_ext', 'foo')); $event = new Event('Controller.startup', $this->Controller); $this->RequestHandler->startup($event); $this->Controller->getEventManager()->on('Controller.beforeRender', function () { From 5e355eee0215c25ce0f57af534c149578459aaac Mon Sep 17 00:00:00 2001 From: ADmad Date: Fri, 28 Sep 2018 18:41:23 +0530 Subject: [PATCH 8/9] Use getter methods instead of properties. --- .../Controller/Component/RequestHandlerComponentTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php b/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php index f9fc94d9fb1..31e170a529d 100644 --- a/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php +++ b/tests/TestCase/Controller/Component/RequestHandlerComponentTest.php @@ -530,11 +530,11 @@ public function testNoViewClassExtension() public function testUnrecognizedExtensionFailure() { Router::extensions(['json', 'foo'], false); - $this->Controller->setRequest($this->Controller->request->withParam('_ext', 'foo')); + $this->Controller->setRequest($this->Controller->getRequest()->withParam('_ext', 'foo')); $event = new Event('Controller.startup', $this->Controller); $this->RequestHandler->startup($event); $this->Controller->getEventManager()->on('Controller.beforeRender', function () { - return $this->Controller->response; + return $this->Controller->getResponse(); }); $this->Controller->render(); $this->assertEquals('RequestHandlerTest' . DS . 'csv', $this->Controller->viewBuilder()->getTemplatePath()); From 6d726045c5802d1d00babcc620736af5c46f457c Mon Sep 17 00:00:00 2001 From: mscherer Date: Mon, 1 Oct 2018 11:27:40 +0200 Subject: [PATCH 9/9] Fix up comment example. --- src/Http/Response.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Response.php b/src/Http/Response.php index 857a449d836..a5a3f6d52ed 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -1140,7 +1140,7 @@ public function type($contentType = null) /** * Sets a content type definition into the map. * - * E.g.: setType('xhtml' => ['application/xhtml+xml', 'application/xhtml']) + * E.g.: setTypeMap('xhtml', ['application/xhtml+xml', 'application/xhtml']) * * This is needed for RequestHandlerComponent and recognition of types. *