Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1 Issue with the way deleting cookies is done in Mage_Core_Model_Cookie #1958

Closed
Sdfendor opened this issue Jan 5, 2022 · 0 comments · Fixed by #1984
Closed

PHP 8.1 Issue with the way deleting cookies is done in Mage_Core_Model_Cookie #1958

Sdfendor opened this issue Jan 5, 2022 · 0 comments · Fixed by #1984

Comments

@Sdfendor
Copy link
Contributor

Sdfendor commented Jan 5, 2022

Summary (*)

There is a delete method in the cookie model (Mage_Core_Model_Cookie) where the deletion is done by setting the cookie - given by its name (in addition with other optional parameters) - to null via the set method within the same model. In this set method the built-in function setcookie is used.
Since PHP 8.1 passing null to a non-nullable parameter emits a deprecation error. The 2nd parameter value of the setcookie function has the type string and is non-nullable (php.net reference).
Regarding cookie deletion the reference states:
Cookies must be deleted with the same parameters as they were set with. If the value argument is an empty string, and all other arguments match a previous call to setcookie, then the cookie with the specified name will be deleted from the remote client. This is internally achieved by setting value to 'deleted' and expiration time in the past.

The deprecation error is only thrown and not ignored if one follows the steps described in #1812 ("disabled Suppress deprecation warnings in app/code/core/Mage/Core/functions.php"). Either way the cookie isn't deleted when calling the delete method. Using setcookie with a value of null has no effect anymore! Currently the cookie is still deleted regardless of the ignore status of deprecation errors.

The problem exists in both OM 19.4.5 or 20.0.13 since 19.4.9 in which the delete method code has been refactored to use the set method.

Examples (*)

After logging into the admin interface (adminhtml try to instantly logout by clicking the appropriate button. The Mage_Adminhtml_IndexController tries to delete the session cookie (by using the cookie model) which in turn leads to the deprecation error in the system log and the rest of the action is performed. When opening the admin login dialog again there is still the session cookie for adminhtml the action tried to delete prior.

Proposed solution

Instead of setting a cookie to null set it to empty string and for good measure directly to a point in time in the past to let it expire instantaneously. The set method in the cookie model should already support that by using value = '' and period = -1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant