#32567 closed defect (bug) (fixed)
Cookies not being deleted.
Reported by: | shanee | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 4.8 | Priority: | normal |
Severity: | normal | Version: | 2.7 |
Component: | Users | Keywords: | has-patch |
Focuses: | Cc: |
Description
Hello. I noticed that a Wordpress site I had was storing 53 cookies on my computer.
Internet Explorer only allows a domain name to set 50 so this would cause issues (potentially being very hard to debug).
20 of these cookies are from Wordpress "wp-settings-" and "wp-settings-time-" cookies.
I think that these cookies should be deleted after you log out rather than persisting (as they are stored in a database anyway).
Below is a patch that would delete these on log out. (It's a change to the wp_clear_auth_cookie function.)
Index: wp-includes/pluggable.php =================================================================== --- wp-includes/pluggable.php (revision 31001) +++ wp-includes/pluggable.php (working copy) @@ -937,6 +937,8 @@ */ do_action( 'clear_auth_cookie' ); + setcookie( 'wp-settings-time-' . get_current_user_id(), ' ', time() - YEAR_IN_SECONDS, COOKIEPATH, COOKIE_DOMAIN ); + setcookie( 'wp-settings-' . get_current_user_id(), ' ', time() - YEAR_IN_SECONDS, COOKIEPATH, COOKIE_DOMAIN ); setcookie( AUTH_COOKIE, ' ', time() - YEAR_IN_SECONDS, ADMIN_COOKIE_PATH, COOKIE_DOMAIN ); setcookie( SECURE_AUTH_COOKIE, ' ', time() - YEAR_IN_SECONDS, ADMIN_COOKIE_PATH, COOKIE_DOMAIN ); setcookie( AUTH_COOKIE, ' ', time() - YEAR_IN_SECONDS, PLUGINS_COOKIE_PATH, COOKIE_DOMAIN );
I'm sorry if this is a bit messy. Additionally, sorry if this is the intended behaviour.
Kind regards,
Shanee Vanstone.
Attachments (2)
Change History (10)
#1
@
10 years ago
- Component changed from General to Options, Meta APIs
- Keywords needs-patch added
- Version changed from 4.2.2 to 2.7
#2
follow-up:
↓ 5
@
10 years ago
Hello. Thank you for your quick reply.
I have 53 cookies in total for my domain but only 20 of them are from this bug (presumably from 10 different users).
There's 3 separate installs for different languages (/de and /es for example) although they're not set up as a multisite.
We have a large Wordpress website with quite a lot of staff. I will sometimes log in with an alternate account or a staff member will log in on my computer. As the cookies last for a year, 10 different users is very possible.
If this is patched do you think that deleting a single cookie (as above) is the correct way or would it be better to loop though all cookies similar to this and clear each in turn (in case multiple were present)?
Kind regards,
Shanee Vanstone.
#3
@
8 years ago
Interested in seeing this as well as this impacts caching buckets which rely on cookies; once a user is logged out they should get cached content back from the general non-logged-in bucket, but since these unique cookies remain dangling a unique bucket is considered, hurting cache-hit rates.
#4
@
8 years ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Awaiting Review to 4.8
- Owner set to johnbillion
- Status changed from new to reviewing
#5
in reply to:
↑ 2
@
8 years ago
Replying to shanee:
If this is patched do you think that deleting a single cookie (as above) is the correct way or would it be better to loop though all cookies similar to this and clear each in turn (in case multiple were present)?
Should absolutely not mess with any of the cookies from other installations unless a logout was explicitly requested for those user sessions. Besides, once/if this behavior accepted you won't be accumulating settings cookies this way so it wouldn't be an issue.
#8
@
7 years ago
@johnbillion I'm fairly certain we have a regression with [40580]. iThemes Security Pro is now infinite looping when auth cookie becomes expired.
Specifically, their core/modules/hide-backend/class-itsec-hide-backend.php
module does this:
add_action( 'auth_cookie_expired', array( $this, 'auth_cookie_expired' ) );
and then
/** * Lets the module know that this is a reauthorization * * @since 4.1 * * @return void */ public function auth_cookie_expired() { $this->auth_cookie_expired = true; wp_clear_auth_cookie(); }
I'm fairly certain that you calling get_current_user_id()
in [40580] triggers a cookie loop. Reverting this patch immediately restores correct operation.
Your thoughts? I vote re-open and investigate.
Thanks for the report, shanee. I agree that these cookies should be cleared when the user logs out.
Do you know how you've come to have fifty of these cookies in place? Are you running multisite? Or are you switching between users on the site?