diff --git a/CHANGELOG-5.0.md b/CHANGELOG-5.0.md index f65598c369..df7d4bfe32 100644 --- a/CHANGELOG-5.0.md +++ b/CHANGELOG-5.0.md @@ -16,6 +16,7 @@ ### Fixed - Parse multipart/form-data from PUT request [#16271](https://github.com/phalcon/cphalcon/issues/16271) +- Infinite save loop in Model::save() [#16395](https://github.com/phalcon/cphalcon/issues/16395) ## [5.2.3](https://github.com/phalcon/cphalcon/releases/tag/v5.2.3) (2023-07-26) diff --git a/phalcon/Mvc/Model.zep b/phalcon/Mvc/Model.zep index 7a9c65e2d9..5380448160 100644 --- a/phalcon/Mvc/Model.zep +++ b/phalcon/Mvc/Model.zep @@ -42,6 +42,7 @@ use Phalcon\Mvc\Model\ValidationFailed; use Phalcon\Mvc\ModelInterface; use Phalcon\Filter\Validation\ValidationInterface; use Serializable; +use SplObjectStorage; /** * Phalcon\Mvc\Model @@ -2579,12 +2580,34 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, * $robot->save(); *``` */ + public function save() -> bool + { + var visited; + let visited = new SplObjectStorage(); + return this->doSave(visited); + } + + /** + * Inserted or updates model instance, expects a visited list of objects. + * + * @param SplObjectStorage $visited + * + * @return bool + */ + + public function doSave( visited) -> bool { var metaData, schema, writeConnection, readConnection, source, table, identityField, exists, success, relatedToSave; bool hasRelatedToSave; + if true === visited->contains(this) { + return true; + } + + visited->attach(this); + let metaData = this->getModelsMetaData(); /** @@ -2610,7 +2633,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, let hasRelatedToSave = count(relatedToSave) > 0; if hasRelatedToSave { - if this->preSaveRelatedRecords(writeConnection, relatedToSave) === false { + if this->preSaveRelatedRecords(writeConnection, relatedToSave, visited) === false { return false; } } @@ -2695,7 +2718,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, /** * Change the dirty state to persistent */ - if success { + if true === success{ let this->dirtyState = self::DIRTY_STATE_PERSISTENT; } @@ -2711,7 +2734,8 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, */ let success = this->postSaveRelatedRecords( writeConnection, - relatedToSave + relatedToSave, + visited ); } } @@ -4964,9 +4988,10 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, * Saves related records that must be stored prior to save the master record * * @param ModelInterface[] related + * @param SplObjectStorage visited * @return bool */ - protected function preSaveRelatedRecords( connection, related) -> bool + protected function preSaveRelatedRecords( connection, related, visited) -> bool { var className, manager, type, relation, columns, referencedFields, nesting, name, record; @@ -4981,6 +5006,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, manager = this->getModelsManager(); for name, record in related { + /** * Try to get a relation with the same name */ @@ -5006,7 +5032,6 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, "Only objects can be stored as part of belongs-to relations in '" . get_class(this) . "' Relation " . name ); } - let columns = relation->getFields(), referencedFields = relation->getReferencedFields(); // let columns = relation->getFields(), @@ -5023,7 +5048,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, * If dynamic update is enabled, saving the record must not take any action * Only save if the model is dirty to prevent circular relations causing an infinite loop */ - if record->dirtyState !== Model::DIRTY_STATE_PERSISTENT && !record->save() { + if record->dirtyState !== Model::DIRTY_STATE_PERSISTENT && !record->doSave(visited) { /** * Get the validation messages generated by the * referenced model @@ -5074,7 +5099,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, * @param ModelInterface[] related * @return bool */ - protected function postSaveRelatedRecords( connection, related) -> bool + protected function postSaveRelatedRecords( connection, related, visited) -> bool { var nesting, className, manager, relation, name, record, columns, referencedModel, referencedFields, relatedRecords, value, @@ -5157,7 +5182,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, /** * Save the record and get messages */ - if !recordAfter->save() { + if !recordAfter->doSave(visited) { /** * Get the validation messages generated by the * referenced model @@ -5221,7 +5246,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, /** * Save the record and get messages */ - if !intermediateModel->save() { + if !intermediateModel->doSave(visited) { /** * Get the validation messages generated by the referenced model */ @@ -5244,7 +5269,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface, /** * Save the record and get messages */ - if !recordAfter->save() { + if !recordAfter->doSave(visited) { /** * Get the validation messages generated by the * referenced model diff --git a/tests/database/Mvc/Model/SaveCest.php b/tests/database/Mvc/Model/SaveCest.php index 3bf55a34f7..ec30963f2b 100644 --- a/tests/database/Mvc/Model/SaveCest.php +++ b/tests/database/Mvc/Model/SaveCest.php @@ -644,6 +644,35 @@ public function mvcModelSaveWithRelatedManyAndBelongsRecordsProperty(DatabaseTes $I->assertEquals($expected, $actual); } + /** + * Tests Phalcon\Mvc\Model\ :: save() Infinite Loop + * + * @author Phalcon Team + * @since 2023-08-09 + * @issue #16395 + * + * @group mysql + * @group sqlite + */ + public function infiniteSaveLoop(DatabaseTester $I) + { + $I->wantToTest('Mvc\Model - save() infinite Save loop'); + + /** @var \PDO $connection */ + $connection = $I->getConnection(); + $invoicesMigration = new InvoicesMigration($connection); + $invoicesMigration->insert(77, 1, 0, uniqid('inv-', true)); + + $customersMigration = new CustomersMigration($connection); + $customersMigration->insert(1, 1, 'test_firstName_1', 'test_lastName_1'); + + $customer = Customers::findFirst(1); + $invoice = Invoices::findFirst(77); + $invoice->customer = $customer; + $customer->invoices = [ $invoice ]; + $customer->save(); + } + /** * @return \string[][] */