diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..e789da75 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "cSpell.words": [ + "undot" + ] +} \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index aa47b2f0..eb156502 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [5.1.1](https://github.com/userfrosting/framework/compare/5.1.0...5.1.1) +- Fix InputArray in Fortress (See https://github.com/userfrosting/UserFrosting/issues/1251) + ## [5.1.0](https://github.com/userfrosting/framework/compare/5.0.0...5.1.0) - Removed Assets - Drop PHP 8.1 support, add PHP 8.3 support diff --git a/src/Fortress/Transformer/RequestDataTransformer.php b/src/Fortress/Transformer/RequestDataTransformer.php index 2871e24c..1e41101a 100644 --- a/src/Fortress/Transformer/RequestDataTransformer.php +++ b/src/Fortress/Transformer/RequestDataTransformer.php @@ -14,6 +14,7 @@ use HTMLPurifier; use HTMLPurifier_Config; +use Illuminate\Support\Arr; use UserFrosting\Fortress\FortressException; use UserFrosting\Fortress\RequestSchema\RequestSchemaInterface; @@ -50,28 +51,30 @@ public function transform( // Get schema fields $schemaFields = $schema->all(); - // 1. Perform sequence of transformations on each field. - $transformedData = []; - foreach ($data as $name => $value) { - // Handle values not listed in the schema. Pass not found to - // transformField if allow is set, transformField will return the value as is. - if (array_key_exists($name, $schemaFields) || $onUnexpectedVar === 'allow') { - $transformedData[$name] = $this->transformField($schema, $name, $value); - } elseif ($onUnexpectedVar === 'error') { - $e = new FortressException("The field '$name' is not a valid input field."); - - throw $e; + // 1. If we skip or error on unexpected var, purge unwanted fields + if ($onUnexpectedVar === 'skip' || $onUnexpectedVar === 'error') { + $data = $this->purge($schemaFields, $data, ($onUnexpectedVar === 'error')); + } + + // 2° Apply each transformation rules. Skip we field as no transformation rules + foreach ($schemaFields as $field => $rules) { + if (!isset($rules['transformations']) || !is_array($rules['transformations'])) { + continue; } + + $data = $this->applyNestedTransformation($rules['transformations'], explode('.', $field), $data); } - // 2. Get default values for any fields missing from $data. Especially useful for checkboxes, etc which are not submitted when they are unchecked + // 3. Get default values for any fields missing from $data. Especially + // useful for checkboxes, etc which are not submitted when they are + // unchecked foreach ($schemaFields as $fieldName => $field) { - if (!isset($transformedData[$fieldName]) && isset($field['default'])) { - $transformedData[$fieldName] = $field['default']; + if (!isset($data[$fieldName]) && isset($field['default'])) { + $data[$fieldName] = $field['default']; } } - return $transformedData; + return $data; } /** @@ -92,29 +95,191 @@ public function transformField(RequestSchemaInterface $schema, string $name, mix return $value; } else { // Field exists in schema, so apply sequence of transformations - $transformedValue = $value; - foreach ($fieldParameters['transformations'] as $transformation) { - $transformedValue = match (strtolower($transformation)) { - 'purify' => $this->purify($transformedValue), - 'escape' => $this->escapeHtmlCharacters($transformedValue), - 'purge' => $this->purgeHtmlCharacters($transformedValue), - 'trim' => $this->trim($transformedValue), - default => $transformedValue, - }; + return $this->applyTransformation($fieldParameters['transformations'], $value); + } + } + + /** + * Apply transformations to a set of nested keys. + * + * @param string[] $rules Rules to apply + * @param string[] $keys Nested keys. Dot notation keys (eg. 'foo.bar') + * represented as an array (eg. array('foo', 'bar')) + * @param mixed $data The data to transform + * + * @return mixed The transformed data + */ + protected function applyNestedTransformation(array $rules, array $keys, mixed $data): mixed + { + $key = array_shift($keys); + + // Parse each element in non-associative array + if ($key === '*' && is_array($data)) { + foreach ($data as $id => $row) { + $data[$id] = $this->applyNestedTransformation($rules, $keys, $row); + } + + return $data; + } + + // Reached the deepest level. Transform the data directly. + if ($key === null) { + return $this->applyTransformation($rules, $data); + } + + // If data don't exist for this key, can't transform what doesn't exist. + if (!isset($data[$key])) { + return $data; + } + + // Reached last key, and data exist, apply transformation to the key. + if (count($keys) === 0) { + $data[$key] = $this->applyTransformation($rules, $data[$key]); + + return $data; + } + + // Dig down another level. + $data[$key] = $this->applyNestedTransformation($rules, $keys, $data[$key]); + + return $data; + } + + /** + * Apply rules to a set of values. + * + * @param string[] $rules The rules to apply + * @param mixed $value The value to transform + * + * @return mixed The transformed value + */ + protected function applyTransformation(array $rules, mixed $value): mixed + { + foreach ($rules as $transformation) { + $value = match (strtolower($transformation)) { + 'purify' => $this->purify($value), + 'escape' => $this->escapeHtmlCharacters($value), + 'purge' => $this->purgeHtmlCharacters($value), + 'trim' => $this->trim($value), + default => $value, + }; + } + + return $value; + } + + /** + * Purge all fields not present the schema fields list. + * + * @param array $schemaFields The fields from the schema to keep. + * @param mixed[] $data The data to purge + * @param bool $throw If true, a FortressException will be thrown if something to purge is found + * + * @throws FortressException If $throw is true and we found something to purge + * @return mixed[] The purged data + */ + protected function purge(array $schemaFields, array $data, bool $throw = false): array + { + // N.B.: The '*' wildcard in the schema fields makes it difficult to + // fetch everything we need to keep. Instead, we use double negation : + // It's easier to remove the one we want to keep, then compare this list + // with the original. Plus It will allow to throw exception with the + // extra field. + + // First, we remove all rules, as we don't need them and don't want to + // dot the nested rules. + $fields = array_flip(array_keys($schemaFields)); + + // Then, we need to remove duplicate and overlaps. For example, `Foo.*` + // and `Foo` overlaps. + $fields = Arr::dot(Arr::undot($fields)); + + // Next, find all data that need to be purged. + $toPurge = $data; + foreach ($fields as $field => $rules) { + $toPurge = $this->purgeParts(explode('.', $field), $toPurge); + } + + // Throw exception if we have fields to purge and onUnexpectedVar + // is set to error, continue otherwise (if it's skip) + if ($throw && count($toPurge) > 0) { + $fields = implode(', ', array_keys(Arr::dot($toPurge))); + + throw new FortressException("The fields '$fields' are not a valid input field."); + } + + // Finally we loop again using the same method to apply the purge + // with the converted '*' wildcard to the original data. However, + // this time we use the field to purge instead of the schema data. + $dotToForget = Arr::dot($toPurge); + foreach ($dotToForget as $field => $value) { + $data = $this->purgeParts(explode('.', $field), $data); + } + + return $data; + } + + /** + * Purge a set of nested keys. + * + * @param string[] $keys Nested keys. Dot notation keys (eg. 'foo.bar') + * represented as an array (eg. array('foo', 'bar')) + * @param mixed[] $data The data to purge from + * + * @return mixed[] The purged data + */ + protected function purgeParts(array $keys, array $data): array + { + $key = array_shift($keys); + + // Parse each element in non-associative array + if ($key === '*') { + foreach ($data as $id => $row) { + // Do we need to do another level deeper? + if (is_array($row)) { + $data[$id] = $this->purgeParts($keys, $row); + + // Delete empty array + if (count($data[$id]) === 0) { + unset($data[$id]); + } + } else { + unset($data[$id]); + } } - return $transformedValue; + return $data; + } + + // If data don't exist for this key, can't transform what doesn't exist. + if ($key === null || !isset($data[$key])) { + return $data; + } + + // Reached the last key, and data exist, remove it. + if (count($keys) === 0) { + unset($data[$key]); + + return $data; + } + + // Last resort, we dig into another level + $data[$key] = $this->purgeParts($keys, $data[$key]); + if (count($data[$key]) === 0) { + unset($data[$key]); } + + return $data; } /** * Autodetect if a field is an array or scalar, and filter appropriately. * - * @param string|string[] $value + * @param mixed $value * - * @return string|string[] + * @return mixed */ - protected function escapeHtmlCharacters(string|array $value): string|array + protected function escapeHtmlCharacters(mixed $value): mixed { if (is_array($value)) { return filter_var_array($value, FILTER_SANITIZE_SPECIAL_CHARS); // @phpstan-ignore-line @@ -126,14 +291,19 @@ protected function escapeHtmlCharacters(string|array $value): string|array /** * Autodetect if a field is an array or scalar, and filter appropriately. * - * @param string|string[] $value + * @param mixed $value * - * @return string|string[] + * @return mixed */ - protected function purgeHtmlCharacters(string|array $value): string|array + protected function purgeHtmlCharacters(mixed $value): mixed { if (is_array($value)) { - return array_map('strip_tags', $value); + return $this->arrayMapRecursive('strip_tags', $value); + } + + // Nothing to purge if it's not a string + if (!is_string($value)) { + return $value; } return strip_tags($value); @@ -142,14 +312,19 @@ protected function purgeHtmlCharacters(string|array $value): string|array /** * Autodetect if a field is an array or scalar, and filter appropriately. * - * @param string|string[] $value + * @param mixed $value * - * @return string|string[] + * @return mixed */ - protected function trim(string|array $value): string|array + protected function trim(mixed $value): mixed { if (is_array($value)) { - return array_map('trim', $value); + return $this->arrayMapRecursive('trim', $value); + } + + // Nothing to purge if it's not a string + if (!is_string($value)) { + return $value; } return trim($value); @@ -158,16 +333,40 @@ protected function trim(string|array $value): string|array /** * Autodetect if a field is an array or scalar, and filter appropriately. * - * @param string|string[] $value + * @param mixed $value * - * @return string|string[] + * @return mixed */ - protected function purify(string|array $value): string|array + protected function purify(mixed $value): mixed { if (is_array($value)) { - return array_map([$this->purifier, 'purify'], $value); + return $this->arrayMapRecursive([$this->purifier, 'purify'], $value); + } + + // Nothing to purge if it's not a string + if (!is_string($value)) { + return $value; } return $this->purifier->purify($value); } + + /** + * Applies the callback to the elements of the given arrays recursively. + * Required to apply transformation on multidimensional arrays. + * @see https://stackoverflow.com/a/39637749/445757 + * + * @param callable $callback + * @param mixed[] $array + * + * @return mixed[] + */ + protected function arrayMapRecursive(callable $callback, array $array): array + { + $func = function ($item) use (&$func, &$callback) { + return is_array($item) ? array_map($func, $item) : call_user_func($callback, $item); + }; + + return array_map($func, $array); + } } diff --git a/tests/Fortress/Transformer/RequestDataTransformerTest.php b/tests/Fortress/Transformer/RequestDataTransformerTest.php index f07a2520..af3d5ad8 100644 --- a/tests/Fortress/Transformer/RequestDataTransformerTest.php +++ b/tests/Fortress/Transformer/RequestDataTransformerTest.php @@ -35,12 +35,32 @@ public function setUp(): void $this->transformer = new RequestDataTransformer(); } - public function testTransformFieldForNotInSchema(): void + public function testTransformField(): void { $schema = new RequestSchema([ - 'email' => [], + 'email' => [ + 'transformations' => ['purge', 'trim'], + ], + ]); + + $result = $this->transformer->transformField($schema, 'email', ' foo@bar.com '); + $this->assertSame('foo@bar.com', $result); + } + + public function testTransformFieldButNoRules(): void + { + $schema = new RequestSchema([ + 'email' => [], ]); + $result = $this->transformer->transformField($schema, 'email', ' foo@bar.com '); + $this->assertSame(' foo@bar.com ', $result); + } + + public function testTransformFieldForNotInSchema(): void + { + $schema = new RequestSchema([]); + $result = $this->transformer->transformField($schema, 'foo', 'bar'); $this->assertSame('bar', $result); } @@ -121,7 +141,7 @@ public function testBasicWithOnUnexpectedVarError(): void // Set expectations $this->expectException(Exception::class); - $this->expectExceptionMessage("The field 'admin' is not a valid input field."); + $this->expectExceptionMessage("The fields 'admin' are not a valid input field."); // Act $this->transformer->transform($schema, $rawInput, 'error'); @@ -142,7 +162,7 @@ public function testTrim(): void // Assert $transformedData = [ 'display_name' => 'THE GREATEST', - 'email' => 'david@owlfancy.com', + 'email' => 'david@owlfancy.com', // Default value ]; $this->assertSame($transformedData, $result); @@ -277,6 +297,24 @@ public function testPurifyWithArrayValue(): void $this->assertSame($transformedData, $result); } + public function testPurifyForNonString(): void + { + // Act + $rawInput = [ + 'puppies' => true, + ]; + + $result = $this->transformer->transform($this->schema, $rawInput, 'skip'); + + // Assert + $transformedData = [ + 'puppies' => true, + 'email' => 'david@owlfancy.com', + ]; + + $this->assertSame($transformedData, $result); + } + /** * default transformer. */ @@ -297,4 +335,321 @@ public function testUnsupportedTransformation(): void $this->assertSame($transformedData, $result); } + + public function testInputArray(): void + { + // Data + $rawInput = [ + 'InputArray' => [ + 20, + '>10<
', + ' whitespace ', + ] + ]; + + // Schema + $schema = new RequestSchema($this->basePath . '/InputArray.yaml'); + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations + $transformedData = [ + 'InputArray' => [ + 20, + '>10<', + 'whitespace', + ] + ]; + + $this->assertSame($transformedData, $result); + } + + public function testMultidimensional(): void + { + // Data + $rawInput = [ + 'Settings' => [ + [ + 'name' => ' Foo ', + 'threshold' => ' 20 ', + ], + [ + 'name' => 'Bar
', + 'threshold' => '73
', + ], + ] + ]; + + // Schema + $schema = new RequestSchema($this->basePath . '/Multidimensional.yaml'); + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations + // - threshold has Trim + // - name has purge + $transformedData = [ + 'Settings' => [ + [ + 'name' => ' Foo ', + 'threshold' => '20', // Only threshold is trimmed + ], + [ + 'name' => 'Bar ', // Only name is purged, but since we don't trim, the space will stay ! + 'threshold' => '73
', + ], + ] + ]; + $this->assertSame($transformedData, $result); + } + + /** + * Same as previous test, but the transformation are at the root "Settings". + * Per element rules will be overwritten by the root version. Everything + * will be purged & trimmed. + */ + public function testMultidimensionalRootLevel(): void + { + // Data + $rawInput = [ + 'Settings' => [ + [ + 'name' => ' Foo ', + 'threshold' => ' 20 ', + ], + [ + 'name' => 'Bar
', + 'threshold' => '73
', + ], + ] + ]; + + // Schema + $schema = new RequestSchema([ + 'Settings' => [ + 'transformations' => ['purge', 'trim'], + ], + 'Settings.*.threshold' => [ + 'transformations' => ['trim'], + ], + 'Settings.*.name' => [ + 'transformations' => ['purge'], + ], + ]); + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations + $transformedData = [ + 'Settings' => [ + [ + 'name' => 'Foo', + 'threshold' => '20', + ], + [ + 'name' => 'Bar', + 'threshold' => '73', + ], + ] + ]; + $this->assertSame($transformedData, $result); + } + + public function testMultidimensionalPurge(): void + { + // Schema - Use two multidimensional field : Named has a name only, + // Colored has color only. + $schema = new RequestSchema([ + 'Named' => [], + 'Named.*.name' => [], + 'Colored.*.color' => [], + ]); + + // Data + $rawInput = [ + 'Named' => [ + [ + 'name' => 'Joe', + 'color' => 'blue', + ], + [ + 'name' => 'Kathy', + 'color' => 'pink', + 'description' => 'Something', + ], + ], + 'Colored' => [ + [ + 'name' => 'John', + 'color' => 'red', + ] + ], + ]; + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations - Again, Named has a name only, Colored has color only + $transformedData = [ + 'Named' => [ + ['name' => 'Joe'], + ['name' => 'Kathy'], + ], + 'Colored' => [ + ['color' => 'red'] + ], + ]; + $this->assertSame($transformedData, $result); + } + + /** + * Same as the previous test, but 'Named' doesn't have a root field in the + * schema, to prove both situation are the same + */ + public function testMultidimensionalPurgeNoRoot(): void + { + $schema = new RequestSchema([ + 'Named.*.name' => [], + 'Colored.*.color' => [], + ]); + + // Data + $rawInput = [ + 'Named' => [ + [ + 'name' => 'Joe', + 'color' => 'blue', + ], + [ + 'name' => 'Kathy', + 'color' => 'pink', + 'description' => 'Something', + ], + ], + 'Colored' => [ + [ + 'name' => 'John', + 'color' => 'red', + ] + ], + ]; + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations - Again, Named has a name only, Colored has color only + $transformedData = [ + 'Named' => [ + ['name' => 'Joe'], + ['name' => 'Kathy'], + ], + 'Colored' => [ + ['color' => 'red'] + ], + ]; + $this->assertSame($transformedData, $result); + } + + public function testPurgeNotInSchema(): void + { + // Use empty schema - All input will be purged + $schema = new RequestSchema([]); + + // Data + $rawInput = [ + 'Named' => [ + [ + 'name' => 'Joe', + 'color' => 'blue', + ], + [ + 'name' => 'Kathy', + 'color' => 'pink', + 'description' => 'Something', + ], + ], + 'Colored' => [ + [ + 'name' => 'John', + 'color' => 'red', + ] + ], + 'FooBar' => true, // Will be purged + 'FooBarArray' => [true, false], // Will be purged + ]; + + // Act and assert + $result = $this->transformer->transform($schema, $rawInput); + $this->assertSame([], $result); + } + + public function testInputArrayPurge(): void + { + // Schema + $schema = new RequestSchema([ + 'InputArray.*' => [], + ]); + + // Data + $rawInput = [ + 'InputArray' => [ + '10', + 20, + true, + ], + 'Foo' => true // Will be purged + ]; + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations + $transformedData = [ + 'InputArray' => [ + '10', + 20, + true, + ] + ]; + + $this->assertSame($transformedData, $result); + } + + /** + * Same as previous, but without the '*' wildcard + */ + public function testInputArrayRootPurge(): void + { + // Schema + $schema = new RequestSchema([ + 'InputArray' => [], + ]); + + // Data + $rawInput = [ + 'InputArray' => [ + '10', + 20, + true, + ], + 'Foo' => true // Will be purged + ]; + + // Act + $result = $this->transformer->transform($schema, $rawInput); + + // Set expectations + $transformedData = [ + 'InputArray' => [ + '10', + 20, + true, + ] + ]; + + $this->assertSame($transformedData, $result); + } } diff --git a/tests/Fortress/Validator/ServerSideValidatorTest.php b/tests/Fortress/Validator/ServerSideValidatorTest.php index c9c28e4b..d6c92877 100644 --- a/tests/Fortress/Validator/ServerSideValidatorTest.php +++ b/tests/Fortress/Validator/ServerSideValidatorTest.php @@ -20,11 +20,13 @@ class ServerSideValidatorTest extends TestCase { + protected string $basePath; protected Translator $translator; protected ServerSideValidator $validator; public function setUp(): void { + $this->basePath = __DIR__.'/../data'; $this->translator = new Translator(new DictionaryStub()); $this->validator = new ServerSideValidator($this->translator); } @@ -927,4 +929,118 @@ public function testValidateWithNoValidatorMessage(): void $this->assertNotEmpty($errors); $this->assertSame(['User Name Invalid'], $errors['user_name']); } + + public function testInputArray(): void + { + // Get schema + $schema = new RequestSchema($this->basePath.'/InputArray.yaml'); + + // Test no errors + $errors = $this->validator->validate($schema, [ + 'InputArray' => [20, 73] + ]); + $this->assertEmpty($errors); + + // Test each element must be integer + $errors = $this->validator->validate($schema, [ + 'InputArray' => [20, 'string'] + ]); + $this->assertNotEmpty($errors); + $this->assertSame(['Input elements must be integers.'], $errors['InputArray.*']); + + // Test the input itself is required + $errors = $this->validator->validate($schema, []); + $this->assertNotEmpty($errors); + $this->assertSame(['InputArray is required', 'InputArray must be an array'], $errors['InputArray']); + + // Test the input itself must be an array + $errors = $this->validator->validate($schema, [ + 'InputArray' => 20 + ]); + $this->assertNotEmpty($errors); + $this->assertSame(['InputArray must be an array'], $errors['InputArray']); + } + + public function testMultidimensional(): void + { + // Get schema + $schema = new RequestSchema($this->basePath.'/Multidimensional.yaml'); + + // String threshold + $errors = $this->validator->validate($schema, [ + 'Settings' => [ + [ + 'name' => 'Foo', + 'threshold' => 'eleven', // Bad + ], + [ + 'name' => 'Bar', + 'threshold' => 73, + ], + ] + ]); + $this->assertCount(1, $errors); + $this->assertSame(['Value must be max 100', 'Input elements must be integers'], $errors['Settings.*.threshold']); + + // Max threshold + $errors = $this->validator->validate($schema, [ + 'Settings' => [ + [ + 'name' => 'Foo', + 'threshold' => 11, + ], + [ + 'name' => 'Bar', + 'threshold' => 730, // Bad, to high + ], + ] + ]); + $this->assertCount(1, $errors); + $this->assertSame(['Value must be max 100'], $errors['Settings.*.threshold']); + + // String int are accepted + $errors = $this->validator->validate($schema, [ + 'Settings' => [ + [ + 'name' => 'Foo', + 'threshold' => '11', // Actually good + ], + [ + 'name' => 'Bar', + 'threshold' => '73', // Actually good + ], + ] + ]); + $this->assertCount(0, $errors); + + // Required name, but Threshold is optional + $errors = $this->validator->validate($schema, [ + 'Settings' => [ + [ + 'name' => 'Foo', + 'threshold' => 11, + ], + [ + 'threshold' => 73, // Bad, missing name + ], + [ + 'name' => 'Foo Bar', + ], + ] + ]); + $this->assertCount(1, $errors); + $this->assertSame(['Each settings must have a name'], $errors['Settings.*.name']); + + // Test the Settings input itself is required + $errors = $this->validator->validate($schema, []); + $this->assertNotEmpty($errors); + $this->assertSame(['Settings array is required', 'Settings must be an input array'], $errors['Settings']); + + // Test the input itself must be an array + $errors = $this->validator->validate($schema, [ + 'Settings' => 20 + ]); + $this->assertNotEmpty($errors); + $this->assertSame(['Settings must be an input array'], $errors['Settings']); + } } diff --git a/tests/Fortress/data/InputArray.yaml b/tests/Fortress/data/InputArray.yaml new file mode 100644 index 00000000..0d038d4f --- /dev/null +++ b/tests/Fortress/data/InputArray.yaml @@ -0,0 +1,15 @@ +InputArray: + validators: + required: + message: InputArray is required + array: + message: InputArray must be an array + +InputArray.*: + validators: + integer: + message: Input elements must be integers. + transformations: + - purge + - trim + \ No newline at end of file diff --git a/tests/Fortress/data/Multidimensional.yaml b/tests/Fortress/data/Multidimensional.yaml new file mode 100644 index 00000000..5fae4110 --- /dev/null +++ b/tests/Fortress/data/Multidimensional.yaml @@ -0,0 +1,25 @@ +Settings: + validators: + required: + message: Settings array is required + array: + message: Settings must be an input array + +Settings.*.threshold: + validators: + range: + max: 100 + message: Value must be max 100 + integer: + message: Input elements must be integers + transformations: + - trim + +Settings.*.name: + validators: + required: + message: Each settings must have a name + string: + message: Name elements must be string + transformations: + - purge \ No newline at end of file