fixes greedy dots in extensions parsing

src/Routing/Route/Route.php

@@ -101,19 +101,18 @@ class Route
     {
         $this->template = $template;
         $this->defaults = (array)$defaults;
-        $this->options = $options;
         if (isset($this->defaults['[method]'])) {
             $this->defaults['_method'] = $this->defaults['[method]'];
             unset($this->defaults['[method]']);
         }
-        if (isset($this->options['_ext'])) {
-            $this->_extensions = (array)$this->options['_ext'];
-        }
+        $this->options = $options + ['_ext' => []];
+        $this->setExtensions((array)$this->options['_ext']);
     }
 
     /**
      * Get/Set the supported extensions for this route.
      *
+     * @deprecated 3.3.9 Use getExtensions/setExtensions instead.
      * @param null|string|array $extensions The extensions to set. Use null to get.
      * @return array|null The extensions or null.
      */
@@ -126,6 +125,29 @@ class Route
     }
 
     /**
+     * Set the supported extensions for this route.
+     *
+     * @param array $extensions The extensions to set.
+     * @return $this
+     */
+    public function setExtensions(array $extensions)
+    {
+        $this->_extensions = array_map('strtolower', $extensions);
+
+        return $this;
+    }
+
+    /**
+     * Get the supported extensions for this route.
+     *
+     * @return array
+     */
+    public function getExtensions()
+    {
+        return $this->_extensions;
+    }
+
+    /**
      * Check if a Route has been compiled into a regular expression.
      *
      * @return bool
@@ -346,20 +368,12 @@ class Route
      */
     protected function _parseExtension($url)
     {
-        if (empty($this->_extensions)) {
-            return [$url, null];
-        }
-        preg_match('/\.([0-9a-z\.]*)$/', $url, $match);
-        if (empty($match[1])) {
-            return [$url, null];
-        }
-        $ext = strtolower($match[1]);
-        $len = strlen($match[1]);
-        foreach ($this->_extensions as $name) {
-            if (strtolower($name) === $ext) {
-                $url = substr($url, 0, ($len + 1) * -1);
-
-                return [$url, $ext];
+        if (count($this->_extensions) && strpos($url, '.')) {
+            foreach ($this->_extensions as $ext) {
+                $len = strlen($ext) + 1;
+                if (substr($url, -$len) === '.' . $ext) {
+                    return [substr($url, 0, $len * -1), $ext];
+                }
             }
         }
 

src/Routing/RouteCollection.php

@@ -104,8 +104,8 @@ class RouteCollection
         }
         $this->_paths[$path][] = $route;
 
-        $extensions = $route->extensions();
-        if ($extensions) {
+        $extensions = $route->getExtensions();
+        if (count($extensions) > 0) {
             $this->extensions($extensions);
         }
     }

tests/TestCase/Routing/Route/RouteTest.php

@@ -20,6 +20,29 @@ use Cake\Routing\Route\Route;
 use Cake\TestSuite\TestCase;
 
 /**
+ * Used to expose protected methods for testing.
+ */
+class RouteProtected extends Route
+{
+    /**
+     * @return array
+     */
+    public function peekExtensions()
+    {
+        return $this->_extensions;
+    }
+
+    /**
+     * @param $url
+     * @return array
+     */
+    public function parseExtension($url)
+    {
+        return $this->_parseExtension($url);
+    }
+}
+
+/**
  * Test case for Route
  */
 class RouteTest extends TestCase
@@ -47,12 +70,12 @@ class RouteTest extends TestCase
 
         $this->assertEquals('/:controller/:action/:id', $route->template);
         $this->assertEquals([], $route->defaults);
-        $this->assertEquals(['id' => '[0-9]+'], $route->options);
+        $this->assertEquals(['id' => '[0-9]+', '_ext' => []], $route->options);
         $this->assertFalse($route->compiled());
     }
 
     /**
-     * test Route compiling.
+     * Test Route compiling.
      *
      * @return void
      */
@@ -109,8 +132,7 @@ class RouteTest extends TestCase
         $result = $route->parse('/posts/index.pdf', 'GET');
         $this->assertFalse(isset($result['_ext']));
 
-        $route->extensions(['pdf', 'json', 'xml', 'xml.gz']);
-        $result = $route->parse('/posts/index.pdf', 'GET');
+        $result = $route->setExtensions(['pdf', 'json', 'xml', 'xml.gz'])->parse('/posts/index.pdf', 'GET');
         $this->assertEquals('pdf', $result['_ext']);
 
         $result = $route->parse('/posts/index.json', 'GET');
@@ -124,7 +146,109 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that route parameters that overlap don't cause errors.
+     * @return array
+     */
+    public function provideMatchParseExtension()
+    {
+        return [
+            ['/foo/bar.xml', ['/foo/bar', 'xml'], ['xml', 'json', 'xml.gz']],
+            ['/foo/bar.json', ['/foo/bar', 'json'], ['xml', 'json', 'xml.gz']],
+            ['/foo/bar.xml.gz', ['/foo/bar', 'xml.gz'], ['xml', 'json', 'xml.gz']],
+            ['/foo/with.dots.json.xml.zip', ['/foo/with.dots.json.xml', 'zip'], ['zip']],
+            ['/foo/confusing.extensions.dots.json.xml.zip', ['/foo/confusing.extensions.dots.json.xml', 'zip'], ['json', 'xml', 'zip']],
+            ['/foo/confusing.extensions.dots.json.xml', ['/foo/confusing.extensions.dots.json', 'xml'], ['json', 'xml', 'zip']],
+            ['/foo/confusing.extensions.dots.json', ['/foo/confusing.extensions.dots', 'json'], ['json', 'xml', 'zip']],
+        ];
+    }
+
+    /**
+     * Expects _parseExtension to match extensions in URLs
+     *
+     * @param string $url
+     * @param array $expected
+     * @param array $ext
+     * @return void
+     * @dataProvider provideMatchParseExtension
+     */
+    public function testMatchParseExtension($url, array $expected, array $ext)
+    {
+        $route = new RouteProtected('/:controller/:action/*', [], ['_ext' => $ext]);
+        $result = $route->parseExtension($url);
+        $this->assertEquals($expected, $result);
+    }
+
+    /**
+     * @return array
+     */
+    public function provideNoMatchParseExtension()
+    {
+        return [
+            ['/foo/bar', ['xml']],
+            ['/foo/bar.zip', ['xml']],
+            ['/foo/bar.xml.zip', ['xml']],
+            ['/foo/bar.', ['xml']],
+            ['/foo/bar.xml', []],
+            ['/foo/bar...xml...zip...', ['xml']]
+        ];
+    }
+
+    /**
+     * Expects _parseExtension to not match extensions in URLs
+     *
+     * @param string $url
+     * @param array $ext
+     * @return void
+     * @dataProvider provideNoMatchParseExtension
+     */
+    public function testNoMatchParseExtension($url, array $ext)
+    {
+        $route = new RouteProtected('/:controller/:action/*', [], ['_ext' => $ext]);
+        list($outUrl, $outExt) = $route->parseExtension($url);
+        $this->assertEquals($url, $outUrl);
+        $this->assertNull($outExt);
+    }
+
+    /**
+     * Expects extensions to be set
+     *
+     * @return void
+     */
+    public function testSetExtensions()
+    {
+        $route = new RouteProtected('/:controller/:action/*', []);
+        $this->assertEquals([], $route->peekExtensions());
+        $route->setExtensions(['xml']);
+        $this->assertEquals(['xml'], $route->peekExtensions());
+        $route->setExtensions(['xml', 'json', 'zip']);
+        $this->assertEquals(['xml', 'json', 'zip'], $route->peekExtensions());
+        $route->setExtensions([]);
+        $this->assertEquals([], $route->peekExtensions());
+
+        $route = new RouteProtected('/:controller/:action/*', [], ['_ext' => ['one', 'two']]);
+        $this->assertEquals(['one', 'two'], $route->peekExtensions());
+    }
+
+    /**
+     * Expects extensions to be return.
+     *
+     * @return void
+     */
+    public function testGetExtensions()
+    {
+        $route = new RouteProtected('/:controller/:action/*', []);
+        $this->assertEquals([], $route->getExtensions());
+
+        $route = new RouteProtected('/:controller/:action/*', [], ['_ext' => ['one', 'two']]);
+        $this->assertEquals(['one', 'two'], $route->getExtensions());
+
+        $route = new RouteProtected('/:controller/:action/*', []);
+        $this->assertEquals([], $route->getExtensions());
+        $route->setExtensions(['xml', 'json', 'zip']);
+        $this->assertEquals(['xml', 'json', 'zip'], $route->getExtensions());
+    }
+
+    /**
+     * Test that route parameters that overlap don't cause errors.
      *
      * @return void
      */
@@ -140,7 +264,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test compiling routes with keys that have patterns
+     * Test compiling routes with keys that have patterns
      *
      * @return void
      */
@@ -209,6 +333,11 @@ class RouteTest extends TestCase
         $this->assertEquals(['url_title', 'id'], $route->keys);
     }
 
