diff --git a/src/CLI/CLI.php b/src/CLI/CLI.php index 0611d29..e4c96e2 100644 --- a/src/CLI/CLI.php +++ b/src/CLI/CLI.php @@ -7,6 +7,8 @@ use Utopia\DI\Container; use Utopia\Servers\Hook; use Utopia\Validator; +use Utopia\Validator\Boolean; +use Utopia\Validator\Nullable; class CLI { @@ -302,7 +304,7 @@ protected function getParams(Hook $hook): array $this->validate($key, $param, $value); - $params[$this->camelCaseIt($key)] = $value; + $params[$this->camelCaseIt($key)] = $this->coerce($param['validator'], $value); } foreach ($hook->getDependencies() as $dependency) { @@ -410,6 +412,50 @@ protected function validate(string $key, array $param, $value): void } } + /** + * Coerce string CLI inputs to native PHP types based on the param's validator. + * + * CLI args arrive as strings via getopt. When the validator is `Boolean` + * (loose mode), strings like "true"/"false"/"1"/"0" pass validation but + * remain strings. If the action callback declares a `bool` parameter, PHP's + * implicit string-to-bool cast turns any non-empty string except "0" into + * `true` -- so `--commit=false` silently becomes `true`. Validators in + * utopia-php are pure (validate only, never mutate), so the coercion has + * to happen here at the dispatch boundary. + * + * Only string inputs are coerced; bool defaults are passed through + * untouched. Strings that `filter_var` doesn't recognise (including the + * empty string, which bypasses `validate()` for optional params) are + * passed through unchanged so callers that use `''` as a sentinel for + * "not set" keep working. + * + * @param Validator|callable $validator + * @param mixed $value + * @return mixed + */ + protected function coerce(Validator|callable $validator, mixed $value): mixed + { + if (! is_string($value) || $value === '') { + return $value; + } + + if (\is_callable($validator)) { + $validator = $validator(); + } + + while ($validator instanceof Nullable) { + $validator = $validator->getValidator(); + } + + if (! ($validator instanceof Boolean)) { + return $value; + } + + $coerced = \filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + + return $coerced === null ? $value : $coerced; + } + public function setContainer(Container $container): self { $this->parentContainer = $container; diff --git a/tests/CLI/CLITest.php b/tests/CLI/CLITest.php index 078536f..3abb6d9 100755 --- a/tests/CLI/CLITest.php +++ b/tests/CLI/CLITest.php @@ -7,6 +7,8 @@ use Utopia\CLI\CLI; use Utopia\DI\Container; use Utopia\Validator\ArrayList; +use Utopia\Validator\Boolean; +use Utopia\Validator\Nullable; use Utopia\Validator\Text; class CLITest extends TestCase @@ -289,6 +291,121 @@ public function testMatch() $this->assertEquals(null, $cli->match()); } + /** + * @return iterable + */ + public static function looseBooleanValuesProvider(): iterable + { + yield '"false" string' => ['false', false]; + yield '"true" string' => ['true', true]; + yield '"0" string' => ['0', false]; + yield '"1" string' => ['1', true]; + } + + /** + * Regression: --flag=false used to arrive as the literal string "false", + * which PHP's implicit string-to-bool cast turned into `true` at the + * `bool $flag` parameter boundary. The CLI dispatcher now coerces string + * inputs whose validator is `Boolean` to a real PHP bool. + * + * @dataProvider looseBooleanValuesProvider + */ + public function testBooleanParamCoercesStringInput(string $input, bool $expected): void + { + $captured = null; + + $cli = new CLI(new Generic(), ['test.php', 'build', '--commit='.$input]); + + $cli + ->task('build') + ->param('commit', false, new Boolean(true), 'Commit changes', true) + ->action(function (bool $commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertSame($expected, $captured); + } + + public function testBooleanParamUsesDefaultWhenOmitted(): void + { + $captured = null; + + $cli = new CLI(new Generic(), ['test.php', 'build']); + + $cli + ->task('build') + ->param('commit', false, new Boolean(true), 'Commit changes', true) + ->action(function (bool $commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertFalse($captured); + } + + public function testBooleanParamCoercionUnwrapsNullable(): void + { + $captured = 'untouched'; + + $cli = new CLI(new Generic(), ['test.php', 'build', '--commit=false']); + + $cli + ->task('build') + ->param('commit', null, new Nullable(new Boolean(true)), 'Commit changes', true) + ->action(function (bool $commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertFalse($captured); + } + + /** + * Empty-string params bypass `validate()` when optional, so they reach + * `coerce()` un-validated. We must NOT silently turn them into `false` + * (callers like Cloud's `Patch.php` use `''` as a "not set" sentinel and + * later resolve it to `null`/three-state). + */ + public function testBooleanParamPreservesEmptyStringSentinel(): void + { + $captured = 'untouched'; + + $cli = new CLI(new Generic(), ['test.php', 'build']); + + $cli + ->task('build') + ->param('commit', '', new Boolean(true), 'Commit changes', true) + ->action(function ($commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertSame('', $captured); + } + + public function testNonBooleanValidatorPassesValueThroughUnchanged(): void + { + $captured = null; + + $cli = new CLI(new Generic(), ['test.php', 'build', '--name=false']); + + $cli + ->task('build') + ->param('name', '', new Text(64), 'A name') + ->action(function (string $name) use (&$captured) { + $captured = $name; + }); + + $cli->run(); + + $this->assertSame('false', $captured); + } + public function testEscaping() { ob_start();