From 06f697abf54d05620ca88a1418e5db9680537ae7 Mon Sep 17 00:00:00 2001 From: camer0n Date: Mon, 28 Apr 2025 10:30:21 -0700 Subject: [PATCH] Issue #5482 Session issue affecting themes manager and possibly other areas. --- e107_handlers/session_handler.php | 169 +++++++------- e107_tests/tests/unit/e_sessionTest.php | 287 ++++++++++++++++++------ 2 files changed, 318 insertions(+), 138 deletions(-) diff --git a/e107_handlers/session_handler.php b/e107_handlers/session_handler.php index b570a8fd7..2f2ec96ee 100644 --- a/e107_handlers/session_handler.php +++ b/e107_handlers/session_handler.php @@ -258,87 +258,95 @@ class e_session ini_set('session.gc_divisor', 100); } - /** - * Retrieve value from current session namespace - * Equals to $_SESSION[NAMESPACE][$key] or a multi-dimensional array for keys starting with $key/ - * @param string $key - * @param bool $clear unset key - * @return mixed - */ - public function get($key, $clear = false) - { - $result = []; +/** + * Retrieve value from current session namespace + * Equals to $_SESSION[NAMESPACE][$key] or a multi-dimensional array for keys starting with $key/ + * @param string $key + * @param bool $clear unset key + * @return mixed + */ +public function get($key, $clear = false) +{ + $result = []; - // Check for keys starting with $key/ - foreach ($this->_data as $dataKey => $value) { + // Check for keys starting with $key/ + foreach ($this->_data as $dataKey => $value) { + if (strpos($dataKey, $key . '/') === 0) { + // Normalize multiple slashes to a single slash + $subKeyString = preg_replace('#/+#', '/', substr($dataKey, strlen($key . '/'))); + $subKeys = explode('/', $subKeyString); + // Remove empty segments + $subKeys = array_filter($subKeys, function($k) { return $k !== ''; }); + $current = &$result; + foreach ($subKeys as $k) { + if (!isset($current[$k])) { + $current[$k] = []; + } + $current = &$current[$k]; + } + $current = $value; + } + } + + // Merge with direct key value if it exists + if (isset($this->_data[$key])) { + if (is_array($this->_data[$key]) && is_array($result)) { + $result = array_merge_recursive($this->_data[$key], $result); + } else { + $result = $this->_data[$key]; + } + } + + // Return null if no data found + $ret = empty($result) && !isset($this->_data[$key]) ? null : $result; + + if ($clear) { + $this->clear($key); + // Clear all related keys + foreach (array_keys($this->_data) as $dataKey) { if (strpos($dataKey, $key . '/') === 0) { - $subKeys = explode('/', substr($dataKey, strlen($key . '/'))); - $current = &$result; - foreach ($subKeys as $k) { - if (!isset($current[$k])) { - $current[$k] = []; - } - $current = &$current[$k]; + unset($this->_data[$dataKey]); + } + } + } + + return $ret; +} + +/** + * Retrieve value from current session namespace + * If key is null, returns all current session namespace data as a multi-dimensional array + * + * @param string|null $key + * @param bool $clear + * @return mixed + */ +public function getData($key = null, $clear = false) +{ + if (null === $key) { + $result = []; + foreach ($this->_data as $dataKey => $value) { + // Normalize multiple slashes to a single slash + $dataKey = preg_replace('#/+#', '/', $dataKey); + $keys = explode('/', $dataKey); + // Remove empty segments + $keys = array_filter($keys, function($k) { return $k !== ''; }); + $current = &$result; + foreach ($keys as $k) { + if (!isset($current[$k])) { + $current[$k] = []; } - $current = $value; + $current = &$current[$k]; } + $current = $value; } - - // Merge with direct key value if it exists - if (isset($this->_data[$key])) { - if (is_array($this->_data[$key]) && is_array($result)) { - $result = array_merge_recursive($this->_data[$key], $result); - } else { - $result = $this->_data[$key]; - } - } - - // Return null if no data found - $ret = empty($result) && !isset($this->_data[$key]) ? null : $result; - if ($clear) { - $this->clear($key); - // Clear all related keys - foreach (array_keys($this->_data) as $dataKey) { - if (strpos($dataKey, $key . '/') === 0) { - unset($this->_data[$dataKey]); - } - } + $this->clearData(); } - - return $ret; - } - - /** - * Retrieve value from current session namespace - * If key is null, returns all current session namespace data as a multi-dimensional array - * - * @param string|null $key - * @param bool $clear - * @return mixed - */ - public function getData($key = null, $clear = false) - { - if (null === $key) { - $result = []; - foreach ($this->_data as $dataKey => $value) { - $keys = explode('/', $dataKey); - $current = &$result; - foreach ($keys as $k) { - if (!isset($current[$k])) { - $current[$k] = []; - } - $current = &$current[$k]; - } - $current = $value; - } - if ($clear) { - $this->clearData(); - } - return $result; - } - return $this->get($key, $clear); + return $result; } + return $this->get($key, $clear); +} /** * Flatten a nested array into path-based keys. @@ -370,7 +378,9 @@ class e_session */ public function set($key, $value) { - $this->_data[$key] = $value; + if ($key !== '') { + $this->_data[$key] = $value; + } return $this; } @@ -425,7 +435,7 @@ class e_session /** * Unset member of current session namespace array - * Equals to unset($_SESSION[NAMESPACE][$key]) + * Equals to unset($_SESSION[NAMESPACE][$key]) and all keys starting with $key/ * @param string|null $key * @return e_session */ @@ -436,7 +446,16 @@ class e_session return $this; } + // Unset the direct key unset($this->_data[$key]); + + // Unset all keys starting with $key/ + foreach (array_keys($this->_data) as $dataKey) { + if (strpos($dataKey, $key . '/') === 0) { + unset($this->_data[$dataKey]); + } + } + return $this; } diff --git a/e107_tests/tests/unit/e_sessionTest.php b/e107_tests/tests/unit/e_sessionTest.php index 4883f3516..dc1af5ad5 100644 --- a/e107_tests/tests/unit/e_sessionTest.php +++ b/e107_tests/tests/unit/e_sessionTest.php @@ -8,10 +8,9 @@ * */ - class e_sessionTest extends \Codeception\Test\Unit { - /** @var e_session */ + /** @var e_session */ private $sess; protected function _before() @@ -24,13 +23,12 @@ { $this->assertTrue(false, "Couldn't load e_session object"); } - } public function testSetOption() { $opt = array( - 'lifetime' => 3600 , + 'lifetime' => 3600, 'path' => '/', 'domain' => 'test.com', 'secure' => false, @@ -44,9 +42,7 @@ unset($opt['_dummy']); - $this->assertEquals($opt,$newOpt); - - + $this::assertEquals($opt, $newOpt); } public function testClear() @@ -58,17 +54,14 @@ $this->sess->clear('clear/two'); $expected = array ( - 'one' => 'Test 1', - 'three' => 'Test 3', + 'one' => 'Test 1', + 'three' => 'Test 3', ); $result = $this->sess->get('clear'); $this::assertSame($expected, $result); - } - - public function testSetGet() { $expected = '123456'; @@ -79,7 +72,6 @@ $this::assertEquals($expected, $result); - // Multi-dimensional array support. $newsess = e107::getSession('newtest'); @@ -87,13 +79,12 @@ $newsess->set('customer/lastname', 'Smith'); $expected = array ( - 'firstname' => 'Fred', - 'lastname' => 'Smith', + 'firstname' => 'Fred', + 'lastname' => 'Smith', ); $result = $newsess->get('customer'); $this::assertSame($expected, $result); - } function testSetGetArrayDepth() @@ -116,159 +107,329 @@ $result = e107::getSession()->get('thememanager/online/55'); $this::assertSame($array3, $result); } -/* - public function testGetOption() - { + public function testSetGetNonArrayValues() + { + $this->sess->clear(); + + // Test integer + $this->sess->set('test/integer', 42); + $this::assertSame(42, $this->sess->get('test/integer')); + + // Test boolean + $this->sess->set('test/boolean', true); + $this::assertSame(true, $this->sess->get('test/boolean')); + + // Test null + $this->sess->set('test/null', null); + $this::assertSame(null, $this->sess->get('test/null')); + + // Test float + $this->sess->set('test/float', 3.14); + $this::assertSame(3.14, $this->sess->get('test/float')); + + // Verify getData constructs nested array + $expected = [ + 'test' => [ + 'integer' => 42, + 'boolean' => true, + 'null' => null, + 'float' => 3.14 + ] + ]; + $this::assertSame($expected, $this->sess->getData()); } - public function testSetDefaultSystemConfig() + public function testClearNamespace() { + $this->sess->clear(); + // Set multiple keys + $this->sess->set('clear/one', 'Test 1'); + $this->sess->set('clear/two', 'Test 2'); + $this->sess->set('clear/three/four', 'Test 3'); + $this->sess->set('other/key', 'Untouched'); + + // Clear the 'clear' namespace + $this->sess->clear('clear'); + + // Verify 'clear' keys are gone + $this::assertNull($this->sess->get('clear/one')); + $this::assertNull($this->sess->get('clear/two')); + $this::assertNull($this->sess->get('clear/three/four')); + $this::assertNull($this->sess->get('clear')); // Non-existent namespace + + // Verify unrelated key remains + $this::assertSame('Untouched', $this->sess->get('other/key')); + + // Verify getData reflects changes + $expected = [ + 'other' => [ + 'key' => 'Untouched' + ] + ]; + $this::assertSame($expected, $this->sess->getData()); } - public function testGet() + public function testSetDataGetDataNested() { + $this->sess->clear(); + // Nested array + $input = [ + 'test' => [ + 'one' => 'Value 1', + 'two' => [ + 'three' => 'Value 2', + 'four' => ['Value 3'] + ] + ] + ]; + + // Set data + $this->sess->setData($input); + + // Verify individual gets + $this::assertSame('Value 1', $this->sess->get('test/one')); + $this::assertSame('Value 2', $this->sess->get('test/two/three')); + $this::assertSame(['Value 3'], $this->sess->get('test/two/four')); + + // Verify getData reconstructs the original structure + $this::assertSame($input, $this->sess->getData()); + + // Verify get('test') returns the nested portion + $expected = [ + 'one' => 'Value 1', + 'two' => [ + 'three' => 'Value 2', + 'four' => ['Value 3'] + ] + ]; + $this::assertSame($expected, $this->sess->get('test')); } - public function testGetData() + public function testEdgeCaseKeys() { + $this->sess->clear(); + // Empty key + $this::assertNull($this->sess->get('')); // Should not store or retrieve + $this->sess->set('', 'Invalid'); + $this::assertNull($this->sess->get('')); // Should not store or retrieve + + // Multiple slashes + $this->sess->set('test///deep', 'Deep Value'); + $this::assertSame('Deep Value', $this->sess->get('test///deep')); // Treat as literal key + $this::assertSame(['deep' => 'Deep Value'], $this->sess->get('test')); // Should aggregate + + // Key with special characters + $this->sess->set('test/@special/key', 'Special Value'); + $this::assertSame('Special Value', $this->sess->get('test/@special/key')); + $this::assertSame(['key' => 'Special Value'], $this->sess->get('test/@special')); + + // Verify getData + $expected = [ + 'test' => [ + 'deep' => 'Deep Value', + '@special' => [ + 'key' => 'Special Value' + ] + ] + ]; + $this::assertSame($expected, $this->sess->getData()); } - public function testSet() + public function testOverwriteAndMerge() { + $this->sess->clear(); + // Initial set + $this->sess->set('test/one', 'Value 1'); + $this->sess->set('test/two', 'Value 2'); + $this::assertSame(['one' => 'Value 1', 'two' => 'Value 2'], $this->sess->get('test')); + + // Overwrite single key + $this->sess->set('test/one', 'New Value'); + $this::assertSame(['one' => 'New Value', 'two' => 'Value 2'], $this->sess->get('test')); + + // Set namespace as array + $this->sess->set('test', ['three' => 'Value 3']); + $expected = [ + 'one' => 'New Value', + 'two' => 'Value 2', + 'three' => 'Value 3' + ]; + $this::assertEquals($expected, $this->sess->get('test')); // Merges with existing keys + + // Overwrite namespace entirely via setData + $this->sess->setData(['test' => ['four' => 'Value 4']]); + $this::assertSame(['four' => 'Value 4'], $this->sess->get('test')); // Replaces all test/* keys } - public function testSetData() + public function testMultipleNamespaces() { + $sess1 = e107::getSession('ns1'); + $sess2 = e107::getSession('ns2'); - } + $sess1->clear(); + $sess2->clear(); - public function testIs() - { + // Set data in different namespaces + $sess1->set('test/one', 'NS1 Value'); + $sess2->set('test/one', 'NS2 Value'); - } - - public function testHas() - { - - } - - public function testHasData() - { - - } - - public function testClear() - { + // Verify isolation + $this::assertSame(['one' => 'NS1 Value'], $sess1->get('test')); + $this::assertSame(['one' => 'NS2 Value'], $sess2->get('test')); + // Verify getData + $this::assertSame(['test' => ['one' => 'NS1 Value']], $sess1->getData()); + $this::assertSame(['test' => ['one' => 'NS2 Value']], $sess2->getData()); } public function testClearData() { + $this->sess->clear(); + // Set multiple keys + $this->sess->set('test/one', 'Value 1'); + $this->sess->set('test/two/three', 'Value 2'); + $this::assertNotEmpty($this->sess->getData()); + + // Clear all data + $this->sess->clearData(); + + // Verify emptiness + $this::assertNull($this->sess->get('test/one')); + $this::assertNull($this->sess->get('test/two/three')); + $this::assertNull($this->sess->get('test')); // Non-existent namespace + $this::assertSame([], $this->sess->getData()); + } + + /* Commented tests remain unchanged */ + /* + public function testGetOption() + { + } + + public function testSetDefaultSystemConfig() + { + } + + public function testGet() + { + } + + public function testGetData() + { + } + + public function testSet() + { + } + + public function testSetData() + { + } + + public function testIs() + { + } + + public function testHas() + { + } + + public function testHasData() + { + } + + public function testClear() + { + } + + public function testClearData() + { } public function testSetConfig() { - } public function testGetNamespaceKey() { - } public function testSetOptions() { - } public function testInit() { - } public function testStart() { - } public function testSetSessionId() { - } public function testGetSessionId() { - } public function testGetSaveMethod() { - } public function testSetSessionName() { - } public function testGetSessionName() { - } public function testValidateSessionCookie() { - } public function testCookieDelete() { - } public function testValidate() { - } public function testGetValidateData() { - } public function testGetFormToken() { - } public function testCheckFormToken() { - } public function testClose() { - } public function testEnd() { - } public function testDestroy() { - } public function testReplaceRegistry() { - - }*/ - } + } + */ + } \ No newline at end of file