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!

[desrosj]

Unfortunately, this did not get the attention it deserves during the 6.7 cycle. With RC1 due out any moment, I'm going to punt.

[ideag]

Do we see any way forward with this? Any way I can help to move it along before WCEU 2025?

@desrosj @spacedmonkey - I'd really like to give this one last push, but if we do not see a way forward, it should be closed as a wontfix. I don't want to leave unfinished business here.

The ticket is 12 years old and the proposed patch is 3 years old at this point. We should either do something, or admit it is broken beyond repair.

[johnjamesjacoby]

Reviewed & refreshed @ideag's PR, by resolving conflicts that arose between 2023 and today.

---

I like the simplicity of a new WP_DYNAMIC_CONTENT_URL constant, but I think it is not ideal.

• It could be mistakenly set manually (say, via wp-config.php) introducing a new opportunity for confusion.
• It introduces a relatively unique problem-solving approach into the core codebase.
• It is only used to simplify this single comparison.

I'm also worried that changes to content_url() may have unforeseen consequences. It isn't widely used, but it is an old (2.6.0) relatively low-level function.

---

Thinking about the PR more deeply, I think it may not be exactly the right approach yet.

I've also identified some inconsistencies with where & how the related option values are set, I assume for legacy reasons, but have not confirmed...

• add_network() sets upload_path and upload_path_url to non-empty values on the root site.
• populate_options() defaults upload_path and upload_path_url to empty values.
• wp_initialize_site() sets only upload_path to a non-empty value.

At this time, I'd recommend not committing this PR as-is, and researching a different avenue.

[ideag]

@johnjamesjacoby - do you have any specific suggestions for an alternate approach?

I have explored a way to avoid introducing a new constant here: https://core.trac.wordpress.org/ticket/25650#comment:16

In my opinion, that solution would be worse than the one using a constant.

I do not see a way to avoid modifying content_url(), as that is where the actual bug is. If that is a deal breaker, then I would say this is a wontfix and should just be closed as such.

[johnjamesjacoby]

Replying to ideag:

@johnjamesjacoby - do you have any specific suggestions for an alternate approach?

Nothing concrete at the moment.

Broadly speaking, this bug seems to affect all multisite installs when switching between sites where get_option( 'upload_url_path' ) is empty — which, from what I’ve seen, includes all secondary (non-main) sites.

Perhaps _wp_upload_dir() could benefit from a more targeted override in those cases, rather than falling back to WP_CONTENT_URL?

I have explored a way to avoid introducing a new constant here: https://core.trac.wordpress.org/ticket/25650#comment:16

In my opinion, that solution would be worse than the one using a constant.

I did see that, and I totally agree with you — it’s clever, but doesn’t seem like an improvement! 😄

I do not see a way to avoid modifying content_url(), as that is where the actual bug is. If that is a deal breaker, then I would say this is a wontfix and should just be closed as such.

Are we certain that’s where the bug lives? 😅

It’s still reproducible even without calling content_url(), and using that function doesn’t seem to resolve it either.

I wouldn’t go so far as to call it a wontfix — the issue is very real and reproducible. I’m just not fully convinced this is the right fix yet.

[ideag]

Replying to johnjamesjacoby:

Are we certain that’s where the bug lives? 😅

Well, without this patch (line 6 in the table here: https://core.trac.wordpress.org/ticket/25650?replyto=32#comment:19) content_url() will use get_option('siteurl') from the *origninal* (incorrect) site, so it is part of the buggy behaviour.

If the proposed solution is to to move that logic from content_url() and move it directly into _wp_upload_dir() directly, that would still solve the bug at the title, but content_url() would still be using the incorrect WP_CONTENT_URL, which is not ideal.

[ideag]

There is a world where we stop using WP_CONTENT_URL altogether if it was not set before default-constants.php, but that would be a whole different can of worms and probably much more dangerous to mess with.