Make WordPress Core

Opened 2 years ago

Closed 22 months ago

#56010 closed defect (bug) (fixed)

apache_mod_loaded returns fatal TypeError in some environments with PHP 8

Reported by: engahmeds3ed's profile engahmeds3ed Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.1 Priority: normal
Severity: major Version:
Component: General Keywords: php8 has-patch commit
Focuses: Cc:

Description

When we call WP Core function (apache_mod_loaded) in environment like WP Engine which has the PHP function (apache_get_modules) already disabled also with php 8 or above will lead to the following fatal error:

PHP Fatal error: Uncaught TypeError: in_array(): Argument #2 ($haystack) must be of type array, null given in /<redacted>/wp-includes/functions.php:5834\nStack trace:
#0 /<redacted>/wp-includes/functions.php(5834): in_array('mod_pagespeed', NULL, true)
#1 /<redacted>/wp-content/plugins/wp-rocket/inc/ThirdParty/Plugins/ModPagespeed.php(52): apache_mod_loaded('mod_pagespeed', false)

and this is happening because of the following code:

https://github.com/WordPress/WordPress/blob/master/wp-includes/functions.php#L5890

Here we get the return value from (apache_get_modules) which is this cases returns null because it's already disabled by the hosting company then pass this null as a second argument to the in_array function which expects an array.

Before PHP 8, we may see only notice in logs but after PHP 8 we get fatal error which blocks the site completely.

Possible solutions:-

  1. Cast the value passed to (in_array) to be array (but this will return not valid values in such mentioned environments)
  2. Add the following condition along with the check if the function exists or not like this
    <?php
    if ( function_exists( 'apache_get_modules' ) && false === strpos( ini_get( 'disable_functions' ), 'apache_get_modules' ) ) {
    

to make sure that this function isn't in the disabled functions.

Whatever solution we will take, I can work on it.
Thanks

Attachments (1)

56010.diff (977 bytes) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (8)

#1 @costdev
2 years ago

  • Keywords php8 added

#2 @engahmeds3ed
2 years ago

  • Severity changed from critical to major

Today when I tried again in WP Engine I found that (apache_get_modules) returns empty array, so we will not have this fatal error in WP Engine but still returns not accurate results as it always returns empty array, which means no apache module is installed and activated which isn't correct.

@SergeyBiryukov
2 years ago

#3 @SergeyBiryukov
2 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 6.1

Hi there, welcome to WordPress Trac! Thanks for the report.

I think ensuring that $mods is an array might make sense in theory. However, an array is the documented return value for apache_get_modules(). Unless I'm missing something, the only way to get that fatal error is to redeclare the disabled function (which is possible in PHP 8.0+) and return null instead of an array, which seems like a wrong thing to do on the host's part, an empty array should be returned instead.

If the function is simply disabled and not redeclared, the function_exists() check should return false, though that may not always be the case, see comment:10:ticket:55711.

Based on comment:2, it looks like this was corrected and the host made adjustments to return an empty array. In that case, I think we can use the existing phpinfo() fallback, see 56010.diff.

#4 @engahmeds3ed
2 years ago

Amazing as usual!

You are totally right, returning null was the host's problem and this shouldn't be the case we rely on in CORE.
falling back to the phpinfo solution will fix the issue and return the real expected value.

Thanks

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


22 months ago

#6 @audrasjb
22 months ago

  • Keywords commit added

As per today's bug scrub, @Clorith and I agreed to mark this for commit consideration.
@SergeyBiryukov feel free to do the honors as you proposed the patch :D

#7 @SergeyBiryukov
22 months ago

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

In 54299:

General: Correct the fallback logic in apache_mod_loaded().

If the apache_get_modules() function is redeclared to return an empty array, apache_mod_loaded() would assume that no Apache modules are installed and activated, which may not be correct.

This commit improves the logic by using pre-existing phpinfo() fallback to check for loaded modules in that case.

Includes replacing a hardcoded number passed as a flag to phpinfo() with the INFO_MODULES predefined constant for clarity.

Follow-up to [7441], [7508], [29330].

Props engahmeds3ed, audrasjb, Clorith, SergeyBiryukov.
Fixes #56010.

Note: See TracTickets for help on using tickets.