Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#29095 closed defect (bug) (fixed)

The get/set_user_setting fails on multisite/network with sub-domains

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords:
Focuses: Cc:

Description

When both the main site (network.domain) and a particular site (site.network.domain) set the wp-settings-123 cookie for a user, both cookies are send by the browsers when that user visits the sub-domain site. This results in one cookie overloading the other in the $_COOKIE array, as both have the same name.

The end result is that user settings like default editor, image size and link when inserting in a post, menu folded/unfolded, etc. are not set properly on page load.

Attachments (1)

29095.patch (12.2 KB) - added by azaozz 10 years ago.

Download all attachments as: .zip

Change History (13)

@azaozz
10 years ago

#1 @azaozz
10 years ago

29095.patch fixes this by appending the current blog id to the cookie name. The users won't notice any changes. The previous settings will be added to the new cookie at the first run after updating. The old cookie will be deleted when the user changes a setting (or will expire).

#2 @azaozz
10 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 29362:

Add blog_id to the wp-settings-* cookie (used for storing user state) to prevent it being overloaded on sub-domain sites. Fixes #29095.

#3 @azaozz
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Ugh, this was actually broken by [28895], more specifically by adding COOKIE_DOMAIN to the 'wp-settings-' . $user_id cookie...

Reference:

"... However, there is a difference between a cookie set from foo.com without a domain, and a cookie set with the foo.com domain. In the former case, the cookie will only be sent for requests to foo.com. In the latter case, all sub domains are also included."

Some of this needs to be reverted.

Last edited 10 years ago by azaozz (previous) (diff)

#4 @azaozz
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 29478:

Fix the wp-settings-* cookies used in getUserSetting()/setUserSetting(). They should be set without COOKIE_DOMAIN to work properly for sub-domains. Fixes #29095.

#5 @nacin
10 years ago

So how is this now site-specific again?

Re-opening for further review as there is a lot of churn here at a fairly low level.

#6 @helen
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @azaozz
10 years ago

[29478] reverts most of [29362] keeping only whitespace/braces changes and adds secure when setting the cookie from JS. The actual fix is removing COOKIE_DOMAIN from the 'wp-settings-' . $user_id cookies when setting them from PHP. That was added in [28895] probably by mistake, and I missed it the first time so [29362] didn't really fix it well.

When a cookie is set with a domain, the browsers send it to that domain and to all sub-domains. When it is set without a domain, it is sent only to the current domain, not to any sub-domains. So if we set a cookie from core.trac.wordpress.org with a domain of wordpress.org, it will be sent to all other sub-domains. However if we set the same cookie without a domain, it will only be sent to the current site.

The same is true for the "root" domain. A cookie set on wordpress.org with a domain of wordpress.org will be sent to all sub-domains. The same cookie set without a domain will be sent only to wordpress.org. Some info and links on Wikipedia.

#8 follow-up: @nacin
10 years ago

So to be clear, this was all about restoring 3.9 code?

#9 in reply to: ↑ 8 @azaozz
10 years ago

Replying to nacin:

Pretty much. The only difference is passing the secure flag to JS so the cookie can be set from there in exactly the same way as from PHP.

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.


10 years ago

#11 @helen
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Appears as though this is fixed.

This ticket was mentioned in Slack in #core-multisite by richardtape. View the logs.


8 years ago

Note: See TracTickets for help on using tickets.