fb-instant-articles icon indicating copy to clipboard operation
fb-instant-articles copied to clipboard

PHP 7.4 Compatibility

Open douglas-johnson opened this issue 4 years ago • 1 comments

I am using this plugin on a WordPress VIP Go site. We received our PHP 7.4 compatibility report in anticipation of a server upgrade in February. There were several issues created based on code in this plugin. I started a VIP support ticket about it here: https://wordpressvip.zendesk.com/hc/en-us/requests/121347

Compatibility Warnings

All of the warnings appear in /vendor packages. Is it possible to update these in the plugin to resolve the warnings?

Error in plugins/fb-instant-articles/vendor/facebook/graph-sdk/src/Facebook/PseudoRandomString/McryptPseudoRandomStringGenerator.php: Extension 'mcrypt' is deprecated since PHP 7.1 and removed since PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available instead

Warning in plugins/fb-instant-articles/vendor/facebook/facebook-instant-articles-sdk-extensions-in-php/src/Facebook/InstantArticles/Utils/Observer.php: Since PHP 7.0, functions inspecting arguments, like func_get_args(), no longer report the original value as passed to a parameter, but will instead provide the current value. The parameter "$tag" was used, and possibly changed (by reference), on line 150.

Warning in plugins/fb-instant-articles/vendor/facebook/facebook-instant-articles-sdk-extensions-in-php/src/Facebook/InstantArticles/AMP/AMPHeader.php: Passing the $glue and $pieces parameters in reverse order to implode has been deprecated since PHP 7.4; $glue should be the first parameter and $pieces the second

Error in plugins/fb-instant-articles/vendor/apache/log4php/src/main/php/pattern/LoggerPatternConverterSuperglobal.php: "$this" can no longer be used with the "global" keyword since PHP 7.1

Version Info

  • Plugin version: 4.2.1
  • WordPress version: 5.6
  • PHP version: 7.3 - 7.4

douglas-johnson avatar Jan 08 '21 15:01 douglas-johnson

Error in plugins/fb-instant-articles/vendor/facebook/graph-sdk/src/Facebook/PseudoRandomString/McryptPseudoRandomStringGenerator.php: Extension 'mcrypt' is deprecated since PHP 7.1 and removed since PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available instead

✅ The containing McryptPseudoRandomStringGenerator::getPseudoRandomString() is not used directly in this plugin. Furthermore, the package has support for multiple Pseudo Random String generators, and a factory which defaults to random_bytes before checking for mcrypt. The three instances of starting up a generator object, all have no specific generator in the Facebook object config, so the default is used. That all means this is safe to ignore for PHP >=7.2 compatibility as it's essentially just code for backwards compatibility.

The fork of the php-graph-sdk that this plugin uses now ignores that error from being reported in PHPCS.

Warning in plugins/fb-instant-articles/vendor/facebook/facebook-instant-articles-sdk-extensions-in-php/src/Facebook/InstantArticles/Utils/Observer.php: Since PHP 7.0, functions inspecting arguments, like func_get_args(), no longer report the original value as passed to a parameter, but will instead provide the current value. The parameter "$tag" was used, and possibly changed (by reference), on line 150.

✅ The use of func_get_args() was removed here, but this seems incorrect to me, so I've opened a new PR to address this. The PHPCS violation is a warning, and in this case, a false positive, as the $tag value wasn't changed before func_get_args() was called. Adding in my PR will restore the previous functionality for applying (non-WordPress) filters, and that was for the handling of AMP articles within the Instant Articles SDK.

Warning in plugins/fb-instant-articles/vendor/facebook/facebook-instant-articles-sdk-extensions-in-php/src/Facebook/InstantArticles/AMP/AMPHeader.php: Passing the $glue and $pieces parameters in reverse order to implode has been deprecated since PHP 7.4; $glue should be the first parameter and $pieces the second

✅ This was fixed here. The Composer build for this plugin pulls in the php8 branch of that repo, so it inherits the fix.

Error in plugins/fb-instant-articles/vendor/apache/log4php/src/main/php/pattern/LoggerPatternConverterSuperglobal.php: "$this" can no longer be used with the "global" keyword since PHP 7.1

✅ I can't see which current dependencies pulls in apache/log4php, but it was likely a dev dependency anyway, so it wouldn't be included for the --no-dev build.

All of these are in-hand, so I'll mark this as closed. Thanks for reporting!

GaryJones avatar Oct 16 '22 01:10 GaryJones