Inconsistent behavior in wpmu_current_site() when using persistent object cache

[gradyetc]

We're running a multisite sub-directory install, using the Networks for WordPress to serve multiple domains from one install. We're also using memcached as a persistent object cache.

Since we need to support multiple networks we do not set the DOMAIN_CURRENT_SITE or PATH_CURRENT_SITE constants as instructed by the ​network install instructions.

We've noticed some odd behavior after a fresh install, before we add additional networks due to logic in wpmu_current_site().

Excerpting the relevant code from ms-load.php:

$current_site = wp_cache_get( 'current_site', 'site-options' );
if ( $current_site )
	return $current_site;

$sites = $wpdb->get_results( "SELECT * FROM $wpdb->site" ); // usually only one site
if ( 1 == count( $sites ) ) {
	$current_site = $sites[0];
	wp_load_core_site_options( $current_site->id );
	$path = $current_site->path;
	$current_site->blog_id = $wpdb->get_var( $wpdb->prepare( "SELECT blog_id FROM $wpdb->blogs WHERE domain = %s AND path = %s", $current_site->domain, $current_site->path ) );
	$current_site = get_current_site_name( $current_site );
	if ( substr( $current_site->domain, 0, 4 ) == 'www.' )
		$current_site->cookie_domain = substr( $current_site->domain, 4 );
	wp_cache_set( 'current_site', $current_site, 'site-options' );
	return $current_site;
}

When the 'current_site' cache item exists, the $domain and $path globals are not updated to reflect the cached values before returning. This is inconsistent with behavior exhibited when cached values do not exist. The end result is that trying to access pages on non-root sites (blogs) results in a 404 since the current site (blog) is not set correctly.

We get around this by installing another network and flushing the cache (after which the 'current_site' cache item is never set).

It's a minor issue, but I would think the $path and $domain variables should be set consistently.

[nacin]

The end result is that trying to access pages on non-root sites (blogs) results in a 404 since the current site (blog) is not set correctly.

Could you explain this? I only see $domain and $path used in two places: ms_not_installed(), and during signups.

I kind of hate that it is intended for the $domain and $path variables to be global here. When fixing this bug, it would be nice to stop using these globals. For example, ms_not_installed() could receive $domain and $path as arguments instead.

[gradyetc]

The end result is that trying to access pages on non-root sites (blogs) results in a 404 since the current site (blog) is not set correctly.

Could you explain this? I only see $domain and $path used in two places: ms_not_installed(), and during signups.

Lines 66 through 80 in wp-includes/ms-settings.php.

$blogname = htmlspecialchars( substr( $_SERVER[ 'REQUEST_URI' ], strlen( $path ) ) );
if ( false !== strpos( $blogname, '/' ) )
	$blogname = substr( $blogname, 0, strpos( $blogname, '/' ) );
if ( false !== strpos( $blogname, '?' ) )
	$blogname = substr( $blogname, 0, strpos( $blogname, '?' ) );
$reserved_blognames = array( 'page', 'comments', 'blog', 'wp-admin', 'wp-includes', 'wp-content', 'files', 'feed' );
if ( $blogname != '' && ! in_array( $blogname, $reserved_blognames ) && ! is_file( $blogname ) )
	$path .= $blogname . '/';
$current_blog = wp_cache_get( 'current_blog_' . $domain . $path, 'site-options' );
if ( ! $current_blog ) {
	$current_blog = get_blog_details( array( 'domain' => $domain, 'path' => $path ), false );
	if ( $current_blog )
		wp_cache_set( 'current_blog_' . $domain . $path, $current_blog, 'site-options' );
}
unset($reserved_blognames);

It derives $blogname from $path. If it isn't set appropriately by wpmu_current_site when called on line 48, it messes up everything that tries to populate $current_blog after it.

Took me a while to trace this ... globals are fun if you like surprises. :)

[jeremyfelt]

Replying to gradyetc:

It derives $blogname from $path. If it isn't set appropriately by wpmu_current_site when called on line 48, it messes up everything that tries to populate $current_blog after it.

I'm still trying to follow. $path and $domain are set before wpmu_current_site() based on $_SERVER['REQUEST_URI'] and $_SERVER['HTTP_HOST'] (starting on ​line 25). Are they not matching what should be in the $current_blog after wpmu_current_site() is finished?

[gradyetc]

Sorry for the lack of clarity -- perhaps an example would help:

​http://example.com/
​http://example.com/site/
​http://example.com/site/about/

Where /site/ is a site in a sub-directory multisite install.

If you navigation to /site/about/, this is what happens:

Before the call to wpmu_current_site(), $path looks like this (on line 46):
/site/

Inside wpmu_current_site() under normal conditions, $path will get reset to this:
/

If this does not happen, the value for $path just before the attempts to set $current_blog starting on 74 is this:
/site/about/

Instead of "/site/". (And also, $blogname is "about" instead of "site").

Since there is no blog with that path, it winds up using the root site instead. As a side note $domain doesn't actually change in wpmu_current_site(); so the real issue here is with $path.

[jeremyfelt]

Ahh, got it now.

To clarify - this shows itself on a fresh sub-directory multisite install when DOMAIN_CURRENT_SITE and PATH_CURRENT_SITE are not defined as part of prep for a multi network install. As soon as more than one network exists, the cache is flushed and then never generated again.

[jeremyfelt]

25316.diff​ adds $path = $current_site->path; when the data is retrieved from cache, matching how $current_site is returned in other parts of wpmu_current_site().

[gradyetc]

Replying to jeremyfelt:

25316.diff​ adds $path = $current_site->path; when the data is retrieved from cache, matching how $current_site is returned in other parts of wpmu_current_site().

This works. Another approach would be not relying on wpmu_current_site() to set that global at all. You could re-set $path in wp-includes/ms-settings.php with $current_site->path after it is set on line 48.

Looking at it more closely -- there are three lines in ms-settings.php that manipulate $path (44-46), right before the call to wpmu_current_site() clobbers it completely. This confused both of us...

[nacin]

Replying to gradyetc:

Looking at it more closely -- there are three lines in ms-settings.php that manipulate $path (44-46), right before the call to wpmu_current_site() clobbers it completely. This confused both of us...

[Denis-de-Bernardy]

Probably related:

​https://github.com/kieranbarnes/memcached/commit/c8160594e8d681528769f7d36fbac57dc5cf1f78

[jeremyfelt]

I think this may be taken care of via #27003. $domain and $path are set once at the top of ms-settings.php.

@gradyetc Can you verify whether this issue is resolved in latest trunk?

[gradyetc]

Replying to jeremyfelt:

I think this may be taken care of via #27003. $domain and $path are set once at the top of ms-settings.php.

@gradyetc Can you verify whether this issue is resolved in latest trunk?

@jeremyfelt Confirmed -- this is no longer an issue in trunk. (Though I did find a related bug while confirming: https://core.trac.wordpress.org/ticket/27003#comment:38).

[jeremyfelt]

Great, thanks @gradyetc. Good catch in the other ticket! We'll have to get that in soon.

Fixed in [27359]