WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#24760 closed defect (bug) (fixed)

Creating default object from empty value in wp-includes/ms-settings.php on line 111

Reported by: conner_bw Owned by: jeremyfelt
Milestone: 4.5 Priority: normal
Severity: normal Version: 3.0
Component: Multisite Keywords: has-patch
Focuses: multisite Cc:

Description (last modified by SergeyBiryukov)

Getting:

Creating default object from empty value in wp-includes/ms-settings.php on line 111

On MU signups.

Line ~111 is:

if ( empty( $current_blog->site_id ) )
  $current_blog->site_id = 1;

Short version: Depending on caching system used, or if some plugin "enhances" the signup procedure, $current_blog can be null.

Would benefit from a patch similar to: [21815]

Thank you for your consideration.

Attachments (2)

24760.patch (657 bytes) - added by uglyrobot 5 years ago.
Make new site activation email links consistent
24760_2.patch (672 bytes) - added by uglyrobot 5 years ago.
Set additional properties of $current_blog global to avoid notice

Download all attachments as: .zip

Change History (24)

#1 @markoheijnen
7 years ago

  • Milestone changed from Awaiting Review to 3.6

#2 @SergeyBiryukov
7 years ago

  • Component changed from General to Multisite
  • Description modified (diff)
  • Version changed from 3.5.2 to 3.0

Related: mu:1373

#3 @nacin
7 years ago

  • Milestone changed from 3.6 to Future Release

Short version: Depending on caching system used, or if some plugin "enhances" the signup procedure, $current_blog can be null.

We already reference $current_blog higher in the stack, we just don't write to it. So a patch like [21815] sounds like it would be insufficient.

Could we have additional information on caching systems or creative plugins?

#4 @SergeyBiryukov
7 years ago

  • Keywords reporter-feedback added

#5 @conner_bw
7 years ago

We use memcached.

We don't run a lot of plugins. Basically


And:

Which I write myself.

Regards,

#6 in reply to: ↑ description @jeremyfelt
7 years ago

Replying to conner_bw:

... or if some plugin "enhances" the signup procedure, $current_blog can be null.

Do you have an example of a plugin that enhances the signup procedure or steps to replicate?

#7 follow-up: @conner_bw
7 years ago

We do

RewriteRule ^wp-(signup|activate)\.php /path/to/.core/wp-$1.php [L,QSA]

Then, we cut and paste wp-signup and wp-activate, move to .core/, we change the require() paths, we change the text to reflect books instead of sites.

The "stack" may or may not be bypassed.

Thanks.

#8 in reply to: ↑ 7 @conner_bw
7 years ago

The "stack" may or may not be bypassed.

What I mean is "I don't know."

All I know is that the warning/error occurs, I see it in my logs. $current_blog is empty. The rest I am unsure. I would like to see that warning go away.

Cheers.

#9 @nacin
7 years ago

All I know is that the warning/error occurs, I see it in my logs. $current_blog is empty. The rest I am unsure. I would like to see that warning go away.

It appears the warning is occurring because of a developer error, so it should certainly stay as someone is doing something wrong. I don't see a path through ms-settings.php that should ever cause $current_blog to be empty.

We can't do anything about it until we actually do know why $current_blog is not properly populated in cache.

#10 @conner_bw
7 years ago

Ok, mark as invalid then.
I will post more info if ever something else becomes evident.

#11 @conner_bw
7 years ago

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

#12 @SergeyBiryukov
7 years ago

  • Milestone Future Release deleted

#13 in reply to: ↑ description @EllyTronic
7 years ago

  • Cc iwork@… added

I just wanted to add in case anyone else comes here from Google looking for an answer, I had the same problem and can tell you why you're having it -- this is only tested with MU setup for subdomains.

You'll notice the activation email contains the address of the new site followed by /wp-activate.php?key=xxxxxx

You are also probably running pro sites plugin or some WPMU plugin similar to this where a user must do something "special" to activate the site -- in "Pro Sites" this usually means paying. Thus, because the site isn't activated and doesn't yet exist in the DB, $current_blog is null. If you have the user follow the link but remove their subdomain from the email, it will work just fine.. I'm sure there's a better solution out there, but I haven't found it yet. Hopefully this illuminates the problem though, as it seems to be a popular one.

Replying to conner_bw:

Getting:

Creating default object from empty value in wp-includes/ms-settings.php on line 111

On MU signups.

Line ~111 is:

if ( empty( $current_blog->site_id ) )
  $current_blog->site_id = 1;

