Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#23685 closed defect (bug) (fixed)

Explicitly globalize version variables

Reported by: nbachiyski's profile nbachiyski Owned by: wonderboymusic's profile wonderboymusic
Milestone: 3.8 Priority: normal
Severity: normal Version: 3.6
Component: Unit Tests Keywords: has-patch 3.7-early commit
Focuses: Cc:

Description

When WordPress is loaded in a function (e.g. unit tests) the variables initialized at the top level aren't globals, but we expect them to be.

For example, if in a unit test there is no way to access $wp_version. Neither via $wp_version (it's not in the same scope), not via $GLOBALS['wp_version'] (it's not a global).

To fix that we need to explicitly make the version variables global.

See #17749.

Attachments (2)

globalize-version-vars.diff (357 bytes) - added by nbachiyski 11 years ago.
globalize-version-vars-in-wp-settings.diff (808 bytes) - added by nbachiyski 11 years ago.

Download all attachments as: .zip

Change History (12)

#1 @nacin
11 years ago

We sometimes include version.php in local scope inside a function on purpose, so we can get untainted variables (say, for an update check).

So, we would actually want to globalize these variables where this file is included (specifically wp-settings.php).

#2 @nbachiyski
11 years ago

Now I understand. In the previous ticket dd32 mentioned this, but I thought of something else. Sorry about the wasted cycles.

Here is a patch, which works for me.

#3 @nacin
11 years ago

  • Keywords 3.7-early added
  • Milestone changed from Awaiting Review to Future Release

#4 @wonderboymusic
11 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#5 @SergeyBiryukov
11 years ago

If you load a Multisite install in a function (by including wp-load.php), you'll get an error:

Fatal error: Call to a member function set_prefix() on a non-object in wp-includes/ms-settings.php on line 126

Tested in 3.3, 3.5.1, and trunk. See the IRC discussion.

Perhaps we should explicitly globalize $wpdb as well.

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

#6 @nacin
10 years ago

  • Keywords commit added
  • Milestone changed from 3.7 to 3.8

#7 @wonderboymusic
10 years ago

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

In 26008:

When WordPress is loaded in a function (e.g. unit tests) the variables initialized at the top level aren't globals, but we expect them to be. Explicitly make the version variables global.

Props nbachiyski.
Fixes #23685.

#8 @dd32
10 years ago

In [26008]

That commit applied both patches here, where it was only one or the other that was intended on being applied I think.

Adding the globals to version.php will mean that if a plugin is overwriting the $wp_version variable (to prevent the version being displayed or something) we'll then re-overwrite it. That's OK to me, but is worth noting.

Since we always include version.php within the update checks, this should still mean we use an untainted version variable though.

Also, SergeyBiryukov's comment regarding $wpdb is still valid, but should probably be pushed to another ticket.

#9 @wonderboymusic
10 years ago

I spaced - which one is preferred?

#10 @wonderboymusic
10 years ago

In 26009:

Don't globalize version variables twice. See #23685.

Note: See TracTickets for help on using tickets.