Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#21316 closed defect (bug) (fixed)

wp_check_php_mysql_versions() is broken in PHP 4

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: nacin's profile nacin
Milestone: 3.4.2 Priority: normal
Severity: normal Version: 3.4
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Since [19760].

wp_load_translations_early() includes functions.php, which in turn includes option.php. This leads to the parse error when running PHP 4 (due to the clone keyword being used there):

Parse error: syntax error, unexpected T_VARIABLE in wp-includes/option.php on line 225

http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/option.php#L224

In 3.3, a proper error message was shown:

Your server is running PHP version 4.4.7 but WordPress 3.3.3 requires at least 5.2.4.

Attachments (6)

21316.patch (467 bytes) - added by SergeyBiryukov 12 years ago.
21316.2.patch (894 bytes) - added by SergeyBiryukov 12 years ago.
21316.3.patch (1.2 KB) - added by SergeyBiryukov 12 years ago.
21316.4.patch (2.4 KB) - added by SergeyBiryukov 12 years ago.
21316.5.patch (3.5 KB) - added by SergeyBiryukov 12 years ago.
21316.6.patch (6.5 KB) - added by SergeyBiryukov 12 years ago.

Download all attachments as: .zip

Change History (25)

#1 @SergeyBiryukov
12 years ago

21316.patch is an attempt to fix this by conditionally including option.php.

#2 @SergeyBiryukov
12 years ago

  • Description modified (diff)

#3 follow-ups: @nacin
12 years ago

Conditionally including a file can slow things down because it results in an extra sysopen call and you don't get any benefit from an opcode cache.

We could do two different things here:

  • Remove the functions.php dependency from wp_load_translations_early(). I think it is there for wp_die().
  • In wp_check_php_mysql_versions(), check if the php version is < 5, in which case, die() with a message in English instead.

#4 in reply to: ↑ 3 @Otto42
12 years ago

Replying to nacin:

  • Remove the functions.php dependency from wp_load_translations_early(). I think it is there for wp_die().

This. Changing it to just "die" and eliminating that dependency seems to work fine.

#5 in reply to: ↑ 3 @SergeyBiryukov
12 years ago

Replying to nacin:

  • Remove the functions.php dependency from wp_load_translations_early(). I think it is there for wp_die().

Thanks, done in 21316.3.patch.

Looking at [19760], this seems to be the only place where die() was replaced with wp_die().

#6 @nacin
12 years ago

That's just because most of them were already wp_die(). Note that [19760/trunk/wp-load.php] removed the inclusion of functions.php since it was now tasked to wp_load_translations_early(). 21316.3.patch could cause fatal errors.

By removing the dependency, I was suggesting we move wp_die() to a different (or new) file. But we can also go back through and restore include calls to functions.php for uses of wp_die(), and removing the line otherwise.

#7 @SergeyBiryukov
12 years ago

I see. Moving the function doesn't seem worth it.

21316.4.patch restores the include calls in wp-load.php and setup-config.php.

#8 follow-up: @scribu
12 years ago

So, are we going to care about PHP 4 until we move to PHP 5.3?

#9 in reply to: ↑ 8 @nacin
12 years ago

Replying to scribu:

So, are we going to care about PHP 4 until we move to PHP 5.3?

We should always strive to display an error message in a PHP < 5.2.4 environment. (Remember, this means plenty of 5.x as well.) It was something I should have taken into account in [19760] et al. It isn't that big a deal to ensure we are syntactically safe up to the point where we display an error.

#10 @SergeyBiryukov
12 years ago

  • Description modified (diff)

#11 @SergeyBiryukov
12 years ago

  • Keywords commit added

#12 @SergeyBiryukov
12 years ago

Hmm, setup-config.php gives another parse error (new in 3.4):

Parse error: syntax error, unexpected '&', expecting T_VARIABLE or '$' in wp-admin/setup-config.php on line 221

Visiting /wp-admin/ (this one was in 3.3 as well):

Parse error: syntax error, unexpected T_OBJECT_OPERATOR in wp-admin/index.php on line 35

#13 @nacin
12 years ago

We'll have to remove the reference from that foreach loop, then. Simple enough, just need to set $config_file[ $key ] instead of $line.

We should also make sure there is a comment at the top of the file noting that the file must be parseable by PHP4.

#14 @SergeyBiryukov
12 years ago

21316.5.patch handles setup-config.php. A comment about PHP 4 is already there:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-admin/setup-config.php#L1

21316.6.patch handles the error in wp-admin/index.php as well, which was a regression in 3.3.

#15 @nacin
12 years ago

In [21715]:

Don't include functions.php in wp_load_translations_early()
to avoid a parse error in a PHP4 environment (caused by use of
the clone keyword in option.php).

Manually include functions.php in the rare situations where it
is not already included by the time we need to load translations
early.

Remove the functions.php dependency by switching the wp_die() calls
to die(), in wp_check_php_mysql_versions().

props SergeyBiryukov, see #21316, for trunk.

#16 @nacin
12 years ago

In [21716]:

setup-config.php must be parseable by PHP4 so we can show a sane error message. wp-admin/index.php should ideally be as well. props SergeyBiryukov, see #21316.

#17 @nacin
12 years ago

[21716] and [21715] are good to be merged to branches/3.4.

#18 @SergeyBiryukov
12 years ago

Tested [21715] and [21716] on PHP 4, didn't see any issues.

#19 @nacin
12 years ago

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

In [21739]:

Ensure we are parseable by PHP4 until wp_check_php_mysql_versions() has a chance to run.

Merges [21715], [21716] to the 3.4 branch.
props SergeyBiryukov.
fixes #21316.

Note: See TracTickets for help on using tickets.