From 0943b3b1a06e6c9130500532fd979c720b65c761 Mon Sep 17 00:00:00 2001
From: Loz Calver <lozcalver@bigfork.co.uk>
Date: Wed, 26 Aug 2015 10:29:37 +0100
Subject: [PATCH] FIX: Recursion errors when sorting objects with circular
 dependencies (fixes #4464)

---
 model/ArrayList.php           |  9 ++++++++-
 tests/model/ArrayListTest.php | 24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/model/ArrayList.php b/model/ArrayList.php
index 89bba7143..0b580ca50 100644
--- a/model/ArrayList.php
+++ b/model/ArrayList.php
@@ -422,10 +422,15 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
 			throw new InvalidArgumentException("Bad arguments passed to sort()");
 		}
 
+		// Store the original keys of the items as a sort fallback, so we can preserve the original order in the event
+		// that array_multisort is unable to work out a sort order for them. This also prevents array_multisort trying
+		// to inspect object properties which can result in errors with circular dependencies
+		$originalKeys = array_keys($this->items);
+
 		// This the main sorting algorithm that supports infinite sorting params
 		$multisortArgs = array();
 		$values = array();
-		foreach($columnsToSort as $column => $direction ) {
+		foreach($columnsToSort as $column => $direction) {
 			// The reason these are added to columns is of the references, otherwise when the foreach
 			// is done, all $values and $direction look the same
 			$values[$column] = array();
@@ -442,6 +447,8 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
 			$multisortArgs[] = &$sortDirection[$column];
 		}
 
+		$multisortArgs[] = &$originalKeys;
+		
 		$list = clone $this;
 		// As the last argument we pass in a reference to the items that all the sorting will be applied upon
 		$multisortArgs[] = &$list->items;
diff --git a/tests/model/ArrayListTest.php b/tests/model/ArrayListTest.php
index e0f3c74b2..5ea6de38a 100644
--- a/tests/model/ArrayListTest.php
+++ b/tests/model/ArrayListTest.php
@@ -424,6 +424,30 @@ class ArrayListTest extends SapphireTest {
 		$this->assertEquals($list->first()->ID, 1, 'Aron.2 should be first in the list');
 		$this->assertEquals($list->last()->ID, 3, 'Bert.3 should be last in the list');
 	}
+
+	/**
+	 * Check that we don't cause recursion errors with array_multisort() and circular dependencies
+	 */
+	public function testSortWithCircularDependencies() {
+		$itemA = new stdClass;
+		$childA = new stdClass;
+		$itemA->child = $childA;
+		$childA->parent = $itemA;
+		$itemA->Sort = 1;
+
+		$itemB = new stdClass;
+		$childB = new stdClass;
+		$itemB->child = $childB;
+		$childB->parent = $itemB;
+		$itemB->Sort = 1;
+
+		$items = new ArrayList;
+		$items->add($itemA);
+		$items->add($itemB);
+
+		// This call will trigger a fatal error if there are issues with circular dependencies
+		$items->sort('Sort');
+	}
 	
 	/**
 	 * $list->filter('Name', 'bob'); // only bob in the list