Fix #18450: Allow empty string to be passed as a nullable typed argument to a controller's action

framework/CHANGELOG.md

@@ -23,6 +23,7 @@ Yii Framework 2 Change Log
 - Bug #18508: Fix Postgres SQL query for load table indexes with correct column order (insolita)
 - Enh #18518: Add support for ngrok’s `X-Original-Host` header (brandonkelly)
 - Bug #18529: Fix asset files path with `appendTimestamp` option for non-root-relative base URLs (bizley)
+- Bug #18450: Allow empty string to be passed as a nullable typed argument to a controller's action (dicrtarasov, bizley)
 
 
 2.0.40 December 23, 2020

framework/web/Controller.php

@@ -142,31 +142,38 @@ class Controller extends \yii\base\Controller
                 } elseif (is_array($params[$name])) {
                     $isValid = false;
                 } elseif (
-                    PHP_VERSION_ID >= 70000 &&
+                    PHP_VERSION_ID >= 70000
-                    ($type = $param->getType()) !== null &&
+                    && ($type = $param->getType()) !== null
-                    $type->isBuiltin() &&
+                    && $type->isBuiltin()
-                    ($params[$name] !== null || !$type->allowsNull())
+                    && ($params[$name] !== null || !$type->allowsNull())
                 ) {
                     $typeName = PHP_VERSION_ID >= 70100 ? $type->getName() : (string)$type;
-                    switch ($typeName) {
+
-                        case 'int':
+                    if ($params[$name] === '' && $type->allowsNull()) {
-                            $params[$name] = filter_var($params[$name], FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE);
+                        if ($typeName !== 'string') { // for old string behavior compatibility
-                            break;
+                            $params[$name] = null;
-                        case 'float':
+                        }
-                            $params[$name] = filter_var($params[$name], FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE);
+                    } else {
-                            break;
+                        switch ($typeName) {
-                        case 'bool':
+                            case 'int':
-                            $params[$name] = filter_var($params[$name], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
+                                $params[$name] = filter_var($params[$name], FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE);
-                            break;
+                                break;
-                    }
+                            case 'float':
-                    if ($params[$name] === null) {
+                                $params[$name] = filter_var($params[$name], FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE);
-                        $isValid = false;
+                                break;
+                            case 'bool':
+                                $params[$name] = filter_var($params[$name], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
+                                break;
+                        }
+                        if ($params[$name] === null) {
+                            $isValid = false;
+                        }
                     }
                 }
                 if (!$isValid) {
-                    throw new BadRequestHttpException(Yii::t('yii', 'Invalid data received for parameter "{param}".', [
+                    throw new BadRequestHttpException(
-                        'param' => $name,
+                        Yii::t('yii', 'Invalid data received for parameter "{param}".', ['param' => $name])
-                    ]));
+                    );
                 }
                 $args[] = $actionParams[$name] = $params[$name];
                 unset($params[$name]);
@@ -184,16 +191,16 @@ class Controller extends \yii\base\Controller
         }
 
         if (!empty($missing)) {
-            throw new BadRequestHttpException(Yii::t('yii', 'Missing required parameters: {params}', [
+            throw new BadRequestHttpException(
-                'params' => implode(', ', $missing),
+                Yii::t('yii', 'Missing required parameters: {params}', ['params' => implode(', ', $missing)])
-            ]));
+            );
         }
 
         $this->actionParams = $actionParams;
 
         // We use a different array here, specifically one that doesn't contain service instances but descriptions instead.
-        if (\Yii::$app->requestedParams === null) {
+        if (Yii::$app->requestedParams === null) {
-            \Yii::$app->requestedParams = array_merge($actionParams, $requestedParams);
+            Yii::$app->requestedParams = array_merge($actionParams, $requestedParams);
         }
 
         return $args;

tests/framework/web/ControllerTest.php

@@ -22,19 +22,40 @@ class ControllerTest extends TestCase
 {
     /** @var FakeController */
     private $controller;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->mockWebApplication();
+        $this->controller = new FakeController('fake', new \yii\web\Application([
+            'id' => 'app',
+            'basePath' => __DIR__,
+            'components' => [
+                'request' => [
+                    'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq',
+                    'scriptFile' => __DIR__ . '/index.php',
+                    'scriptUrl' => '/index.php',
+                ],
+            ],
+        ]));
+
+        Yii::$app->controller = $this->controller;
+    }
+
     public function testBindActionParams()
     {
         $aksi1 = new InlineAction('aksi1', $this->controller, 'actionAksi1');
 
-        $params = ['fromGet' => 'from query params', 'q' => 'd426', 'validator' => 'avaliable'];
+        $params = ['fromGet' => 'from query params', 'q' => 'd426', 'validator' => 'available'];
         list($fromGet, $other) = $this->controller->bindActionParams($aksi1, $params);
         $this->assertEquals('from query params', $fromGet);
         $this->assertEquals('default', $other);
 
-        $params = ['fromGet' => 'from query params', 'q' => 'd426', 'other' => 'avaliable'];
+        $params = ['fromGet' => 'from query params', 'q' => 'd426', 'other' => 'available'];
         list($fromGet, $other) = $this->controller->bindActionParams($aksi1, $params);
         $this->assertEquals('from query params', $fromGet);
-        $this->assertEquals('avaliable', $other);
+        $this->assertEquals('available', $other);
     }
 
     public function testNullableInjectedActionParams()
@@ -62,7 +83,7 @@ class ControllerTest extends TestCase
         $injectionAction = new InlineAction('injection', $this->controller, 'actionNullableInjection');
         $params = [];
         $args = $this->controller->bindActionParams($injectionAction, $params);
-        $this->assertEquals(\Yii::$app->request, $args[0]);
+        $this->assertEquals(Yii::$app->request, $args[0]);
         $this->assertNull($args[1]);
     }
 
@@ -89,7 +110,7 @@ class ControllerTest extends TestCase
 
         $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection');
         $params = ['between' => 'test', 'after' => 'another', 'before' => 'test'];
-        \Yii::$container->set(VendorImage::className(), function() { throw new \RuntimeException('uh oh'); });
+        Yii::$container->set(VendorImage::className(), function() { throw new \RuntimeException('uh oh'); });
 
         $this->expectException(get_class(new RuntimeException()));
         $this->expectExceptionMessage('uh oh');
@@ -106,7 +127,6 @@ class ControllerTest extends TestCase
         $this->controller = new FakePhp71Controller('fake', new \yii\web\Application([
             'id' => 'app',
             'basePath' => __DIR__,
-
             'components' => [
                 'request' => [
                     'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq',
@@ -119,7 +139,7 @@ class ControllerTest extends TestCase
 
         $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection');
         $params = ['between' => 'test', 'after' => 'another', 'before' => 'test'];
-        \Yii::$container->clear(VendorImage::className());
+        Yii::$container->clear(VendorImage::className());
         $this->expectException(get_class(new ServerErrorHttpException()));
         $this->expectExceptionMessage('Could not load required service: vendorImage');
         $this->controller->bindActionParams($injectionAction, $params);
@@ -135,7 +155,6 @@ class ControllerTest extends TestCase
         $this->controller = new FakePhp71Controller('fake', new \yii\web\Application([
             'id' => 'app',
             'basePath' => __DIR__,
-
             'components' => [
                 'request' => [
                     'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq',
@@ -148,16 +167,16 @@ class ControllerTest extends TestCase
 
         $injectionAction = new InlineAction('injection', $this->controller, 'actionInjection');
         $params = ['between' => 'test', 'after' => 'another', 'before' => 'test'];
-        \Yii::$container->set(VendorImage::className(), VendorImage::className());
+        Yii::$container->set(VendorImage::className(), VendorImage::className());
         $args = $this->controller->bindActionParams($injectionAction, $params);
         $this->assertEquals($params['before'], $args[0]);
-        $this->assertEquals(\Yii::$app->request, $args[1]);
+        $this->assertEquals(Yii::$app->request, $args[1]);
-        $this->assertEquals('Component: yii\web\Request $request', \Yii::$app->requestedParams['request']);
+        $this->assertEquals('Component: yii\web\Request $request', Yii::$app->requestedParams['request']);
         $this->assertEquals($params['between'], $args[2]);
         $this->assertInstanceOf(VendorImage::className(), $args[3]);
-        $this->assertEquals('Container DI: yiiunit\framework\web\stubs\VendorImage $vendorImage', \Yii::$app->requestedParams['vendorImage']);
+        $this->assertEquals('Container DI: yiiunit\framework\web\stubs\VendorImage $vendorImage', Yii::$app->requestedParams['vendorImage']);
         $this->assertNull($args[4]);
-        $this->assertEquals('Unavailable service: post', \Yii::$app->requestedParams['post']);
+        $this->assertEquals('Unavailable service: post', Yii::$app->requestedParams['post']);
         $this->assertEquals($params['after'], $args[5]);
     }
 
@@ -170,7 +189,6 @@ class ControllerTest extends TestCase
         $module = new \yii\base\Module('fake', new \yii\web\Application([
             'id' => 'app',
             'basePath' => __DIR__,
-
             'components' => [
                 'request' => [
                     'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq',
@@ -189,7 +207,7 @@ class ControllerTest extends TestCase
         $injectionAction = new InlineAction('injection', $this->controller, 'actionModuleServiceInjection');
         $args = $this->controller->bindActionParams($injectionAction, []);
         $this->assertInstanceOf(\yii\data\ArrayDataProvider::className(), $args[0]);
-        $this->assertEquals('Module yii\base\Module DI: yii\data\DataProviderInterface $dataProvider', \Yii::$app->requestedParams['dataProvider']);
+        $this->assertEquals('Module yii\base\Module DI: yii\data\DataProviderInterface $dataProvider', Yii::$app->requestedParams['dataProvider']);
     }
 
     /**
@@ -206,7 +224,6 @@ class ControllerTest extends TestCase
         $this->controller = new FakePhp7Controller('fake', new \yii\web\Application([
             'id' => 'app',
             'basePath' => __DIR__,
-
             'components' => [
                 'request' => [
                     'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq',
@@ -226,6 +243,17 @@ class ControllerTest extends TestCase
         $this->assertSame(true, $true);
         $this->assertSame(false, $false);
 
+        // allow nullable argument to be set to empty string (as null)
+        // https://github.com/yiisoft/yii2/issues/18450
+        $params = ['foo' => 100, 'bar' => '', 'true' => true, 'false' => true];
+        list(, $bar) = $this->controller->bindActionParams($aksi1, $params);
+        $this->assertSame(null, $bar);
+
+        // make sure nullable string argument is not set to null when empty string is passed
+        $stringy = new InlineAction('stringy', $this->controller, 'actionStringy');
+        list($foo) = $this->controller->bindActionParams($stringy, ['foo' => '']);
+        $this->assertSame('', $foo);
+
         $params = ['foo' => 'oops', 'bar' => null];
         $this->expectException('yii\web\BadRequestHttpException');
         $this->expectExceptionMessage('Invalid data received for parameter "foo".');
@@ -274,23 +302,4 @@ class ControllerTest extends TestCase
         $this->assertEquals($this->controller->redirect(['//controller/index', 'id_1' => 3, 'id_2' => 4])->headers->get('location'), '/index.php?r=controller%2Findex&id_1=3&id_2=4');
         $this->assertEquals($this->controller->redirect(['//controller/index', 'slug' => 'äöüß!"§$%&/()'])->headers->get('location'), '/index.php?r=controller%2Findex&slug=%C3%A4%C3%B6%C3%BC%C3%9F%21%22%C2%A7%24%25%26%2F%28%29');
     }
-
-    protected function setUp()
-    {
-        parent::setUp();
-        $this->mockWebApplication();
-        $this->controller = new FakeController('fake', new \yii\web\Application([
-            'id' => 'app',
-            'basePath' => __DIR__,
-
-            'components' => [
-                'request' => [
-                    'cookieValidationKey' => 'wefJDF8sfdsfSDefwqdxj9oq',
-                    'scriptFile' => __DIR__ . '/index.php',
-                    'scriptUrl' => '/index.php',
-                ],
-            ],
-        ]));
-        Yii::$app->controller = $this->controller;
-    }
 }

tests/framework/web/FakePhp7Controller.php

@@ -20,4 +20,8 @@ class FakePhp7Controller extends Controller
     public function actionAksi1(int $foo, float $bar = null, bool $true, bool $false)
     {
     }
+
+    public function actionStringy(string $foo = null)
+    {
+    }
 }