From 302f2f177dc1f6d920ddd8a3a23d0f8455897386 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 14 May 2023 10:34:21 +0900 Subject: [PATCH 1/5] refactor: do not reassign $key --- system/Entity/Entity.php | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/system/Entity/Entity.php b/system/Entity/Entity.php index 92cedeff21..128f5c3c81 100644 --- a/system/Entity/Entity.php +++ b/system/Entity/Entity.php @@ -259,19 +259,19 @@ class Entity implements JsonSerializable return $this->original !== $this->attributes; } - $key = $this->mapProperty($key); + $dbColumn = $this->mapProperty($key); // Key doesn't exist in either - if (! array_key_exists($key, $this->original) && ! array_key_exists($key, $this->attributes)) { + if (! array_key_exists($dbColumn, $this->original) && ! array_key_exists($dbColumn, $this->attributes)) { return false; } // It's a new element - if (! array_key_exists($key, $this->original) && array_key_exists($key, $this->attributes)) { + if (! array_key_exists($dbColumn, $this->original) && array_key_exists($dbColumn, $this->attributes)) { return true; } - return $this->original[$key] !== $this->attributes[$key]; + return $this->original[$dbColumn] !== $this->attributes[$dbColumn]; } /** @@ -435,19 +435,19 @@ class Entity implements JsonSerializable */ public function __set(string $key, $value = null) { - $key = $this->mapProperty($key); + $dbColumn = $this->mapProperty($key); // Check if the field should be mutated into a date - if (in_array($key, $this->dates, true)) { + if (in_array($dbColumn, $this->dates, true)) { $value = $this->mutateDate($value); } - $value = $this->castAs($value, $key, 'set'); + $value = $this->castAs($value, $dbColumn, 'set'); // if a set* method exists for this key, use that method to // insert this value. should be outside $isNullable check, // so maybe wants to do sth with null value automatically - $method = 'set' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $key))); + $method = 'set' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $dbColumn))); if (method_exists($this, $method)) { $this->{$method}($value); @@ -459,7 +459,7 @@ class Entity implements JsonSerializable // class properties that are undefined, though they cannot be // saved. Useful for grabbing values through joins, assigning // relationships, etc. - $this->attributes[$key] = $value; + $this->attributes[$dbColumn] = $value; return $this; } @@ -480,12 +480,12 @@ class Entity implements JsonSerializable */ public function __get(string $key) { - $key = $this->mapProperty($key); + $dbColumn = $this->mapProperty($key); $result = null; // Convert to CamelCase for the method - $method = 'get' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $key))); + $method = 'get' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $dbColumn))); // if a get* method exists for this key, // use that method to insert this value. @@ -495,17 +495,17 @@ class Entity implements JsonSerializable // Otherwise return the protected property // if it exists. - elseif (array_key_exists($key, $this->attributes)) { - $result = $this->attributes[$key]; + elseif (array_key_exists($dbColumn, $this->attributes)) { + $result = $this->attributes[$dbColumn]; } // Do we need to mutate this into a date? - if (in_array($key, $this->dates, true)) { + if (in_array($dbColumn, $this->dates, true)) { $result = $this->mutateDate($result); } // Or cast it as something? elseif ($this->_cast) { - $result = $this->castAs($result, $key); + $result = $this->castAs($result, $dbColumn); } return $result; @@ -521,15 +521,15 @@ class Entity implements JsonSerializable return false; } - $key = $this->mapProperty($key); + $dbColumn = $this->mapProperty($key); - $method = 'get' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $key))); + $method = 'get' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $dbColumn))); if (method_exists($this, $method)) { return true; } - return isset($this->attributes[$key]); + return isset($this->attributes[$dbColumn]); } /** @@ -541,9 +541,9 @@ class Entity implements JsonSerializable return; } - $key = $this->mapProperty($key); + $dbColumn = $this->mapProperty($key); - unset($this->attributes[$key]); + unset($this->attributes[$dbColumn]); } /** From af0c2f174c9b6eccd5b46b273c218064aaab24ef Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 14 May 2023 10:54:00 +0900 Subject: [PATCH 2/5] refactor: rename variable name --- system/Entity/Entity.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/Entity/Entity.php b/system/Entity/Entity.php index 128f5c3c81..61a54e4c4e 100644 --- a/system/Entity/Entity.php +++ b/system/Entity/Entity.php @@ -551,10 +551,10 @@ class Entity implements JsonSerializable */ protected function isMappedDbColumn(string $key): bool { - $maybeColumnName = $this->mapProperty($key); + $dbColumn = $this->mapProperty($key); // Property name which has mapped column name - if ($key !== $maybeColumnName) { + if ($key !== $dbColumn) { return false; } From 9b7409986a8c824354a0a0b45fe7b7836dd6e14a Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 14 May 2023 10:56:33 +0900 Subject: [PATCH 3/5] docs: improve comment --- system/Entity/Entity.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Entity/Entity.php b/system/Entity/Entity.php index 61a54e4c4e..4247c82b66 100644 --- a/system/Entity/Entity.php +++ b/system/Entity/Entity.php @@ -553,7 +553,7 @@ class Entity implements JsonSerializable { $dbColumn = $this->mapProperty($key); - // Property name which has mapped column name + // The $key is a property name which has mapped db column name if ($key !== $dbColumn) { return false; } From 2f405da4f4ac2daa3a736866c1ade6f436437145 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 May 2023 11:00:54 +0900 Subject: [PATCH 4/5] fix: Session::stop() does not destory the session --- system/Session/Session.php | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/system/Session/Session.php b/system/Session/Session.php index 497111b600..175a275f3e 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -272,21 +272,13 @@ class Session implements SessionInterface } /** - * Does a full stop of the session: + * Destroys the current session. * - * - destroys the session - * - unsets the session id - * - destroys the session cookie + * @deprecated Use destroy() instead. */ public function stop() { - setcookie( - $this->sessionCookieName, - session_id(), - ['expires' => 1, 'path' => $this->cookie->getPath(), 'domain' => $this->cookie->getDomain(), 'secure' => $this->cookie->isSecure(), 'httponly' => true] - ); - - session_regenerate_id(true); + $this->destroy(); } /** From 4baf5ab2d1c0d63370d461a6574c6be499ba50bb Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 17 May 2023 11:02:40 +0900 Subject: [PATCH 5/5] docs: update docs --- user_guide_src/source/changelogs/v4.3.5.rst | 9 ++++++++ .../source/installation/upgrade_435.rst | 12 ++++++++++ user_guide_src/source/libraries/sessions.rst | 22 +++++++++++++++---- .../source/libraries/sessions/038.php | 3 --- 4 files changed, 39 insertions(+), 7 deletions(-) delete mode 100644 user_guide_src/source/libraries/sessions/038.php diff --git a/user_guide_src/source/changelogs/v4.3.5.rst b/user_guide_src/source/changelogs/v4.3.5.rst index 7ff5f2e4b5..76b348fd2c 100644 --- a/user_guide_src/source/changelogs/v4.3.5.rst +++ b/user_guide_src/source/changelogs/v4.3.5.rst @@ -9,6 +9,12 @@ Release Date: Unreleased :local: :depth: 3 +SECURITY +******** + +- Fixed that ``Session::stop()`` did not destroy the session. + See :ref:`Session Library ` for details. + BREAKING ******** @@ -21,6 +27,9 @@ Changes Deprecations ************ +- **Session:** The :ref:`Session::stop() ` method is deprecated. + Use the :ref:`Session::destroy() ` instead. + Bugs Fixed ********** diff --git a/user_guide_src/source/installation/upgrade_435.rst b/user_guide_src/source/installation/upgrade_435.rst index e594f5994f..142e7318cd 100644 --- a/user_guide_src/source/installation/upgrade_435.rst +++ b/user_guide_src/source/installation/upgrade_435.rst @@ -18,6 +18,18 @@ Mandatory File Changes Breaking Changes **************** +Session::stop() +=============== + +Prior to v4.3.5, the ``Session::stop()`` method did not destroy the session due +to a bug. This method has been modified to destroy the session, and now deprecated +because it is exactly the same as the ``Session::destroy()`` method. So use the +:ref:`Session::destroy ` method instead. + +If you have code to depend on the bug, replace it with ``session_regenerate_id(true)``. + +See also :ref:`Session Library `. + Breaking Enhancements ********************* diff --git a/user_guide_src/source/libraries/sessions.rst b/user_guide_src/source/libraries/sessions.rst index 7f316c5692..6c6b75abe0 100644 --- a/user_guide_src/source/libraries/sessions.rst +++ b/user_guide_src/source/libraries/sessions.rst @@ -345,6 +345,11 @@ intend to reuse that same key in the same request, you'd want to use Destroying a Session ==================== +.. _session-destroy: + +destroy() +--------- + To clear the current session (for example, during a logout), you may simply use either PHP's `session_destroy() `_ function, or the library's ``destroy()`` method. Both will work in exactly the @@ -357,11 +362,20 @@ same way: tempdata) will be destroyed permanently and functions will be unusable during the same request after you destroy the session. -You may also use the ``stop()`` method to completely kill the session -by removing the old session ID, destroying all data, and destroying -the cookie that contained the session ID: +.. _session-stop: -.. literalinclude:: sessions/038.php +stop() +------ + +.. deprecated:: 4.3.5 + +The session class also has the ``stop()`` method. + +.. warning:: Prior to v4.3.5, this method did not destroy the session due to a bug. + +Starting with v4.3.5, this method has been modified to destroy the session. +However, it is deprecated because it is exactly the same as the ``destroy()`` +method. Use the ``destroy()`` method instead. Accessing Session Metadata ========================== diff --git a/user_guide_src/source/libraries/sessions/038.php b/user_guide_src/source/libraries/sessions/038.php deleted file mode 100644 index 7b43795e71..0000000000 --- a/user_guide_src/source/libraries/sessions/038.php +++ /dev/null @@ -1,3 +0,0 @@ -stop();