When switching between blogs, wp_upload_dir 'baseurl' and 'url' may be pointing to the current blog not the switched one

[igmoweb]

Only tested on subfolder multisite (I don't have a way to test on a subdomain install right now).

It seems very similar to ticket #23483 but I'm not sure if it should be treated the same:

Example with two blogs in a network:

• www.mydomain.com/my-source-blog (ID:5)
• www.mydomain.com/my-destination-blog (ID:15)

I discover this while trying to move a post with images from a blog to another. The post is copied and images inside the post are copied too.

Starting point: The current blog ID is 15 (my-destination-blog). I'm trying to get upload URLs from the current blog:

switch_to_blog( 5 );
$source_upload_dir = wp_upload_dir();
$source_upload_baseurl = $source_upload_dir['baseurl'];
restore_current_blog();

$destination_upload_dir = wp_upload_dir();
$destination_upload_baseurl = $destination_upload_dir ['baseurl'];

At this point, $source_upload_baseurl is:

http://www.mydomain.com/my-destination-blog/wp-content/uploads/sites/5

Where should be:

http://www.mydomain.com/my-source-blog/wp-content/uploads/sites/5

$destination_upload_baseurl is fine.

It seems that if get_option( 'upload_url_path' ) returns false, wp_upload_dir() makes use of WP_CONTENT_URL constant that is set to the current blog URL at the beggining of the execution (in default-constants.php):

define( 'WP_CONTENT_URL', get_option('siteurl') . '/wp-content');

This value obviously does not change when switching a blog.

[Denis-de-Bernardy]

@nacin: Do we want this fixed in 3.9?

[igmoweb]

I'm not completely sure but in case it helps, I think a fix would be replace current 1578 line in wp-includes/functions.php:

$url = WP_CONTENT_URL . '/uploads';

For something like

$url = get_option('siteurl') . '/wp-content/uploads';

But the problem now would be that the user could not change the content url by himself using WP_CONTENT_URL constant. Or at least, new issues would come, I guess.

[Denis-de-Bernardy]

Replying to igmoweb:

But the problem now would be that the user could not change the content url by himself using WP_CONTENT_URL constant. Or at least, new issues would come, I guess.

Yeah, it would likely be a regression. :-)

