WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#13483 closed defect (bug) (fixed)

fileupload_url / upload_url_path

Reported by: jimisaacs Owned by:
Milestone: 3.0 Priority: high
Severity: major Version: 3.0
Component: Multisite Keywords: attachments fileupload_url upload_url_path
Focuses: multisite Cc:

Description

After the latest trunk update all my attachments and post attachments were breaking in a multisite.

So which is it? I see "fileupload_url" in ms-functions.php, but the function wp_include_dir() is only checking for "upload_url_path".

So I just changed all of the "fileupload_url" options in use in my multisite to "upload_url_path" and everything was fine.

I think maybe, ms-functions.php should be using "upload_url_path" as well?

Not sure is all. I tried to search for useful tickets on this, and just found a bunch of garbage from like a year ago.

Change History (36)

comment:1 @jimisaacs5 years ago

What might have happened is that I saw "fileupload_url" in my options table from a long time ago, and the latest update missed the upload_url_path option.

Still not sure though.

comment:2 @ryan5 years ago

What version did you update from?

comment:3 @jimisaacs5 years ago

Updated to latest trunk from the latest trunk --- Version 3.0 Beta 2

The DB was originally a MU database that I imported a lot of things into the new WordPress 3.0 DB. I was very selective, but might have missed a few options.
That's why I though it might have been a leftover, but the option is still in ms-functions.php (The only place in the entire code base), I posted this ticket.

The revision I can remember exactly, I have my site in a repository, and I commit after I update from the core. If there is a place in the core files you can tell me to look, I'd be happy to find it, and look at my revision's revision.

" ... To see what revision my revision is in ... "

Jim

comment:4 @ryan5 years ago

If it worked before updating, then knowing what repository version of WP you updated from would help narrow down the changesets we have to look through.

comment:5 @ryan5 years ago

[14680] is the only recent change to wp_upload_dir().

comment:6 @jimisaacs5 years ago

Ah I found it, sorry I completely forgot it was in this file.

I did a compare on my version prior to update, and these are the differences...

Prior to update:

$wp_version = '3.0-beta2-14550';
$wp_db_version = 14217;
$manifest_version = '20100502';

After the update:

$wp_version = '3.0-beta2-14769';
$wp_db_version = 14726;
$manifest_version = '20100520';

comment:7 @jimisaacs5 years ago

I can tell you exactly what the problem is:

I am doing a switch_to_blog() loop on the home.php template of the root site, where I query for posts in each blog.

Then in use "the_post_thumbnail()" to output the featured image for each of those posts.

Before the update, I was getting the correct image paths "<host>/<site>/files/<image-path>"

After the update, I was getting this "<host>/wp-content/uploads/<image-path>"

Then I changed the option fileupload_url to upload_url_path for each of the sites in the loop, and it worked again.

comment:8 @ryan5 years ago

It could be due to the is_main_site() check introduced in [14680]. is_main_site() seems to use $current_blog->blog_id rather than $blog_id. switch_to_blog() doesn't seem to do anything with $current_blog. Either is_main_site() should use $blog_id(), or maybe wp_upload_dir() should not set $main_override if switched. Can you revert [14680] and restore your fileupload_url setting on and see if it helps?

comment:9 @ryan5 years ago

The only occurence of fileupload_url is where it is set in install_blog(). It seems to be dead. So, I guess you'll need to set upload_url_path to whatever it was before to test.

comment:10 @jimisaacs5 years ago

I just want to see if I have your comment straight:

upload_url_path path was blank before so leave if blank to test.

"switch_to_blog() doesn't seem to do anything with $current_blog. Either is_main_site() should use $blog_id(), or maybe wp_upload_dir() should not set $main_override if switched"

To me I think $current_blog should change, because why is there a function call switch_to_blog anyway?

I see method in the WP_Importer class. This method is called set_blog(), which actually calls switch_to_blog().

It seems that this method more or less should be implemented into the overall switch_to_blog() function.

Changing the wp_upload_dir() function I believe is a bandaid. Changing switch_to_blog() would make is_main_site() do what it is supposed to.

Thoughts?

comment:11 @jimisaacs5 years ago

Testing my previous suggestion. Putting the logic in WP_Importer::set_blog()

into switch_to_blog()

and blanking out upload_url_path options.

comment:12 @jimisaacs5 years ago

That's a no go on that little diddy.

While I was in there I realized why they are different logic. switch_to_blog and set_blog

