Opened 10 months ago

Closed 9 months ago

#21316 closed defect (bug) (fixed)

wp_check_php_mysql_versions() is broken in PHP 4

Reported by: SergeyBiryukov Owned by: nacin
Priority: normal Milestone: 3.4.2
Component: Upgrade/Install Version: 3.4
Severity: normal Keywords: has-patch commit
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 10 months ago.
21316.2.patch (894 bytes) - added by SergeyBiryukov 10 months ago.
21316.3.patch (1.2 KB) - added by SergeyBiryukov 10 months ago.
21316.4.patch (2.4 KB) - added by SergeyBiryukov 10 months ago.
21316.5.patch (3.5 KB) - added by SergeyBiryukov 10 months ago.
21316.6.patch (6.5 KB) - added by SergeyBiryukov 10 months ago.

Download all attachments as: .zip

Change History (25)

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

  • Description modified (diff)

comment:3 follow-ups: ↓ 4 ↓ 5   nacin10 months 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.

comment:4 in reply to: ↑ 3   Otto4210 months 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.

comment:5 in reply to: ↑ 3   SergeyBiryukov10 months 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().

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.

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.

comment:8 follow-up: ↓ 9   scribu10 months ago

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

comment:9 in reply to: ↑ 8   nacin10 months 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.

  • Description modified (diff)
  • Keywords commit added

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

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.

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.

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.

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.

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

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

  • 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.