diff --git a/control/Session.php b/control/Session.php index a4f13b55c..e97d3ae9b 100644 --- a/control/Session.php +++ b/control/Session.php @@ -295,13 +295,20 @@ class Session { // We still want to do this even if we have strict path checking for legacy code $var = &$this->data; $diffVar = &$this->changedData; - + + // Iterate twice over the names - once to see if the value needs to be changed, + // and secondly to get the changed data value. This is done to solve a problem + // where iterating over the diff var would create empty arrays, and the value + // would then not be set, inadvertently clearing session values. foreach($names as $n) { $var = &$var[$n]; - $diffVar = &$diffVar[$n]; } if($var !== $val) { + foreach($names as $n) { + $diffVar = &$diffVar[$n]; + } + $var = $val; $diffVar = $val; } diff --git a/tests/control/SessionTest.php b/tests/control/SessionTest.php index 78debff69..31e01782b 100644 --- a/tests/control/SessionTest.php +++ b/tests/control/SessionTest.php @@ -45,6 +45,13 @@ class SessionTest extends SapphireTest { $this->assertEquals($session, array('Test' => 'Test', 'Test-2' => 'Test-2')); } + function testSettingExistingDoesntClear() { + $s = new Session(array('something' => array('does' => 'exist'))); + + $s->inst_set('something.does', 'exist'); + $this->assertEquals(array(), $s->inst_changedData()); + } + /** * Check that changedData isn't populated with junk when clearing non-existent entries. */