Opened 6 months ago

Closed 6 months ago

#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
Priority: high Milestone: 3.5
Component: Multisite Version: 3.5
Severity: critical Keywords: has-patch
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 6 months ago.
22702.2.diff (617 bytes) - added by nacin 6 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 6 months ago.

Download all attachments as: .zip

Change History (5)

nacin6 months ago

nacin6 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.

nacin6 months ago

Patch looks good and fixed the issue on allthingsd.com

Last edited 6 months ago by PeteMall (previous) (diff)
  • 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.