Parameter 1 to wp_default_styles() expected to be a reference, value given

[jqz]

This is a follow-up to #37772.

The bug still manifests if the UOPZ extension is loaded.

(It is a PHP warning, so functionality is okay, but log files will get cluttered and performance hit as server writes to log file on each page request.)

Environment:

• Windows 10 x64
• Apache 2.4.27
• PHP 7.1.9 VC14-x64 Thread Safe
• UOPZ extension 5.0.2 (PHP 7.1 Thread Safe x64 - ​https://pecl.php.net/package/uopz/5.0.2/windows)

Fresh install of WordPress 4.9.8.

It can be fixed by removing the & from the function parameter declaration for wp_default_styles() and wp_default_scripts() - the & is not required as the parameter is an object and thus the original object is referenced in any case.

[SergeyBiryukov]

Hi @jqz, welcome to WordPress Trac!

Thanks for the ticket, I was able to reproduce the issue with the UOPZ extension installed.

[pento]

I originally milestoned this for 5.0 as a possible PHP 7.3 compatibility issue. As it's to do with the UOPZ extension, I'm moving this to 5.0.1 for further evaluation.

I suspect the fix is going to be trickier than just removing the & and changing WP_Scripts::init() to call do_action() instead, given historical conversations around the behaviour of this code.

In particular, any fix will need to be tested with PHP 5.2 and 5.3, which had some weird object/reference behaviour that the handful of remaining do_action_ref_array() usages seem to exist to work around.

[aaronholbrook]

Just wanted to chime in and say this is affecting every site we have on WP Engine that is attempting to run PHP 7 and above (7.1, 7.2) and is definitely clogging up our error logging system.

Any way to not kick this down the road would be greatly appreciated as we've been blocked from upgrading PHP for the time being.

[pento]

This ticket needs a patch and significant testing to ensure changing it doesn't break everything.

Alternatively, writing a patch that works for PHP 5.6+, and waiting until WordPress 5.2 may make things easier, as we'll be dropping PHP 5.2-5.5 support.

[jqz]

Alternatively, writing a patch that works for PHP 5.6+, and waiting until WordPress 5.2 may make things easier, as we'll be dropping PHP 5.2-5.5 support.

Why not drop (the problematic) PHP 5.2-5.3 support in WordPress 5.1? Is anyone really using the latest version of WordPress with such old versions of PHP?

Waiting till WordPress 5.2 will presumably mean waiting the best part of a year.

[jorbin]

Bumping this from 5.2 due to lack of progress.

As 5.2 drops support for php versions below 5.6, the old PHP versions is no longer a concern.

[killerbishop]

I've been working on making a patch for this issue and testing against PHP 5.6. Changing the reference requires changing other downstream functions that are called by it such as wp_default_packages(). I noticed in the wp-admin/load-scripts.php that the functions are called outside the class and pass a new instance directly. Is do_action() really needed here or can it just be replaced with a direct call like wp_default_scripts($this)?

Lastly, is there any issue with adding typehints like:

function wp_default_scripts( WP_Scripts $scripts )

I see very few uses of it grepping around, but the contributor's handbook makes no reference either way on this. Historically, I can see it being not used for compatibility with older PHP versions and perhaps is something being discussed still.

[killerbishop]

Patch for 44979 - Switch from pass-by-ref on wp_default_styles() and wp_default_scripts()

[killerbishop]

I've added the patch with do_action() left intact. I did have to switch the noop.php script to disable do_action() instead of do_action_ref_array() since I changed to that. I tested to make sure scripts/styles are still loading as expected in prodction and debug mode. Here is my cloned repo's PR if needed:

​https://github.com/killerbishop/wordpress-develop/pull/3

[Mte90]

On VVV many people confirmed the same error in those days (me too) ​https://github.com/Varying-Vagrant-Vagrants/VVV/issues/1940
At ​https://github.com/Varying-Vagrant-Vagrants/VVV/issues/1940#issuecomment-534122784 we suggested a way for a fix.

[Mte90]

In VVV we saw that this happen with the Tideways extension enabled.
So maybe because is code to support ancient PHP version with some extension create issues like with UOPZ.
In my testing this happen with PHP 7.2 and I can confirm that the patch works!
I am sad that now to get this fix we have to wait for WordPress 5.4...

[TJNowell]

Can we get a clear Todo list on what needs to be done and what needs to be tested to get this resolved? This is becoming a significant support burden, I'd like to spend the energy fixing the issue rather than supporting the issue if I have a choice, but it's unclear what the blocker is from reading the ticket

[jorbin]

I've updated the patch so that 1) It doesn't include the type hints which aren't necessary for this and 2) continues to noop do_action_ref_array to prevent unnecessary side affects. I would like some additional people to test this and confirm it works.

[pcarvalho]

latest patch 44979.2.diff doesn't quite work for me. Still get the warnings. It needs all the script-loader.php hunks. ( not sure if all, but applying first patch solves my warnings )

[valentinbora]

I've encountered these while having the tideways_xhprof extension installed and active (PHP 7.2)

[Mte90]

I can confirm that 44979.2.diff doesn't fix the problem with php 7.2

[SergeyBiryukov]

In my testing, the issue happens with:

• PHP 7.2.x, php_tideways_xhprof extension (tested versions 5.0.1 and 5.0.2).
• PHP 7.2.x, php_uopz extension (version 5.0.2).

Does not happen with:

• PHP 7.2.x and later, php_uopz extension (version 5.1.0+).
• PHP 7.3.x and later, php_tideways_xhprof extension.

Replacing do_action_ref_array() with do_action() was previously explored in #16661 and #25160. It might actually break things and cause the same warning if plugins hooked to wp_default_scripts still use a reference in the function signature, for example:

function my_scripts( &$scripts ) {
	...
}
add_action( 'wp_default_scripts', 'my_scripts' );

As noted in comment:23:ticket:25160, the warning is caused by call_user_func_array() usage in WP_Hook::apply_filters()​. As a bit more complete illustration from comment:24:ticket:25160, this snippet:

include 'wp-load.php';

class Test_Me {
    function __construct() {
        do_action( 'woo', $this );
    }
}

function __woo( &$ref ) {

}

add_action( 'woo', '__woo' );

new Test_Me();
exit();

produces: PHP Warning: Parameter 1 to __woo() expected to be a reference, value given.

In my testing, do_action_ref_array() is not the issue here (there are ~90 instances of do_action_ref_array() and apply_filters_ref_array() in core that don't cause the warning).

The real issue is the reference in these function signatures:

function wp_register_tinymce_scripts( &$scripts, $force_uncompressed = false )
function wp_default_packages_vendor( &$scripts )
function wp_get_script_polyfill( &$scripts, $tests )
function wp_default_packages_scripts( &$scripts )
function wp_default_packages_inline_scripts( &$scripts )
function wp_default_packages( &$scripts )
function wp_default_scripts( &$scripts )
function wp_default_styles( &$styles )

Most of these were just copied from wp_default_scripts(). Removing the references appears to resolve the warnings without affecting the functionality.

[SergeyBiryukov]

In 47355:

Script Loader: Remove unnecessary reference sign from function definitions in script loader.

This resolves PHP warnings when extensions like php_uopz or php_tideways_xhprof are in use.

Object variables in PHP 5+ contain a reference to the object, and it's the reference that's passed around.

Props jqz, killerbishop, Mte90, TJNowell, jorbin, pento, pcarvalho, valentinbora, SergeyBiryukov.
Fixes #44979.