diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 9607926cb5..6548c95e66 100644 --- a/framework/CHANGELOG.md +++ b/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 diff --git a/framework/web/Controller.php b/framework/web/Controller.php index 4e205d00eb..08a15e8ed7 100644 --- a/framework/web/Controller.php +++ b/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 && - ($type = $param->getType()) !== null && - $type->isBuiltin() && - ($params[$name] !== null || !$type->allowsNull()) + PHP_VERSION_ID >= 70000 + && ($type = $param->getType()) !== null + && $type->isBuiltin() + && ($params[$name] !== null || !$type->allowsNull()) ) { $typeName = PHP_VERSION_ID >= 70100 ? $type->getName() : (string)$type; - switch ($typeName) { - case 'int': - $params[$name] = filter_var($params[$name], FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE); - break; - case 'float': - $params[$name] = filter_var($params[$name], FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE); - break; - case 'bool': - $params[$name] = filter_var($params[$name], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); - break; - } - if ($params[$name] === null) { - $isValid = false; + + if ($params[$name] === '' && $type->allowsNull()) { + if ($typeName !== 'string') { // for old string behavior compatibility + $params[$name] = null; + } + } else { + switch ($typeName) { + case 'int': + $params[$name] = filter_var($params[$name], FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE); + break; + case 'float': + $params[$name] = filter_var($params[$name], FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE); + 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}".', [ - 'param' => $name, - ])); + throw new BadRequestHttpException( + 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}', [ - 'params' => implode(', ', $missing), - ])); + throw new BadRequestHttpException( + 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) { - \Yii::$app->requestedParams = array_merge($actionParams, $requestedParams); + if (Yii::$app->requestedParams === null) { + Yii::$app->requestedParams = array_merge($actionParams, $requestedParams); } return $args; diff --git a/tests/framework/web/ControllerTest.php b/tests/framework/web/ControllerTest.php index 1ddc9037fa..f1f85e9f5f 100644 --- a/tests/framework/web/ControllerTest.php +++ b/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('Component: yii\web\Request $request', \Yii::$app->requestedParams['request']); + $this->assertEquals(Yii::$app->request, $args[1]); + $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; - } } diff --git a/tests/framework/web/FakePhp7Controller.php b/tests/framework/web/FakePhp7Controller.php index 68a3517078..5b3ae0b826 100644 --- a/tests/framework/web/FakePhp7Controller.php +++ b/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) + { + } }