WordPress.org

Make WordPress Core

Opened 19 months ago

Closed 18 months ago

Last modified 9 months ago

#22090 closed defect (bug) (fixed)

Eliminate use of global $current_blog

Reported by: ryan Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4.2
Component: Multisite Keywords:
Focuses: Cc:

Description

$current_blog shouldn't be used outside of ms-settings.php and other core MS bootstrap files. Use of it can lead to bugs such as this one in is_main_site():

http://core.trac.wordpress.org/ticket/21459#comment:44
http://wordpress.org/support/topic/switch_to_blog-does-not-properly-set-wp_upload_dir-vars

Instead, get_blog_details( get_current_blog_id() ) should be used. get_blog_details() can default to the current blog id when not passed an argument to shorten this to get_blog_details().

Attachments (6)

22090.diff (5.7 KB) - added by ryan 19 months ago.
22090-ut.diff (1.3 KB) - added by ryan 19 months ago.
22090.2.diff (1.0 KB) - added by ryan 18 months ago.
Set $current_site inly if ! is_multisite()
22090.3.diff (1.5 KB) - added by ryan 18 months ago.
Set permalink structure and flush rules only if not multisite
22090.4.diff (1.8 KB) - added by ryan 18 months ago.
Add a comment
22090.5.diff (1.8 KB) - added by ryan 18 months ago.
Add a comment

Download all attachments as: .zip

Change History (20)

comment:1 follow-up: wonderboymusic19 months ago

btdubs - http://plugins.svn.wordpress.org/wordpress-mu-domain-mapping/trunk/domain_mapping.php

I use $current_blog in Sunrise, and I'm totally willing to stop, just passing info along.

comment:2 nacin19 months ago

Matches outside of ms-settings.php:

wp-admin/includes/class-wp-importer.php
135:			// Restore global $current_blog
136:			global $current_blog;
137:			$current_blog = $blog;
215:		global $current_blog;

wp-admin/ms-delete-site.php
75:		<p><input id="confirmdelete" type="checkbox" name="confirmdelete" value="1" /> <label for="confirmdelete"><strong><?php printf( __( "I'm sure I want to permanently disable my site, and I am aware I can never get it back or use %s again." ), is_subdomain_install() ? $current_blog->domain : $current_blog->domain . $current_blog->path ); ?></strong></label></p>

wp-admin/network/admin.php
18:$redirect_network_admin_request = ( ( $current_blog->domain != $current_site->domain ) || ( $current_blog->path != $current_site->path ) );

wp-admin/user/admin.php
19:$redirect_user_admin_request = ( ( $current_blog->domain != $current_site->domain ) || ( $current_blog->path != $current_site->path ) );

wp-includes/class-wp-xmlrpc-server.php
3663:		global $current_blog;
3664:		$domain = $current_blog->domain;
3665:		$path = $current_blog->path . 'xmlrpc.php';

wp-includes/functions.php
3121:	global $current_site, $current_blog;
3127:		$blog_id = $current_blog->blog_id;

wp-includes/ms-files.php
21:if ( $current_blog->archived == '1' || $current_blog->spam == '1' || $current_blog->deleted == '1' ) {

wp-includes/ms-load.php
72:	global $wpdb, $current_blog;
83:	if ( '1' == $current_blog->deleted ) {
90:	if ( '2' == $current_blog->deleted ) {
97:	if ( $current_blog->archived == '1' || $current_blog->spam == '1' ) {

comment:3 nacin19 months ago

Looks like XML-RPC's _multisite_getUsersBlogs() needs a bit of a rewrite to match the logic in wp_getUsersBlogs().

Otherwise, affected areas are ms_site_check(), ms-files.php, user/admin.php and network/admin.php, ms-delete-site.php, some odd $current_blog-switching action in the importer class, and is_main_site() itself. This should be fairly easy to remove.

comment:4 ryan19 months ago

I don't know if it is worth bothering with the admin.php files or ms-files.php. We'd just be polluting the global namespace with another throw away variable.

ryan19 months ago

ryan19 months ago

comment:5 in reply to: ↑ 1 ryan19 months ago

Replying to wonderboymusic:

btdubs - http://plugins.svn.wordpress.org/wordpress-mu-domain-mapping/trunk/domain_mapping.php

I use $current_blog in Sunrise, and I'm totally willing to stop, just passing info along.

Using $current_blog in sunrise to bypass the $current_site and $current_blog setup in ms-settings.php is totally legit. I think it is the one proper use of $current_blog. Regardless, $current_blog will remain forever for back compat reasons, but it will probably never be switched by switch_to_blog().

comment:6 ryan19 months ago

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

In [22108]:

Reduce use of global. Use get_blog_details() instead. fixes #22090

comment:8 nacin18 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

populate_network() resets $current_site to be an object sans blog_id. A few lines later, it calls flush_rewrite_rules(), which triggers an is_main_site() call. If you are adding a second network (so, is_multisite() is true), the result is a notice for no blog_id property.

ryan18 months ago

Set $current_site inly if ! is_multisite()

ryan18 months ago

Set permalink structure and flush rules only if not multisite

ryan18 months ago

Add a comment

ryan18 months ago

Add a comment

comment:9 ryan18 months ago

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

In [22240]:

In populate_network(), distinguish between upgrading from single to multisite and creating a new network in an existing multisite environment. When creating a new network steps related to setting up the main site must be skipped since the main site is created after populate_network() runs. Further, the global should not be modified since populating a new network does not involve switching to that network and making it current. fixes #22090

Version 0, edited 18 months ago by ryan (next)

comment:10 wdfee18 months ago

  • Cc wdfee added

don't know if it's caused by this, but using domain mapping in debug mode throws a lot of errors (using WP 3.5-beta2 by now):

Notice: Undefined property: stdClass::$blog_id in [...path...]/wp-includes/functions.php on line 3142

Because there isn't a $current_site->blog_id on a subsite.
The solution is simple: change line 3142 from

return $blog_id == $current_site->blog_id;

to

return $blog_id == $current_site->id;

it is line 3154 in current trunk version, in function is_main_site( $blog_id = )

Can you please change it and tell plugin authors (buddypress for example uses this, too), that this is to change now... Or am I somewhere wrong in the logic?

comment:11 wdfee18 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

(reopened because not sure if closed tickets are read)

If it's important to ask for the $current_site->blog_id, another suggestion for solution:

if( isset($current_site->blog_id) && $blog_id == $current_site->blog_id )
	return true;

return false;

comment:12 ryan18 months ago

id and blog_id are different ids. One is the network id and other the blog id for the main site of that network.

These warnings are new to 3.5? I'm not seeing any changes that would cause blog_id to be empty where it was set before. [22240] actually sets blog_id in one place where it wasn't set before. We could add an isset(), but that seems like it is papering over a problem in the domain mapping plugin.

comment:13 wdfee18 months ago

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

you're completely right, I'm not sure if it's related to 3.5

I looked into the ms-settings.php and added two lines to the domain mapping plugins sunrise.php (not by donncha but by incsub), same effect :-)
if anybody is searching for this, I put it here... and contact the plugin author

around line 50

if ( ! isset( $current_site->blog_id ) )
	$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 ) );
Last edited 18 months ago by wdfee (previous) (diff)

comment:14 dd329 months ago

#12993 was marked as a duplicate.

Note: See TracTickets for help on using tickets.