Something along the lines of filtering the uploads url might work, though (see #23221).

[pavelevap]

I probably ran into the same issue with subdomain install, but I am not sure...

Main site is on mydomain, secondary site is on subdomain.mydomain. After switch_to_blog(2) and calling the_post_thumbnail() there is problem with image URL on main site:

http://mydomain/wp-content/uploads/sites/2/2014/10/picture.jpg (does not exist)

But it should be:

http://subdomain.mydomain/wp-content/uploads/sites/2/2014/10/picture.jpg

[yo-l1982]

#30209 was marked as a duplicate.

[flynsarmy]

Here's a workaround I'm using until this is fixed:

add_action('switch_blog', function($new_blog, $prev_blog_id) {
	update_option( 'upload_url_path',  get_option('siteurl') . '/wp-content/uploads');
}, 10, 2);

[chriscct7]

Bug Scrub 1/8/2015 notes:

• The suggestion was made to introduce a function for which the current content directory can be retrieved at any time, thus solving the switch_to_blog bug. The suggested title for this function, which would replace the use of the constant of the same name is wp_content_dir()

[igmoweb]

First approach for a solution

[igmoweb]

Hi, I attached a patch for a first approach to the problem ( 25650_approach_2.patch, forget about the first one). This was what I had in mind for this. Obviously, this is not fully working but it fixes the main problem.

So the idea is to stop trusting WP_CONTENT_URL constant and add a global $wp_content_url (maybe a static one is better, not sure right now). The global is actually based on the constant so the user can still define it in some way. When a switch between blogs is made we can use this global to change the content URL. Right now, I'm just replacing the sites URLs, what can be bad but it's a start.

A dev feedback would be handy in here.

[desrosj]

#40072 was marked as a duplicate.

[ideag]

approach 3

[ideag]

I took a stab at this during WordCamp Europe 2022 Contributors day. I did the following things:

1. replaced the use of WP_CONTENT_URL in _wp_upload_dir() with a call to content_url()
2. added additional logic to content_url() method to check if switch_to_blog() has been used and if the WP_CONTENT_URL has been set in the default-constants.php file, rather than explicitly in wp-config.php
3. added an indication to default-constants.php to signify that the WP_CONTENT_URL wss set by WP itself rather than explicitly in wp-config.php. I'm achieving that by defining an additional constant, WP_DYNAMIC_CONTENT_URL. I don't entirely like this part, but I think it was the best compromise in terms of readability, reliability, performance and testability that I could find.
4. updated tests to test for both the explicit WP_CONTENT_URL setting and the dynamic WP_CONTENT_URL setting.

[ideag]

I had another idea to identify if WP_CONTENT_URL was explicitly set in wp-config.php. It goes along these lines (at the top of content_url() method):

$url = WP_CONTENT_URL;
if ( ms_is_switched() ) {
    $original_blog_id = $GLOBALS['_wp_switched_stack'][0];
    $dynamic_content_url = get_blog_option( $original_blog_id, 'siteurl' ) . '/wp-content';
    if ( WP_CONTENT_URL === $dynamic_content_url ) {
        $url = get_option( 'siteurl' ) . '/wp-content';
    }
}

The upside for this it allows to avoids creating a new constant in the step 3 of the above patch.
The downside is this adds an additional loop of switch_to_blog() inside the get_blog_option() method and an additional call to get_option(), so it should have a bigger performance fingerprint, it uses a global variable and is in general harder to read and understand and I'm not sure I can write a test that would not rely on the same code we are testing.
Also, most significantly, this would fail if someone explicitly set their WP_CONTENT_URL to the upload url of the main site. Can't find a way around that yet.

[ideag]

The above patch only affects the behavior of _wp_upload_dir() function in one specific case:

1. upload_url_path option is not explicitly set for a site;
2. At least one of these are true:
   1. upload_path option is not set;
   2. upload_path option is set to 'wp-content/uploads' (the default for that option);
   3. upload_path option value is equal to ABSPATH (which I don't think is really feasible ?)

In that specific case I'm replacing WP_CONTENT_URL constant with a call to content_url(), which (before patching) always resolves back to the WP_CONTENT_URL constant, but additionally sets the url scheme with set_url_scheme() and is filterable via the content_url filter hook.

This patch further changes the behaviour of content_url() in one specific case. On multisite, when switch_to_blog() is called and was WP_CONTENT_URL NOT explicitly defined in wp-config.php, it does not rely on the constant (which was set back when we have not switched to a different blog yet and does not make sense anymore), but calls get_option('siteurl') for the current blog directly.

Behaviour of content_url() in different scenarios:

No	Multisite?	ms_is_switched()	WP_CONTENT_URL	Current behaviour	Patched behaviour
1	No	No	Explicitly defined in wp-config.php	WP_CONTENT_URL	WP_CONTENT_URL
2	No	No	Dynamically defined in default-constants.php	WP_CONTENT_URL which resolves to get_option( 'siteurl' ) . '/wp-content'	WP_CONTENT_URL which resolves to get_option( 'siteurl' ) . '/wp-content'
3	Yes	No	Explicitly defined in wp-config.php	WP_CONTENT_URL	WP_CONTENT_URL
4	Yes	No	Dynamically defined in default-constants.php	WP_CONTENT_URL which resolves to get_option( 'siteurl' ) . '/wp-content'	WP_CONTENT_URL which resolves to get_option( 'siteurl' ) . '/wp-content'
5	Yes	Yes	Explicitly defined in wp-config.php	WP_CONTENT_URL	WP_CONTENT_URL
6	Yes	Yes	Dynamically defined in default-constants.php	WP_CONTENT_URL which resolves to get_option( 'siteurl' ) . '/wp-content' where get_option() gets data from the original site	get_option( 'siteurl' ) . '/wp-content' where get_option() gets data from the switched-to site

I think it would be OK to stop relying on the constant in all cases where it is set dynamically in the default-constants.php file as it does resolve to the same get_option() call anyway, but I was trying to limit this patch as much as possible to just fix the issue at hand. If you think it is wise to make it broader, I can expand the patch to cove those cases as well.

Trac ticket: https://core.trac.wordpress.org/ticket/25650

[ideag]

#49335 was marked as a duplicate.

[Ankit K Gupta]

This ticket was discussed in this week's test-scrub session for WP 6.7, and we have approximately two weeks for the Beta 1 release. Any details on testing this ticket would be greatly appreciated!