WordPress.org

Make WordPress Core

#22702 closed defect (bug) (fixed)

Base URL is not properly set for the main site when ms-files rewriting is enabled for MU-era installs

Reported by: nacin Owned by: ryan
Milestone: 3.5 Priority: high
Severity: critical Version: 3.5
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description

I attempted to untangle wp_upload_dir() this release via #19235. Clearly, that didn't work out too well.

Finally down to the last bug: In a MU-era install, the main site has the wrong base URL. This is as simple as stale logic. (Aren't all bugs?)

A MU-era install is defined as an install that was installed prior to the merge. As it wasn't created in a single-to-multi upgrade step, the first site uses blogs.dir, among other things. Setting this up is a total pain, but can mostly be done by taking 3.4, upgrading it to multisite, renaming the main site non-global tables to wp_1_ (if your table prefix was wp_), and removing MULTISITE (but keeping SUBDOMAIN_INSTALL). Then update to trunk. It's not perfect, but it works for this test.

So, here's the issue: In prior versions, we calculated the base URL as follows:

if ( defined('UPLOADS') && !$main_override && ( !isset( $switched ) || $switched === false ) ) {
	$dir = ABSPATH . UPLOADS;
	$url = trailingslashit( $siteurl ) . UPLOADS;
}

if ( is_multisite() && !$main_override && ( !isset( $switched ) || $switched === false ) ) {
	if ( defined( 'BLOGUPLOADDIR' ) )
		$dir = untrailingslashit(BLOGUPLOADDIR);
	$url = str_replace( UPLOADS, 'files', $url );
}

UPLOADS is always defined for multisite, so think of this code like this instead:

if ( defined('UPLOADS') && !$main_override && ( !isset( $switched ) || $switched === false ) ) {
	$dir = ABSPATH . UPLOADS;
	$url = trailingslashit( $siteurl ) . UPLOADS;

	if ( is_multisite() ) {
		if ( defined( 'BLOGUPLOADDIR' ) )
			$dir = untrailingslashit(BLOGUPLOADDIR);
		$url = str_replace( UPLOADS, 'files', $url );
	}
}

And, if you look at $url, it's silly to do an str_replace there. So $url becomes just:

$url = trailingslashit( $siteurl ) . 'files';

Got it? Good. Now, here's trunk:

if ( defined( 'UPLOADS' ) && ! ( is_multisite() && get_site_option( 'ms_files_rewriting' ) ) ) {
	$dir = ABSPATH . UPLOADS;
	$url = trailingslashit( $siteurl ) . UPLOADS;
}

Oops, in an ms-files-rewriting situation, we don't touch $dir and $url. Why? Because we need to handle it separately for two cases:

  • When ms-files-rewriting is not enabled (and if not the main site of a post-MU network!)
  • When it is enabled and we are not switched (and if not the main site of a post-MU network!)

So we do that in the subsequent block:

// If not the main site of a post-MU network!
	if ( is_multisite() && ! ( is_main_site() && defined( 'MULTISITE' ) ) ) {

        // If ms-files-rewriting is not enabled
		if ( ! get_site_option( 'ms_files_rewriting' ) ) {
                    [[ truncated, we don't care about this situation right now ]]

        // If it is not enabled and we are not switched
		} elseif ( defined( 'UPLOADS' ) && ! ms_is_switched() ) {
			if ( defined( 'BLOGUPLOADDIR' ) )
				$dir = untrailingslashit( BLOGUPLOADDIR );
			$url = trailingslashit( $siteurl ) . 'files';
		}
	}

So, what's missing? We're setting $dir and $url exactly as before, but we're missing the part where $dir and $url are already set. $dir should be ABSPATH . UPLOADS going into this block, and $url should be $siteurl . UPLOADS.

Patch attached.

Attachments (3)

22702.diff (475 bytes) - added by nacin 17 months ago.
22702.2.diff (617 bytes) - added by nacin 17 months ago.
Not as effective a patch, but this is just to illustrate what the following lines of code were expecting to have already happened.
22702.3.diff (1.0 KB) - added by nacin 17 months ago.

Download all attachments as: .zip

Change History (5)

nacin17 months ago

nacin17 months ago

Not as effective a patch, but this is just to illustrate what the following lines of code were expecting to have already happened.

nacin17 months ago

comment:1 PeteMall17 months ago

Patch looks good and fixed the issue we first noticed on allthingsd.com

Version 0, edited 17 months ago by PeteMall (next)

comment:2 ryan17 months ago

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

In 23002:

Properly set the base URL for the main site when ms-files rewriting is enabled for MU-era installs.

Props nacin
fixes #22702

Note: See TracTickets for help on using tickets.