From c1034418bb7c0fb5e8a1b2f8012933860de27c73 Mon Sep 17 00:00:00 2001 From: othercorey Date: Mon, 2 Oct 2023 17:31:32 -0500 Subject: [PATCH 01/12] Update version to 4.4.18 --- VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION.txt b/VERSION.txt index 7a02623494b..59dfee26fb9 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -16,4 +16,4 @@ // @license https://opensource.org/licenses/mit-license.php MIT License // +--------------------------------------------------------------------------------------------+ // //////////////////////////////////////////////////////////////////////////////////////////////////// -4.4.17 +4.4.18 From 03dfa4f4ad0fb2e18a8fdc860b1db30e0c8d3b5c Mon Sep 17 00:00:00 2001 From: Lars Ebert Date: Fri, 6 Oct 2023 11:38:33 +0200 Subject: [PATCH 02/12] Issue #17318: Cyclic Recursion while using EntityTrait::getErrors and ::hasErrors --- src/Datasource/EntityTrait.php | 51 +++++++++++++++++++++++-------- tests/TestCase/ORM/EntityTest.php | 31 +++++++++++++++++++ 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/Datasource/EntityTrait.php b/src/Datasource/EntityTrait.php index b8ef4a9ddac..a58bd959f92 100644 --- a/src/Datasource/EntityTrait.php +++ b/src/Datasource/EntityTrait.php @@ -118,6 +118,11 @@ trait EntityTrait */ protected $_registryAlias = ''; + /** + * Storing the current visitation status while recursing through entities getting errors. + */ + private $_hasBeenVisited = false; + /** * Magic getter to access fields that have been set in this entity * @@ -845,6 +850,11 @@ public function isNew(): bool */ public function hasErrors(bool $includeNested = true): bool { + if($this->_hasBeenVisited) { + // While recursing through entities, each entity should only be visited once. See https://github.com/cakephp/cakephp/issues/17318 + return false; + } + if (Hash::filter($this->_errors)) { return true; } @@ -853,10 +863,15 @@ public function hasErrors(bool $includeNested = true): bool return false; } - foreach ($this->_fields as $field) { - if ($this->_readHasErrors($field)) { - return true; + $this->_hasBeenVisited = true; + try { + foreach ($this->_fields as $field) { + if ($this->_readHasErrors($field)) { + return true; + } } + } finally { + $this->_hasBeenVisited = false; } return false; @@ -869,17 +884,29 @@ public function hasErrors(bool $includeNested = true): bool */ public function getErrors(): array { + if($this->_hasBeenVisited) { + // While recursing through entities, each entity should only be visited once. See https://github.com/cakephp/cakephp/issues/17318 + return []; + } + $diff = array_diff_key($this->_fields, $this->_errors); - return $this->_errors + (new Collection($diff)) - ->filter(function ($value) { - return is_array($value) || $value instanceof EntityInterface; - }) - ->map(function ($value) { - return $this->_readError($value); - }) - ->filter() - ->toArray(); + $this->_hasBeenVisited = true; + try { + $errors = $this->_errors + (new Collection($diff)) + ->filter(function ($value) { + return is_array($value) || $value instanceof EntityInterface; + }) + ->map(function ($value) { + return $this->_readError($value); + }) + ->filter() + ->toArray(); + } finally { + $this->_hasBeenVisited = false; + } + + return $errors; } /** diff --git a/tests/TestCase/ORM/EntityTest.php b/tests/TestCase/ORM/EntityTest.php index 142525ecb72..b9b59d7a36e 100644 --- a/tests/TestCase/ORM/EntityTest.php +++ b/tests/TestCase/ORM/EntityTest.php @@ -1654,4 +1654,35 @@ public function testHasValue(): void $this->assertTrue($entity->hasValue('floatNonZero')); $this->assertFalse($entity->hasValue('null')); } + + /** + * Test infinite recursion in getErrors and hasErrors + * See https://github.com/cakephp/cakephp/issues/17318 + */ + public function testGetErrorsRecursionError() { + $entity = new Entity(); + $secondEntity = new Entity(); + + $entity->set('child', $secondEntity); + $secondEntity->set('parent', $entity); + + $expectedErrors = ['name' => ['_required' => 'Must be present.']]; + $secondEntity->setErrors($expectedErrors); + + $this->assertEquals(['child' => $expectedErrors], $entity->getErrors()); + } + + /** + * Test infinite recursion in getErrors and hasErrors + * See https://github.com/cakephp/cakephp/issues/17318 + */ + public function testHasErrorsRecursionError() { + $entity = new Entity(); + $secondEntity = new Entity(); + + $entity->set('child', $secondEntity); + $secondEntity->set('parent', $entity); + + $this->assertFalse($entity->hasErrors()); + } } From 514189fca60dec7714000703a469b346a9a35b63 Mon Sep 17 00:00:00 2001 From: Lars Ebert Date: Fri, 6 Oct 2023 11:57:04 +0200 Subject: [PATCH 03/12] #17318: Fixed Formatting and phpcs --- src/Datasource/EntityTrait.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Datasource/EntityTrait.php b/src/Datasource/EntityTrait.php index a58bd959f92..be47264b61a 100644 --- a/src/Datasource/EntityTrait.php +++ b/src/Datasource/EntityTrait.php @@ -120,6 +120,8 @@ trait EntityTrait /** * Storing the current visitation status while recursing through entities getting errors. + * + * @var bool */ private $_hasBeenVisited = false; @@ -850,7 +852,7 @@ public function isNew(): bool */ public function hasErrors(bool $includeNested = true): bool { - if($this->_hasBeenVisited) { + if ($this->_hasBeenVisited) { // While recursing through entities, each entity should only be visited once. See https://github.com/cakephp/cakephp/issues/17318 return false; } @@ -884,7 +886,7 @@ public function hasErrors(bool $includeNested = true): bool */ public function getErrors(): array { - if($this->_hasBeenVisited) { + if ($this->_hasBeenVisited) { // While recursing through entities, each entity should only be visited once. See https://github.com/cakephp/cakephp/issues/17318 return []; } From ed27f44eb1a28415b7f9448a2cd63a2fba02a265 Mon Sep 17 00:00:00 2001 From: Lars Ebert Date: Sat, 7 Oct 2023 11:38:47 +0200 Subject: [PATCH 04/12] #17318 code formatting --- tests/TestCase/ORM/EntityTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/TestCase/ORM/EntityTest.php b/tests/TestCase/ORM/EntityTest.php index b9b59d7a36e..8550a58edac 100644 --- a/tests/TestCase/ORM/EntityTest.php +++ b/tests/TestCase/ORM/EntityTest.php @@ -1659,7 +1659,8 @@ public function testHasValue(): void * Test infinite recursion in getErrors and hasErrors * See https://github.com/cakephp/cakephp/issues/17318 */ - public function testGetErrorsRecursionError() { + public function testGetErrorsRecursionError() + { $entity = new Entity(); $secondEntity = new Entity(); @@ -1676,7 +1677,8 @@ public function testGetErrorsRecursionError() { * Test infinite recursion in getErrors and hasErrors * See https://github.com/cakephp/cakephp/issues/17318 */ - public function testHasErrorsRecursionError() { + public function testHasErrorsRecursionError() + { $entity = new Entity(); $secondEntity = new Entity(); From b9b286ed98c6a5d7da0acff0661b60b273912cdc Mon Sep 17 00:00:00 2001 From: Lars Ebert Date: Sat, 7 Oct 2023 14:38:58 +0200 Subject: [PATCH 05/12] #17318 protected EntityTrait::_hasBeenVisited Co-authored-by: ADmad --- src/Datasource/EntityTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Datasource/EntityTrait.php b/src/Datasource/EntityTrait.php index be47264b61a..c8b6db59c08 100644 --- a/src/Datasource/EntityTrait.php +++ b/src/Datasource/EntityTrait.php @@ -123,7 +123,7 @@ trait EntityTrait * * @var bool */ - private $_hasBeenVisited = false; + protected $_hasBeenVisited = false; /** * Magic getter to access fields that have been set in this entity From c37c03eb1400f5c2b636189d03b04ce4f22cf473 Mon Sep 17 00:00:00 2001 From: othercorey Date: Mon, 9 Oct 2023 22:03:30 -0500 Subject: [PATCH 06/12] Update cancel workflow to version 0.12 --- .github/workflows/cancel.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cancel.yml b/.github/workflows/cancel.yml index d7f9ec485a0..9ba0aedd480 100644 --- a/.github/workflows/cancel.yml +++ b/.github/workflows/cancel.yml @@ -13,6 +13,6 @@ jobs: actions: write # for styfle/cancel-workflow-action to cancel/stop running workflows runs-on: ubuntu-latest steps: - - uses: styfle/cancel-workflow-action@0.11.0 + - uses: styfle/cancel-workflow-action@0.12.0 with: workflow_id: ${{ github.event.workflow.id }} From 75dc4410c8206b3de80a4a77104347a9158183db Mon Sep 17 00:00:00 2001 From: othercorey Date: Mon, 9 Oct 2023 22:38:26 -0500 Subject: [PATCH 07/12] Update phpstan-baseline.neon --- tests/phpstan-baseline.neon | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/phpstan-baseline.neon b/tests/phpstan-baseline.neon index 21f080b0c0d..e7943c62e46 100644 --- a/tests/phpstan-baseline.neon +++ b/tests/phpstan-baseline.neon @@ -10,11 +10,6 @@ parameters: count: 5 path: TestCase/Database/Expression/CaseStatementExpressionTest.php - - - message: "#^Static method Cake\\\\Chronos\\\\ChronosInterface\\:\\:now\\(\\) invoked with 0 parameters, 1 required\\.$#" - count: 5 - path: TestCase/Database/Expression/CaseStatementExpressionTest.php - - message: "#^Instantiated class Cake\\\\Chronos\\\\Date not found\\.$#" count: 15 From 95e39ec60f78895d217c35c6f840f24f1f9ca3b5 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sat, 14 Oct 2023 22:25:20 -0400 Subject: [PATCH 08/12] Update version number to 4.5.0 --- VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION.txt b/VERSION.txt index 606b97669e0..4a7d7a656de 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -16,4 +16,4 @@ // @license https://opensource.org/licenses/mit-license.php MIT License // +--------------------------------------------------------------------------------------------+ // //////////////////////////////////////////////////////////////////////////////////////////////////// -4.5.0-RC1 +4.5.0 From 1743c57ffe0f660eb595b38908163c604b83af88 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sat, 14 Oct 2023 22:54:48 -0400 Subject: [PATCH 09/12] Bump version --- VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION.txt b/VERSION.txt index 606b97669e0..3b2bdc60b80 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -16,4 +16,4 @@ // @license https://opensource.org/licenses/mit-license.php MIT License // +--------------------------------------------------------------------------------------------+ // //////////////////////////////////////////////////////////////////////////////////////////////////// -4.5.0-RC1 +4.6.0-dev From 7551b8c19d7474830d5e57db751cc06479971c80 Mon Sep 17 00:00:00 2001 From: ADmad Date: Mon, 9 Oct 2023 19:42:59 +0530 Subject: [PATCH 10/12] Don't throw an exception for unsupported types. If the ControllerFactory encounters a method argument type like union type which it cannot use for conversion, it no longer throws an exception and passes the value as is. Backport fixes from #17338 to 4.x --- src/Controller/ControllerFactory.php | 11 ---------- .../Controller/ControllerFactoryTest.php | 22 +++++++++++++++++++ .../Controller/DependenciesController.php | 5 +++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/Controller/ControllerFactory.php b/src/Controller/ControllerFactory.php index 32a41e0a2d5..88e776c4b06 100644 --- a/src/Controller/ControllerFactory.php +++ b/src/Controller/ControllerFactory.php @@ -160,17 +160,6 @@ protected function getActionArgs(Closure $action, array $passedParams): array $function = new ReflectionFunction($action); foreach ($function->getParameters() as $parameter) { $type = $parameter->getType(); - if ($type && !$type instanceof ReflectionNamedType) { - // Only single types are supported - throw new InvalidParameterException([ - 'template' => 'unsupported_type', - 'parameter' => $parameter->getName(), - 'controller' => $this->controller->getName(), - 'action' => $this->controller->getRequest()->getParam('action'), - 'prefix' => $this->controller->getRequest()->getParam('prefix'), - 'plugin' => $this->controller->getRequest()->getParam('plugin'), - ]); - } // Check for dependency injection for classes if ($type instanceof ReflectionNamedType && !$type->isBuiltin()) { diff --git a/tests/TestCase/Controller/ControllerFactoryTest.php b/tests/TestCase/Controller/ControllerFactoryTest.php index 7802cfa737e..9d00737e8df 100644 --- a/tests/TestCase/Controller/ControllerFactoryTest.php +++ b/tests/TestCase/Controller/ControllerFactoryTest.php @@ -879,6 +879,28 @@ public function testInvokePassedParamUnsupportedType(): void $this->factory->invoke($controller); } + /** + * Test using an unsupported reflection type. + */ + public function testInvokePassedParamUnsupportedReflectionType(): void + { + $request = new ServerRequest([ + 'url' => 'test_plugin_three/dependencies/unsupportedTypedUnion', + 'params' => [ + 'plugin' => null, + 'controller' => 'Dependencies', + 'action' => 'typedUnion', + 'pass' => ['1'], + ], + ]); + $controller = $this->factory->create($request); + + $result = $this->factory->invoke($controller); + $data = json_decode((string)$result->getBody(), true); + + $this->assertSame(['one' => '1'], $data); + } + public function testMiddleware(): void { $request = new ServerRequest([ diff --git a/tests/test_app/TestApp/Controller/DependenciesController.php b/tests/test_app/TestApp/Controller/DependenciesController.php index feb130a9662..70b42877731 100644 --- a/tests/test_app/TestApp/Controller/DependenciesController.php +++ b/tests/test_app/TestApp/Controller/DependenciesController.php @@ -61,6 +61,11 @@ public function unsupportedTyped(iterable $one) return $this->response->withStringBody(json_encode(compact('one'))); } + public function typedUnion(string|int $one) + { + return $this->response->withStringBody(json_encode(compact('one'))); + } + /** * @param mixed $any * @return \Cake\Http\Response From cfc5bf42fd776f762ea8483f7916be1138f4ca49 Mon Sep 17 00:00:00 2001 From: Corey Taylor Date: Tue, 10 Oct 2023 02:15:18 -0500 Subject: [PATCH 11/12] Throw exception on invalid invalid key for Collection::combine() Backport #17340 to 4.x --- src/Collection/CollectionTrait.php | 27 +++++++++++++- tests/TestCase/Collection/CollectionTest.php | 39 ++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/Collection/CollectionTrait.php b/src/Collection/CollectionTrait.php index 010b06a63f5..7cee9696965 100644 --- a/src/Collection/CollectionTrait.php +++ b/src/Collection/CollectionTrait.php @@ -591,14 +591,37 @@ public function combine($keyPath, $valuePath, $groupPath = null): CollectionInte $rowVal = $options['valuePath']; if (!$options['groupPath']) { - $mapReduce->emit($rowVal($value, $key), $rowKey($value, $key)); + $mapKey = $rowKey($value, $key); + if ($mapKey === null) { + throw new InvalidArgumentException( + 'Cannot index by path that does not exist or contains a null value. ' . + 'Use a callback to return a default value for that path.' + ); + } + + $mapReduce->emit($rowVal($value, $key), $mapKey); return null; } $key = $options['groupPath']($value, $key); + if ($key === null) { + throw new InvalidArgumentException( + 'Cannot group by path that does not exist or contains a null value. ' . + 'Use a callback to return a default value for that path.' + ); + } + + $mapKey = $rowKey($value, $key); + if ($mapKey === null) { + throw new InvalidArgumentException( + 'Cannot index by path that does not exist or contains a null value. ' . + 'Use a callback to return a default value for that path.' + ); + } + $mapReduce->emitIntermediate( - [$rowKey($value, $key) => $rowVal($value, $key)], + [$mapKey => $rowVal($value, $key)], $key ); }; diff --git a/tests/TestCase/Collection/CollectionTest.php b/tests/TestCase/Collection/CollectionTest.php index f54af2407a3..2233c1f18b6 100644 --- a/tests/TestCase/Collection/CollectionTest.php +++ b/tests/TestCase/Collection/CollectionTest.php @@ -1241,6 +1241,45 @@ function ($value, $key) { $this->assertEquals([1 => null, 2 => null, 3 => null], $collection->toArray()); } + public function testCombineNullKey(): void + { + $items = [ + ['id' => 1, 'name' => 'foo', 'parent' => 'a'], + ['id' => null, 'name' => 'bar', 'parent' => 'b'], + ['id' => 3, 'name' => 'baz', 'parent' => 'a'], + ]; + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Cannot index by path that does not exist or contains a null value'); + (new Collection($items))->combine('id', 'name'); + } + + public function testCombineNullGroup(): void + { + $items = [ + ['id' => 1, 'name' => 'foo', 'parent' => 'a'], + ['id' => 2, 'name' => 'bar', 'parent' => 'b'], + ['id' => 3, 'name' => 'baz', 'parent' => null], + ]; + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Cannot group by path that does not exist or contains a null value'); + (new Collection($items))->combine('id', 'name', 'parent'); + } + + public function testCombineGroupNullKey(): void + { + $items = [ + ['id' => 1, 'name' => 'foo', 'parent' => 'a'], + ['id' => 2, 'name' => 'bar', 'parent' => 'b'], + ['id' => null, 'name' => 'baz', 'parent' => 'a'], + ]; + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Cannot index by path that does not exist or contains a null value'); + (new Collection($items))->combine('id', 'name', 'parent'); + } + /** * Tests the nest method with only one level */ From 9cf4547669a0fa1ef9668e80af6de7734f19a330 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Oct 2023 10:35:13 -0400 Subject: [PATCH 12/12] Make tests work in PHP7.4 as well. --- .../Controller/ControllerFactoryTest.php | 5 +-- .../Controller/DependenciesController.php | 5 --- .../UnionDependenciesController.php | 36 +++++++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 tests/test_app/TestApp/Controller/UnionDependenciesController.php diff --git a/tests/TestCase/Controller/ControllerFactoryTest.php b/tests/TestCase/Controller/ControllerFactoryTest.php index 9d00737e8df..3e1eb5ab169 100644 --- a/tests/TestCase/Controller/ControllerFactoryTest.php +++ b/tests/TestCase/Controller/ControllerFactoryTest.php @@ -884,11 +884,12 @@ public function testInvokePassedParamUnsupportedType(): void */ public function testInvokePassedParamUnsupportedReflectionType(): void { + $this->skipIf(version_compare(PHP_VERSION, '8.0', '<='), 'Unions require PHP 8'); $request = new ServerRequest([ - 'url' => 'test_plugin_three/dependencies/unsupportedTypedUnion', + 'url' => 'test_plugin_three/unionDependencies/typedUnion', 'params' => [ 'plugin' => null, - 'controller' => 'Dependencies', + 'controller' => 'UnionDependencies', 'action' => 'typedUnion', 'pass' => ['1'], ], diff --git a/tests/test_app/TestApp/Controller/DependenciesController.php b/tests/test_app/TestApp/Controller/DependenciesController.php index 70b42877731..feb130a9662 100644 --- a/tests/test_app/TestApp/Controller/DependenciesController.php +++ b/tests/test_app/TestApp/Controller/DependenciesController.php @@ -61,11 +61,6 @@ public function unsupportedTyped(iterable $one) return $this->response->withStringBody(json_encode(compact('one'))); } - public function typedUnion(string|int $one) - { - return $this->response->withStringBody(json_encode(compact('one'))); - } - /** * @param mixed $any * @return \Cake\Http\Response diff --git a/tests/test_app/TestApp/Controller/UnionDependenciesController.php b/tests/test_app/TestApp/Controller/UnionDependenciesController.php new file mode 100644 index 00000000000..220297a6b30 --- /dev/null +++ b/tests/test_app/TestApp/Controller/UnionDependenciesController.php @@ -0,0 +1,36 @@ +inject = $inject; + } + + public function typedUnion(string|int $one) + { + return $this->response->withStringBody(json_encode(compact('one'))); + } +}