From 749f2b99d9426e1a548e356aba90f9e008b5d152 Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 13 Apr 2014 06:48:51 -0400 Subject: [PATCH 1/2] Don't 404 extensions that could be handled by routing. Fixes an error in #2750 where routed extensions would always return 404's for plugin requests. When a file extenion could be handled by router, AssetDispatcher cannot 404 the request. Refs #3305 --- lib/Cake/Routing/Filter/AssetDispatcher.php | 11 +++++-- .../Routing/Filter/AssetDispatcherTest.php | 32 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Routing/Filter/AssetDispatcher.php b/lib/Cake/Routing/Filter/AssetDispatcher.php index 0c91c3cd7..e76f8fde5 100644 --- a/lib/Cake/Routing/Filter/AssetDispatcher.php +++ b/lib/Cake/Routing/Filter/AssetDispatcher.php @@ -55,10 +55,17 @@ class AssetDispatcher extends DispatcherFilter { return null; } + $pathSegments = explode('.', $url); + $ext = array_pop($pathSegments); + $isFile = file_exists($assetFile); + if (in_array($ext, Router::extensions()) && !$isFile) { + return null; + } + $response = $event->data['response']; $event->stopPropagation(); - if (!file_exists($assetFile)) { + if (!$isFile) { $response->statusCode(404); $response->send(); return $response; @@ -69,8 +76,6 @@ class AssetDispatcher extends DispatcherFilter { return $response; } - $pathSegments = explode('.', $url); - $ext = array_pop($pathSegments); $this->_deliverAsset($response, $assetFile, $ext); return $response; } diff --git a/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php b/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php index 35c261cd8..86df36deb 100644 --- a/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php +++ b/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php @@ -83,6 +83,38 @@ class AssetDispatcherTest extends CakeTestCase { $this->assertFalse($event->isStopped()); } +/** + * AssetDispatcher should not 404 extensions that could be handled + * by Routing. + * + * @return void + */ + public function testNoHandleRoutedExtension() { + $filter = new AssetDispatcher(); + $response = $this->getMock('CakeResponse', array('_sendHeader')); + Configure::write('Asset.filter', array( + 'js' => '', + 'css' => '' + )); + App::build(array( + 'Plugin' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS), + 'View' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS) + ), App::RESET); + Router::parseExtensions('json'); + Router::connect('/test_plugin/api/v1/:action', array('controller' => 'api')); + CakePlugin::load('TestPlugin'); + + $request = new CakeRequest('test_plugin/api/v1/forwarding.json'); + $event = new CakeEvent('DispatcherTest', $this, compact('request', 'response')); + $this->assertNull($filter->beforeDispatch($event)); + $this->assertFalse($event->isStopped(), 'Events for routed extensions should not be stopped'); + + $request = new CakeRequest('test_plugin/api/v1/forwarding.png'); + $event = new CakeEvent('DispatcherTest', $this, compact('request', 'response')); + $this->assertSame($response, $filter->beforeDispatch($event)); + $this->assertTrue($event->isStopped()); + } + /** * Tests that $response->checkNotModified() is called and bypasses * file dispatching From f1b57d14ab20d779727f7c021ce1d1c18667823d Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 13 Apr 2014 20:00:34 -0400 Subject: [PATCH 2/2] Revert changed added in #2750. While the had the potential to make 404s going through AssetDispatcher much faster, they broke plugins + extension routing. While explicit extensions could be fixed, routing all extensions could not. Because we are trying to keep 2.x as API compatible as possible it makes sense to revert the previous changes. --- lib/Cake/Routing/Filter/AssetDispatcher.php | 19 ++++------------ .../Routing/Filter/AssetDispatcherTest.php | 22 ------------------- 2 files changed, 4 insertions(+), 37 deletions(-) diff --git a/lib/Cake/Routing/Filter/AssetDispatcher.php b/lib/Cake/Routing/Filter/AssetDispatcher.php index e76f8fde5..5e545146e 100644 --- a/lib/Cake/Routing/Filter/AssetDispatcher.php +++ b/lib/Cake/Routing/Filter/AssetDispatcher.php @@ -51,31 +51,20 @@ class AssetDispatcher extends DispatcherFilter { } $assetFile = $this->_getAssetFile($url); - if ($assetFile === null) { + if ($assetFile === null || !file_exists($assetFile)) { return null; } - - $pathSegments = explode('.', $url); - $ext = array_pop($pathSegments); - $isFile = file_exists($assetFile); - if (in_array($ext, Router::extensions()) && !$isFile) { - return null; - } - $response = $event->data['response']; $event->stopPropagation(); - if (!$isFile) { - $response->statusCode(404); - $response->send(); - return $response; - } - $response->modified(filemtime($assetFile)); if ($response->checkNotModified($event->data['request'])) { return $response; } + $pathSegments = explode('.', $url); + $ext = array_pop($pathSegments); + $this->_deliverAsset($response, $assetFile, $ext); return $response; } diff --git a/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php b/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php index 86df36deb..dc43fda46 100644 --- a/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php +++ b/lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php @@ -108,11 +108,6 @@ class AssetDispatcherTest extends CakeTestCase { $event = new CakeEvent('DispatcherTest', $this, compact('request', 'response')); $this->assertNull($filter->beforeDispatch($event)); $this->assertFalse($event->isStopped(), 'Events for routed extensions should not be stopped'); - - $request = new CakeRequest('test_plugin/api/v1/forwarding.png'); - $event = new CakeEvent('DispatcherTest', $this, compact('request', 'response')); - $this->assertSame($response, $filter->beforeDispatch($event)); - $this->assertTrue($event->isStopped()); } /** @@ -161,23 +156,6 @@ class AssetDispatcherTest extends CakeTestCase { $this->assertEquals($time->format('D, j M Y H:i:s') . ' GMT', $response->modified()); } -/** - * Test 404 status code is set on missing asset. - * - * @return void - */ - public function test404OnMissingFile() { - $filter = new AssetDispatcher(); - - $response = $this->getMock('CakeResponse', array('_sendHeader')); - $request = new CakeRequest('/theme/test_theme/img/nope.gif'); - $event = new CakeEvent('Dispatcher.beforeRequest', $this, compact('request', 'response')); - - $response = $filter->beforeDispatch($event); - $this->assertTrue($event->isStopped()); - $this->assertEquals(404, $response->statusCode()); - } - /** * Test that no exceptions are thrown for //index.php type URLs. *