WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 8 months ago

Last modified 8 months ago

#23685 closed defect (bug) (fixed)

Explicitly globalize version variables

Reported by: nbachiyski Owned by: 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 17 months ago.
globalize-version-vars-in-wp-settings.diff (808 bytes) - added by nbachiyski 16 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 nacin16 months 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).

comment:2 nbachiyski16 months 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.

comment:3 nacin12 months ago

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

comment:4 wonderboymusic11 months ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

comment:5 SergeyBiryukov11 months 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 months ago by SergeyBiryukov (previous) (diff)

comment:6 nacin9 months ago

  • Keywords commit added
  • Milestone changed from 3.7 to 3.8

comment:7 wonderboymusic8 months 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.

comment:8 dd328 months 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.

comment:9 wonderboymusic8 months ago

I spaced - which one is preferred?

comment:10 wonderboymusic8 months ago

In 26009:

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

Note: See TracTickets for help on using tickets.