Merge pull request #5114 from cakephp/issue-5089

Fix routing extensions leaking out of their scopes.

src/Routing/RouteBuilder.php

@@ -127,9 +127,10 @@ class RouteBuilder {
 	}
 
 /**
- * Get or set the extensions in this route collection.
+ * Get or set the extensions in this route builder's scope.
  *
- * Setting extensions does not modify existing routes.
+ * Future routes connected in through this builder will have the connected
+ * extensions applied. However, setting extensions does not modify existing routes.
  *
  * @param null|string|array $extensions Either the extensions to use or null.
  * @return array|void

src/Routing/Router.php

@@ -148,6 +148,13 @@ class Router {
 	protected static $_urlFilters = [];
 
 /**
+ * Default extensions defined with Router::extensions()
+ *
+ * @var array
+ */
+	protected static $_defaultExtensions = [];
+
+/**
  * Get or set default route class.
  *
  * @param string|null $routeClass Class name.
@@ -724,7 +731,9 @@ class Router {
 	}
 
 /**
- * Get/Set valid extensions. Instructs the router to parse out file extensions
+ * Get or set valid extensions for all routes connected later.
+ *
+ * Instructs the router to parse out file extensions
  * from the URL. For example, http://example.com/posts.rss would yield a file
  * extension of "rss". The file extension itself is made available in the
  * controller as `$this->request->params['_ext']`, and is used by the RequestHandler
@@ -747,10 +756,13 @@ class Router {
 			if (!static::$initialized) {
 				static::_loadRoutes();
 			}
-			return $collection->extensions();
+			return array_unique(array_merge(static::$_defaultExtensions, $collection->extensions()));
 		}
-
-		return $collection->extensions($extensions, $merge);
+		$extensions = (array)$extensions;
+		if ($merge) {
+			$extensions = array_unique(array_merge(static::$_defaultExtensions, $extensions));
+		}
+		return static::$_defaultExtensions = $extensions;
 	}
 
 /**
@@ -845,7 +857,7 @@ class Router {
 	public static function scope($path, $params = [], $callback = null) {
 		$builder = new RouteBuilder(static::$_collection, '/', [], [
 			'routeClass' => static::defaultRouteClass(),
-			'extensions' => static::$_collection->extensions()
+			'extensions' => static::$_defaultExtensions
 		]);
 		$builder->scope($path, $params, $callback);
 	}

tests/TestCase/Routing/RouterTest.php

@@ -1582,6 +1582,8 @@ class RouterTest extends TestCase {
  * @return void
  */
 	public function testExtensionsWithScopedRoutes() {
+		Router::extensions(['json']);
+
 		Router::scope('/', function ($routes) {
 			$routes->extensions('rss');
 			$routes->connect('/', ['controller' => 'Pages', 'action' => 'index']);
@@ -1592,7 +1594,7 @@ class RouterTest extends TestCase {
 			});
 		});
 
-		$this->assertEquals(['rss', 'xml', 'json'], Router::extensions());
+		$this->assertEquals(['json', 'rss', 'xml'], array_values(Router::extensions()));
 	}
 
 /**
@@ -2645,6 +2647,42 @@ class RouterTest extends TestCase {
 	}
 
 /**
+ * Test to ensure that extensions defined in scopes don't leak.
+ * And that global extensions are propagated.
+ *
+ * @return void
+ */
+	public function testScopeExtensionsContained() {
+		Router::extensions(['json']);
+		Router::scope('/', function ($routes) {
+			$this->assertEquals(['json'], $routes->extensions(), 'Should default to global extensions.');
+			$routes->extensions(['rss']);
+
+			$this->assertEquals(
+				['rss'],
+				$routes->extensions(),
+				'Should include new extensions.'
+			);
+			$routes->connect('/home', []);
+		});
+
+		$this->assertEquals(['json', 'rss'], array_values(Router::extensions()));
+
+		Router::scope('/api', function ($routes) {
+			$this->assertEquals(['json'], $routes->extensions(), 'Should default to global extensions.');
+
+			$routes->extensions(['json', 'csv']);
+			$routes->connect('/export', []);
+
+			$routes->scope('/v1', function ($routes) {
+				$this->assertEquals(['json', 'csv'], $routes->extensions());
+			});
+		});
+
+		$this->assertEquals(['json', 'rss', 'csv'], array_values(Router::extensions()));
+	}
+
+/**
  * Test that prefix() creates a scope.
  *
  * @return void