restore_current_blog() being called when not needed, causing issues with some plugins

[sj-xweb]

bp_get_object_terms() in bp-core/bp-core-taxonomy.php does a root blog check, and switches to root if the check fails.

Then restore_current_blog() is called soon after.

The problem is that restore_current_blog() can be called even if switch_to_blog() didn't get called earlier. This is causing some issues with some plugins that has some functionality that leans on switch_to_blog quite a bit, but can't/don't use restore_to_blog to go back to the original blog. Which means that $GLOBALS['_wp_switched_stack'] (what WP uses to keep track of all the blog switches, and restore_current_blog() uses to determine if there is a blog to restore to) doesn't get cleared out properly.

When BP calls bp_get_object_terms() which in turn calls restore_current_blog() even when not needed, causes current blog to be changed to the last time switch_to_blog() was called.

For example, if $GLOBALS['_wp_switched_stack'] has the following value:

array(
  '0' => 1
  '1' => 2
  '2' => 3
  '3' => 4
  '4' => 5
)

The current blog is 1, and the last element was the last blog the code switched from. The above may happen if the plugin loop through switch_to_blog to poll all the blogs in multisite. So the next time restore_to_blog() is called, the blog it switches to is 5, which is wrong.

Please let me know if you need more clarification.

[johnjamesjacoby]

Interesting. We can add a root blog check, and have done so in at least one other place in the past. I'm more curious what the use case is for switching once and not switching back. I have a hunch many plugins are not prepared for this, including ones that register or rely on any custom post types.

switch_to_blog() went through several iterations in the early days, and I recall it working the way you mention, where switches were, basically wound up and down. Each switch required a restore, even if a switch switched to the same blog 5 times in a row. This means switching without restoring was a bit of sabotage, and what our code here represents is a fine working example of that.

If necessary, I'm fine adding a bp_is_root_blog() check, and since we are storing things in the root blog increasingly often, we may want wrapper functions to more confidently switch around.

[boonebgorges]

Like johnjamesjacoby, I'm a bit dubious of plugins that are doing funky things with switch_to_blog() and especially the _wp_switched_stack etc globals directly. That said, it's harmless to add the checks - they were only excluded initially for the sake of brevity.

[boonebgorges]

In 9519:

In taxonomy functions, don't restore_current_blog() when not necessary.

Fixes #6240.

[boonebgorges]

In 9520:

In taxonomy functions, don't restore_current_blog() when not necessary.

Fixes #6240.

[sj-xweb]

Awesome, thanks guys!

This was a weird case, and honestly probably doesn't affect 99.9% of the people out there. It just happens to affect us in our case, and causing strange things when BP is trying to load custom templates in our themes.