+    /**
+     * Test route with unicode
+     *
+     * @return void
+     */
     public function testRouteCompilingWithUnicodePatterns()
     {
         $route = new Route(
@@ -229,7 +358,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test more complex route compiling & parsing with mid route greedy stars
+     * Test more complex route compiling & parsing with mid route greedy stars
      * and optional routing parameters
      *
      * @return void
@@ -264,7 +393,7 @@ class RouteTest extends TestCase
         $this->assertRegExp($result, '/some_extra/page/this_is_the_slug');
         $this->assertRegExp($result, '/page/this_is_the_slug');
         $this->assertEquals(['slug', 'extra'], $route->keys);
-        $this->assertEquals(['extra' => '[a-z1-9_]*', 'slug' => '[a-z1-9_]+', 'action' => 'view'], $route->options);
+        $this->assertEquals(['extra' => '[a-z1-9_]*', 'slug' => '[a-z1-9_]+', 'action' => 'view', '_ext' => []], $route->options);
         $expected = [
             'controller' => 'pages',
             'action' => 'view'
@@ -290,7 +419,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that routes match their pattern.
+     * Test that routes match their pattern.
      *
      * @return void
      */
@@ -439,7 +568,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that non-greedy routes fail with extra passed args
+     * Test that non-greedy routes fail with extra passed args
      *
      * @return void
      */
@@ -455,7 +584,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that falsey values do not interrupt a match.
+     * Test that falsey values do not interrupt a match.
      *
      * @return void
      */
@@ -469,7 +598,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test match() with greedy routes, and passed args.
+     * Test match() with greedy routes, and passed args.
      *
      * @return void
      */
@@ -632,7 +761,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that match with patterns works.
+     * Test that match with patterns works.
      *
      * @return void
      */
@@ -779,7 +908,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test numerically indexed defaults, get appended to pass
+     * Test numerically indexed defaults, get appended to pass
      *
      * @return void
      */
@@ -797,7 +926,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that http header conditions can cause route failures.
+     * Test that http header conditions can cause route failures.
      *
      * @return void
      */
@@ -817,7 +946,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that http header conditions can cause route failures.
+     * Test that http header conditions can cause route failures.
      *
      * @return void
      */
@@ -843,7 +972,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that http header conditions can work with URL generation
+     * Test that http header conditions can work with URL generation
      *
      * @return void
      */
@@ -911,7 +1040,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that patterns work for :action
+     * Test that patterns work for :action
      *
      * @return void
      */
@@ -942,7 +1071,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test the parseArgs method
+     * Test the parseArgs method
      *
      * @return void
      */
@@ -997,7 +1126,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test restructuring args with pass key
+     * Test restructuring args with pass key
      *
      * @return void
      */
@@ -1063,7 +1192,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test getName();
+     * Test getName();
      *
      * @return void
      */
@@ -1144,7 +1273,7 @@ class RouteTest extends TestCase
     }
 
     /**
-     * test that utf-8 patterns work for :section
+     * Test that utf-8 patterns work for :section
      *
      * @return void
      */