Fixes yiisoft#14903: Fixed route with extra dashes is executed contro…

…ller while it should not
ihipop · Jan 9, 2018 · a559b9f · a559b9f
1 parent 72b69e3
commit a559b9f
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 5 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -9,6 +9,7 @@ Yii Framework 2 Change Log
 - Bug #14276: Fixed I18N format with dotted parameters (developeruz)
 - Bug #14484: Fixed `yii\validators\UniqueValidator` for target classes with a default scope (laszlovl, developeruz)
 - Bug #14604: Fixed `yii\validators\CompareValidator` `compareAttribute` does not work if `compareAttribute` form ID has been changed (mikk150)
+- Bug #14903: Fixed route with extra dashes is executed controller while it should not (developeruz)
 - Bug #15046: Throw an `yii\web\HeadersAlreadySentException` if headers were sent before web response (dmirogin)
 - Bug #15142: Fixed array params replacing in `yii\helpers\BaseUrl::current()` (IceJOKER)
 - Bug #15169: Fixed translating a string when NULL parameter is passed (developeruz)

diff --git a/framework/base/Module.php b/framework/base/Module.php
@@ -626,14 +626,13 @@ public function createControllerByID($id)
             $className = substr($id, $pos + 1);
         }
 
-        if (!preg_match('%^[a-z][a-z0-9\\-_]*$%', $className)) {
-            return null;
-        }
-        if ($prefix !== '' && !preg_match('%^[a-z0-9_/]+$%i', $prefix)) {
+        if ($this->isIncorrectClassNameOrPrefix($className, $prefix)) {
             return null;
         }
 
-        $className = str_replace(' ', '', ucwords(str_replace('-', ' ', $className))) . 'Controller';
+        $className = preg_replace_callback('%-([a-z0-9_])%i', function ($matches) {
+                return ucfirst($matches[1]);
+            }, ucfirst($className)) . 'Controller';
         $className = ltrim($this->controllerNamespace . '\\' . str_replace('/', '\\', $prefix) . $className, '\\');
         if (strpos($className, '-') !== false || !class_exists($className)) {
             return null;
@@ -649,6 +648,23 @@ public function createControllerByID($id)
         return null;
     }
 
+    /**
+     * @param string $className
+     * @param string $prefix
+     * @return bool
+     */
+    private function isIncorrectClassNameOrPrefix($className, $prefix)
+    {
+        if (!preg_match('%^[a-z][a-z0-9\\-_]*$%', $className)) {
+            return true;
+        }
+        if ($prefix !== '' && !preg_match('%^[a-z0-9_/]+$%i', $prefix)) {
+            return true;
+        }
+
+        return false;
+    }
+
     /**
      * This method is invoked right before an action within this module is executed.
      *

diff --git a/tests/framework/base/ModuleTest.php b/tests/framework/base/ModuleTest.php
@@ -120,6 +120,33 @@ public function testServiceLocatorTraversal()
         $this->assertTrue($child->has('test'));
         $this->assertFalse($parent->has('test'));
     }
+
+    public function testCreateControllerByID()
+    {
+        $module = new TestModule('test');
+        $module->controllerNamespace = 'yiiunit\framework\base';
+
+        $route = 'module-test';
+        $this->assertInstanceOf(ModuleTestController::className(), $module->createControllerByID($route));
+
+        $route = 'module-test-';
+        $this->assertNotInstanceOf(ModuleTestController::className(), $module->createControllerByID($route));
+
+        $route = '-module-test';
+        $this->assertNotInstanceOf(ModuleTestController::className(), $module->createControllerByID($route));
+
+        $route = 'very-complex-name-test';
+        $this->assertInstanceOf(VeryComplexNameTestController::className(), $module->createControllerByID($route));
+
+        $route = 'very-complex-name-test--';
+        $this->assertNotInstanceOf(VeryComplexNameTestController::className(), $module->createControllerByID($route));
+
+        $route = '--very-complex-name-test';
+        $this->assertNotInstanceOf(VeryComplexNameTestController::className(), $module->createControllerByID($route));
+
+        $route = 'very---complex---name---test';
+        $this->assertNotInstanceOf(VeryComplexNameTestController::className(), $module->createControllerByID($route));
+    }
 }
 
 class TestModule extends \yii\base\Module
@@ -141,3 +168,11 @@ public function actionTest2()
         ModuleTest::$actionRuns[] = $this->action->uniqueId;
     }
 }
+
+class VeryComplexNameTestController extends Controller
+{
+    public function actionIndex()
+    {
+        ModuleTest::$actionRuns[] = $this->action->uniqueId;
+    }
+}