What I still believe is the_post_thumbnail() should work with switch_to_blog, and I will try the second thing you mentioned with wp_upload_dir()

comment:13 @jimisaacs5 years ago

Well the problem seemed to be with this conditional in both if statements in wp_upload_dir()

if ( defined('UPLOADS') && ( !$main_override
WP_CONTENT_DIR . '/uploads' != ABSPATH . $upload_path ) && ( !isset( $switched ) $switched === true ) ) {
if ( is_multisite() && ( !$main_override
WP_CONTENT_DIR . '/uploads' != ABSPATH . $upload_path ) && ( !isset( $switched ) $switched === true ) ) {

In particular:

&& ( !isset( $switched )
$switched === false )

I took a long hard look at this I realized this is the check to see if we just ran switch_to_blog() and wondered why is was checking for "false

With this logic, what I was seeing is this

IF "'Is a multisite' AND 'not overriding the main site' AND 'NOT switched'" --- then change the url

That didn't make sense, we do want to override if switched right?
What I thought it should be is:

if ( is_multisite() && ( !$main_override
WP_CONTENT_DIR . '/uploads' != ABSPATH . $upload_path ) && ( !isset( $switched ) $switched === true ) ) {

and the final code is:

$override = (( !$main_override
WP_CONTENT_DIR . '/uploads' != ABSPATH . $upload_path ) && $switched === true);

if ( defined('UPLOADS') && $override ) {

$dir = ABSPATH . UPLOADS;
$url = trailingslashit( $siteurl ) . UPLOADS;

}

if ( is_multisite() && $override ) {

if ( defined( 'BLOGUPLOADDIR' ) )

$dir = untrailingslashit(BLOGUPLOADDIR);

$url = str_replace( UPLOADS, 'files', $url );

}

This of course worked for the problem I was having. Please let me know what this looks like... I could be completely wrong on this because I'm seeing a lot of stuff in the code that looks like wordpress is preparing for something larger than multisite, multiserver maybe?

Basically, I am wondering if I should be be using switch_to_blog? Is there a replacement I should be using?

Thanks,
Jim

comment:14 @jimisaacs5 years ago

SCRATCH THAT, this instead:

$override = (( !$main_override && WP_CONTENT_DIR . '/uploads' != ABSPATH . $upload_path ) || $switched === true);
if ( defined('UPLOADS') && $override ) {
	$dir = ABSPATH . UPLOADS;
	$url = trailingslashit( $siteurl ) . UPLOADS;
}

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

That would mean:
Override IF "not overriding main site and upload path is not the default, OR we have switched"

comment:15 @jimisaacs5 years ago

Ok I did some more testing removing my code and going back to the original function. What seem weird is this, I set an arbitrary value in the option upload_path for a blog I intend to run to the swtich to, and regardless of the option, I get :

"<host>/<site>/files/<image-path>" for the thumbnail urls on the template.

Now, as soon as I remove the arbitrary value in he upload_path option (blank), it breaks going back to the wp-content/uploads path as I said before.

I even blanked out the upload_url_path option with and arbitrary value in upload_path and it worked. Only doesn't work when upload_path is empty, which makes me think it has to do with this logic:

if ( empty($upload_path) ) {
	$dir = WP_CONTENT_DIR . '/uploads';
} else {
	$dir = $upload_path;
	if ( 'wp-content/uploads' == $upload_path ) {
		$dir = WP_CONTENT_DIR . '/uploads';
		$main_override = defined( 'MULTISITE' ) && is_main_site();
	} elseif ( 0 !== strpos($dir, ABSPATH) ) {
		// $dir is absolute, $upload_path is (maybe) relative to ABSPATH
		$dir = path_join( ABSPATH, $dir );
	}
}

if ( !$url = get_option( 'upload_url_path' ) ) {
	if ( empty($upload_path) || ( 'wp-content/uploads' == $upload_path ) || ( $upload_path == $dir ) )
		$url = WP_CONTENT_URL . '/uploads';
	else
		$url = trailingslashit( $siteurl ) . $upload_path;
}

comment:16 follow-up: @jimisaacs5 years ago

Ok I was trying to find out why the empty check was like that, and I think I've got the problem.

In my version of the db, I had not value in for the upload_path option, in all of my blogs.

In the function install_blog() there is the line:

update_option('upload_path', "wp-content/blogs.dir/" . $blog_id . "/files");

This is the only place this occurs, it does not occur in the upgrade or network upgrade processes.

Then in the wp_upload_dir(), it checks for empty because a multisite installation from the previous logic, should always have a value within the upload_path option based on the installation process.

The problem now is two things, that option never got installed or updated for me.
It worked with no value in my previous version, and it doesn't work with no value in my current version.

comment:17 @jimisaacs5 years ago

That was it, add the upload_path the way the install_blog function was supposed to add it, and everything works as it is supposed to. I can switch_to_blog(), I can upload, and delete images on all blogs.

comment:18 @anointed5 years ago

  • Cc anointed added

comment:19 in reply to: ↑ 16 @wpmuguru5 years ago

Replying to jimisaacs:

In my version of the db, I had not value in for the upload_path option, in all of my blogs.

In the function install_blog() there is the line:

update_option('upload_path', "wp-content/blogs.dir/" . $blog_id . "/files");

Up above you said you imported from MU into WP. What specifically did you import?

MU adds that option/value to every blog. We have a MU site that was installed pre MU 1.0 and the blogs have that option set to the value above.

BUT, MU works fine without a value in the file_upload_path option.

I won't be able to do anything with this unless I have some representative values to work with. What was your fileupload_url, etc? You can change domain names, etc. to generic values like mysite.com?

comment:20 @wpmuguru5 years ago

After some thought...

Your problem is that you imported MU instead of upgrading MU. In an upgraded MU install the MULTISITE constant is not defined and $main_override will always be false.

comment:21 follow-up: @jimisaacs5 years ago

The import was not what you are thinking, and it was like 8 revisions ago The import was just for posts and post content.

I manually exported and imported, from each blog from my MU installation to my FRESH install of multisite 3.0, all with the builtin admin export/importer.

After that, I set up each plugin that I needed, one at a time. Starting with WordTube which uses separate tables, and for only one blog.

Then I went through each blog post double checking everything.

Then I started to work on my theme. As I said, it worked before (which is after all this, but before the latest update), then it didn't work after the latest update.

The value of fileupload_url was
http://myhost.com/blogname/files

I changed this option to upload_url_path, then eventually I blank this value and removed fileupload_url completely. After more of this, testing, and looking at wp_upload_dir(), I finally found the problem wasn't even with these options, it was with upload_path. Which I hadn't touched through this whole process, until then when I was testing with it.

There were no values in upload_path after the latest update, and what I can also tell you is that there were no values in this option before the update.

What I was saying before we started to talk about my update/upgrade pattern is that in my current version, when I have any value in upload_path, and it doesn't matter what, everything works. When I have nothing, it doesn't work.
That's really it.

If you are trying to find out anything else from my update, I posted version information earlier in these comments.

There is also a dead value being installed in the install_blog() function, "fileupload_url" isn't used anywhere else, as @ryan and I cam to realize.

comment:22 in reply to: ↑ 21 @wpmuguru5 years ago

Replying to jimisaacs:

There is also a dead value being installed in the install_blog() function, "fileupload_url" isn't used anywhere else, as @ryan and I cam to realize.

The reason I can see for both options being there is that the /blogs.dir/blog_id/files structure is limited in the number of sites (32767) that it supports. Beyond that, you need to implement a custom content folder system. Those options contain the actual location & url of the uploaded media and can be used by the custom plugin and implementation doesn't require populating those values.

comment:23 @ryan5 years ago

Anyone with that big of a site will hopefully be on 64 bit hardware and OS. :-)

comment:24 follow-up: @wpmuguru5 years ago

I've tried this with both the upload_path & upload_url_path options blank and the media still works for me. I tried both a main site and sub site. In the main site, the files in the wp-content/uploads folder don't work properly in the admin area anymore with the upload_path blank, but that's expected behavior.

comment:25 in reply to: ↑ 24 ; follow-up: @jimisaacs5 years ago

Replying to wpmuguru:

I've tried this with both the upload_path & upload_url_path options blank and the media still works for me. I tried both a main site and sub site. In the main site, the files in the wp-content/uploads folder don't work properly in the admin area anymore with the upload_path blank, but that's expected behavior.

@wpmuguru Thanks for testing, and I sorry for not mentioning to you what I had earlier in these comments, but the behavior in question is all about the switch_to_blog() functionality. I'm not sure if this is what you tested, if not please refer to my comments above.

comment:26 in reply to: ↑ 25 @wpmuguru5 years ago

  • Milestone changed from 3.0 to Future Release

Replying to jimisaacs:

@wpmuguru Thanks for testing, and I sorry for not mentioning to you what I had earlier in these comments, but the behavior in question is all about the switch_to_blog() functionality.

Ah :) Okay, it's all clear, media was not working properly under switch_to_blog in either MU or in trunk up until I made some recent changes. If you had come up with a custom setup that was working before, it is probably broken now.

Both UPLOADS & BLOGUPLOADDIR are constants that refer to the original unswitched blog. The last change blocks those being used when switched.

You should be able to sort something out using the upload_dir filter.

We may be able to do something more flexible in a future release, but I'm not planning on tackling it this close to RC.

comment:27 @Frumph5 years ago

fileupload_url is used as an option in quite a few wpmu themes and plugins to map the location of where the blogs.dir/#/files to domain.tld/files, it makes the site more secure to store the individual blogs site-files

.. could this be a problem with the network update routine? Where it's not adding those multisite options when doing a network update?

requesting this bumped in severity and needs-patch and 3.0

This is being created properly in wp-includes/ms-functions.php function install_blog however is not being done at all in wp-admin/includes/schema.php function populate_network

comment:28 follow-up: @wpmuguru5 years ago

The issue being reported is after a switch_to_blog. In MU, wp_upload_dir returned the settings for the original unswitched blog after a switch_to_blog.

Original (in MU)

array(6) {
  ["path"]=>
  string(69) "/home/ron/NetBeansProjects/trunk/wp-content/blogs.dir/1/files/2010/05"
  ["url"]=>
  string(32) "http://mutrunk.loc/files/2010/05"
  ["subdir"]=>
  string(8) "/2010/05"
  ["basedir"]=>
  string(62) "/home/ron/NetBeansProjects/trunk/wp-content/blogs.dir/1/files/"
  ["baseurl"]=>
  string(24) "http://mutrunk.loc/files"
  ["error"]=>
  bool(false)
}

While switched (in MU)

array(6) {
  ["path"]=>
  string(69) "/home/ron/NetBeansProjects/trunk/wp-content/blogs.dir/1/files/2010/05"
  ["url"]=>
  string(41) "http://mutrunk.loc/testblog/files/2010/05"
  ["subdir"]=>
  string(8) "/2010/05"
  ["basedir"]=>
  string(62) "/home/ron/NetBeansProjects/trunk/wp-content/blogs.dir/1/files/"
  ["baseurl"]=>
  string(33) "http://mutrunk.loc/testblog/files"
  ["error"]=>
  bool(false)
}

Unless the current code is doing something different for an upgraded MU install, then the issue is not a bug.

If you install the network now, it does set the upload_path option if the option is blank. If the option is not blank that means the install is using a custom content setup and it will be up to the site owner to implement the custom content setup for the network.

comment:29 in reply to: ↑ 28 @wpmuguru5 years ago

Replying to wpmuguru:

The issue being reported is after a switch_to_blog. In MU, wp_upload_dir returned the settings for the original unswitched blog after a switch_to_blog.

Minor correction - the base urls were correct, the paths still pointed to the unswitched blog.

comment:30 @Frumph5 years ago

The option is not being populating in the options table for the main site after an upgrade. fileupload_url

It's a resource which is used in quite a few plugins/themes for wpmu.

Again, being done on creating of sites on the network, but not being added to the site on the network initial install.

And apparently ipstenu reports it not there for a few other sites that were done with import. http://wordpress.org/support/topic/402692?replies=29

comment:31 @Frumph5 years ago

When I say upgrade, I mean upgrade to network install.

comment:32 @jimisaacs5 years ago

@Frumph

The the first post in that link describes what was happening to my image paths to a T. My problem was just on a switch to blog, these people seem like it may be separate problem.

I had the option, I was just convinced it was being used at all, my problems had mostly to do with the upload_path option. in my options table, fileupload_url was there and populated, it was upload_path that was not.

comment:33 @Frumph5 years ago

Understood. Somethings hokey in there for sure, probably better to be diagnosed with more information on it. Seems my upload_path fileupload_url issues could very well be entwined in all of that. Going to make a function in my functions.php to verify the options exist and populate them if they don't just to be on the safe side.

Although a patch to the network update to do this would be nice. ms-upgrade-network.php to verify those options have value if not populate them.

comment:34 @wpmuguru5 years ago

(In [14998]) add fileupload_url option to main site on network install, see #13483

comment:35 @wpmuguru5 years ago

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

Calling this one fixed.

comment:36 @nacin5 years ago

  • Milestone changed from Future Release to 3.0
Note: See TracTickets for help on using tickets.