From 43bf1cd715a672b8e51a9f6cfe2c6a55b6338b06 Mon Sep 17 00:00:00 2001
From: Stephen Holdaway <stecman@gmail.com>
Date: Sun, 28 Apr 2019 20:28:37 +1200
Subject: [PATCH] Fix segfault on infinite recursion into circular model
 relations

In cases where related models were set before save on *both* sides of a
hasMany/belongsTo relationship, `Model::save()` would go into an infinite
loop as its pre and post-save checks for relations to save never checked
if the related model had already been saved:

    Save A
     \_ A hasMany B
      \_ Save B
       \_ B belongsTo A
        \_ Save A
         \_ ...

For me this was causing a segfault, though with xdebug installed it would
error before crashing at the max_nesting_level.
---
 CHANGELOG-4.0.md                         |  1 +
 phalcon/Mvc/Model.zep                    |  6 +++---
 tests/integration/Mvc/Model/SaveCest.php | 27 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG-4.0.md b/CHANGELOG-4.0.md
index 21438bd81f2..7071b8f61d5 100644
--- a/CHANGELOG-4.0.md
+++ b/CHANGELOG-4.0.md
@@ -30,6 +30,7 @@
 - Fixed `Phalcon\Mvc\View::getRender()` to call `view->finish()` instead of `ob_end_clean()`. [#14095](https://github.com/phalcon/cphalcon/issues/14095)
 - Fixed `Phalcon\Cache\Adapter\Libmemcached` failing to set values when `Phalcon\Mvc\Model\MetaData\Libmemcached` was in use. [#14100](https://github.com/phalcon/cphalcon/issues/14100)
 - Fixed `Phalcon\Db\Column` to recognize `tinyint`, `smallint`, `mediumint`, `integer` as valid autoIncrement columns. [#14102](https://github.com/phalcon/cphalcon/issues/14102)
+- Fixed crash in `Phalcon\Mvc\Model::save()` when saving a circular model relation. [#13354](https://github.com/phalcon/cphalcon/pull/13354)
 
 ## Removed
 - Removed `Phalcon\Session\Factory`. [#13672](https://github.com/phalcon/cphalcon/issues/13672)
diff --git a/phalcon/Mvc/Model.zep b/phalcon/Mvc/Model.zep
index 9051f6a8ffa..497746f64c3 100644
--- a/phalcon/Mvc/Model.zep
+++ b/phalcon/Mvc/Model.zep
@@ -4625,10 +4625,10 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
                     }
 
                     /**
-                     * If dynamic update is enabled, saving the record must not
-                     * take any action
+                     * 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->save() {
+                    if record->_dirtyState != Model::DIRTY_STATE_PERSISTENT && !record->save() {
                         /**
                          * Get the validation messages generated by the
                          * referenced model
diff --git a/tests/integration/Mvc/Model/SaveCest.php b/tests/integration/Mvc/Model/SaveCest.php
index ebfdb36a0a7..d2587911a8a 100644
--- a/tests/integration/Mvc/Model/SaveCest.php
+++ b/tests/integration/Mvc/Model/SaveCest.php
@@ -334,4 +334,31 @@ public function mvcModelSaveAfterWithoutDefaultValues(IntegrationTester $I)
             $robot->delete()
         );
     }
+
+    public function mvcModelSaveCircularRelation(IntegrationTester $I)
+    {
+        $I->wantToTest('Mvc\Model::save() with circular unsaved relations');
+
+        $album = new Albums([
+            'name' => 'Loopback'
+        ]);
+        $artist = new Artists([
+            'name' => 'Evil Robot'
+        ]);
+
+        // Assign relationship in both directions on unsaved models
+        $album->artist = $artist;
+        $artist->albums = [
+            $album
+        ];
+
+        // Save should handle the circular relation without issue
+        $I->assertTrue(
+            $artist->save()
+        );
+
+        // Both should have an ID now
+        $I->assertNotEquals(null, $album->id);
+        $I->assertNotEquals(null, $artist->id);
+    }
 }