Short version: Depending on caching system used, or if some plugin "enhances" the signup procedure, $current_blog can be null.

Would benefit from a patch similar to: [21815]

Thank you for your consideration.

#14 @uglyrobot
6 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I've confirmed this exists in stock subdomain Multisite with no plugins or dropins. WP 3.8.3.

Notice: Trying to get property of non-object in /srv/www/wordpress-multisite/htdocs/wp-includes/ms-settings.php on line 107
Notice: Trying to get property of non-object in /srv/www/wordpress-multisite/htdocs/wp-includes/ms-settings.php on line 108
Warning: Creating default object from empty value in /srv/www/wordpress-multisite/htdocs/wp-includes/ms-settings.php on line 111

It seems to me the best option is to keep wp-activate.php links in email at the main site, just like wp-signup.php always is. Linking to the subsite seems strange to me, especially as it uses the main site theme anyway.

#15 @nacin
6 years ago

  • Milestone set to Awaiting Review

This is likely fixed in 3.9.

#16 @chriscct7
5 years ago

  • Keywords needs-testing added

#17 @jeremyfelt
5 years ago

  • Focuses multisite added
  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release

I'm able to confirm the current manifestation of this in trunk thanks to the info provided in the last few comments.

  1. Subdomain configuration of multsite with site registration enabled.
  2. Signup for a new site at site.foo.bar/wp-signup.php.
  3. Follow the activation link sent via email to new.site.foo.bar/wp-signup.php.
Notice: Undefined property: stdClass::$domain in .../src/wp-includes/ms-settings.php on line 189
Notice: Undefined property: stdClass::$public in .../src/wp-includes/ms-settings.php on line 199

As new.site.foo.bar is not yet added as a row in wp_blogs, the given domain is not able to match an existing site. Normally, this would redirect to a signup page or follow NOBLOGREDIRECT. But there's this:

// @todo Investigate when exactly this can occur.
if ( empty( $current_blog ) && wp_installing() ) {
	$current_blog = new stdClass;
	$current_blog->blog_id = $blog_id = 1;
}

That todo has made me laugh for a couple years now and I've never properly traced it through. wp-activate.php sets the WP_INSTALLING constant as true and then loads wp-load.php, which starts the bootstrap process and runs into this block.

We can probably set the path, domain, and public properties of $current_blog to $domain, $path, and 1 respectively to ensure a more full object is provided.

We can also update that todo line. :)

@uglyrobot
5 years ago

Make new site activation email links consistent

#18 @uglyrobot
5 years ago

  • Keywords has-patch added; needs-patch removed

After reviewing this issue, it seems to me the the underlying problem is the inconsistencies with signup activations on subdomain installs. In subdirectory installs the activation link goes to network_site_url() root, not /blog/wp-activate.php. wp-activate.php like wp-signup.php uses the main sites theme as well.

So rather than trying to hack ms-settings.php to not cause errors when you access a non-existent blog, I propose simply making the activation link in the email consistent for both configurations, and I've attached a patch to that effect.

Purposely not adding redirects into wp-activate.php so as to not break backwards compatibility with any plugins that may be overriding the activation email text.

#19 @uglyrobot
5 years ago

  • Keywords needs-testing removed

I've also attached a tested patch with solution proposed by @jeremyfelt in case we want to go that route instead or in addition to making links consistent. I've update the @todo in it as well.

Note that with network changes in 4.4, only

Notice: Undefined property: stdClass::$public in .../src/wp-includes/ms-settings.php

is the remaining notice to fix here.

@uglyrobot
5 years ago

Set additional properties of $current_blog global to avoid notice

#20 @uglyrobot
5 years ago

  • Keywords dev-feedback added

#21 @jeremyfelt
5 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Future Release to 4.5
  • Owner set to jeremyfelt
  • Status changed from reopened to reviewing

Thanks for the patch @uglyrobot! I think we're best off with the second patch to fill in missing properties. Now that I've stared at it a bit more, I'd like to only set public as that's the only obvious thing we need. I'm hesitant that populating domain and path could cause a chain reaction that's not currently visible.

I'm running tests now.

#22 @jeremyfelt
5 years ago

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

In 35782:

MS: Populate public on empty $current_blog during subdomain activation.

Activation of a subdomain site is done through that new site's address. This address does not exist in the wp_blogs table until activation is complete.

In this case we need to make sure public is populated to avoid a PHP notice.

Props uglyrobot.
Fixes #24760.

Note: See TracTickets for help on using tickets.