WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 6 weeks ago

Last modified 4 weeks ago

#47783 closed defect (bug) (fixed)

PHP 7.4: Deprecations

Reported by: ayeshrajans Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch php74 has-dev-note
Focuses: coding-standards Cc:
PR Number:

Description

PHP 7.4 deprecates some functionalities, and Travis builds are currently not even completing properly due to the amount of warnings thrown.

We already have a few tickets (thanks to @jrfnl - 👏👏👏). I will post patches related to PHP 7.4 deprecation changes (https://wiki.php.net/rfc/deprecations_php_7_4) here.

Attachments (6)

47783-magic_quotes_deprecation-wp-core.patch (2.0 KB) - added by ayeshrajans 4 months ago.
47783-magic_quotes_deprecation-getid3.patch (1.2 KB) - added by ayeshrajans 4 months ago.
47783-magic_quotes_deprecation-phpmailer.patch (740 bytes) - added by ayeshrajans 4 months ago.
47783-magic_quotes_deprecation-wp-core--2.patch (2.3 KB) - added by ayeshrajans 4 months ago.
Replaces 47783-magic_quotes_deprecation-wp-core.patch. PHPDoc for map_deep() function is updated to reflect the changes.
47783-magic_quotes_deprecation-PclZip.patch (13.8 KB) - added by ayeshrajans 4 months ago.
Juliette - Thanks you are right PclZip was missing in the patches. This one does it, and it turned out to be a rather big patch because the library had 2 methods to disable and swam back magic quotes functionality. Since this class is from 2009, there is no access indivators. However, I decided to remove the methods themselves and the class property because they started with name priv in properties, and the class property wasn't used anywhere else.
47783-2-magic_quotes_deprecation-phpmailer.patch (1.1 KB) - added by jrf 7 weeks ago.
Refreshed patch for PHPMailer PHP 7.4 compatibility

Download all attachments as: .zip

Change History (35)

#1 @SergeyBiryukov
4 months ago

Related:

#47704
PHP 7.4: Fix issues regarding accessing null/bool/etc as if they were an array
#47746
PHP 7.4 compatibility fix / implode argument order
#47751
PHP 7.4 compatibility fix / accessing arrays/string using curly brace syntax
#47783
PHP 7.4: Deprecations


#2 @jrf
4 months ago

Another related ticket:

  • #47441 PHP 7.4: use of deprecated operator precedence

Travis builds are currently not even completing properly due to the amount of warnings thrown

This is due to the commits related to PHP 7.4 deprecations having gone in this Monday.

All other issues from before then have already been addressed in the above tickets, including patching up external dependencies.

Note: as the related external dependencies have not (yet) released new versions, those dependencies in core have not been updated yet.
See the above mentioned tickets, most notably #47746 and #47751

A scan with the bleeding edge version of the PHPCompatibility scanner - i.e. including the open PRs #844 and #847 - should be able to detect most issues as all the sniffs regarding PHP deprecations which can be detected with a static scanner, have already been written.

Last edited 4 months ago by jrf (previous) (diff)

#3 @ayeshrajans
4 months ago

PHP 7.4 deprecates get_magic_quotes_gpc and get_magic_quotes_runtime. This was long due, and there are few issues in Trac about removing this functionality. Historically, those bits were there because we supported PHP 5.2-5.4, when magic quotes were being used in the wild.

With WordPress no longer supporting PHP versions < 5.6, we can now remove these checks completely.

There are 3 patches following this comment:

1. Fixes in WordPress core: 47783-magic_quotes_deprecation-wp-core.patch

This fixes all magic quote calls in core.

2. Fixes in external libs: Getid3: 47783-magic_quotes_deprecation-getid3.patch

This has come up in other tickets, and I think we can follow the same conclusion in other tickets whether we maintain our fork, or try to push upstream to update.

3. Fixes in external libs: PHPMaier: 47783-magic_quotes_deprecation-phpmailer.patch

PHPMailer is an active project, so I opened a PR https://github.com/PHPMailer/PHPMailer/pull/1794 with the same patch. We are using v5.2.2, although latest version is 5.2.7 in upstream.

#4 @ayeshrajans
4 months ago

Thanks @jrf - I was watching those tickets, and except for 3 instances in wordpress-importer plugin, all tests pass with your patches 👍. Thank you.

Current Travis build on 7.4snapshot: https://travis-ci.org/Ayesh/wordpress-develop/jobs/564094819

Last edited 4 months ago by ayeshrajans (previous) (diff)

#5 @jrf
4 months ago

getID3 is also an active project and has already accepted patches regarding the curly braces issue: https://github.com/JamesHeinrich/getID3

except for 3 instances in wordpress-importer plugin, all tests passes with your patches

The WordPress Importer issue the Core unit tests are running into has been fixed downstream already, there is just no new release yet: https://github.com/WordPress/wordpress-importer/pull/53

Other than that: if you want to keep track of how ready PHPCompatibility is, follow this ticket: #808

#6 @jrf
4 months ago

Reviewed 47783-magic_quotes_deprecation-wp-core.patch and looks sane to me.

get_magic_quotes_gpc() will always return false on PHP 5.4+, so removing the conditions related to this completely is the right course of action.


Reviewed 47783-magic_quotes_deprecation-getid3.patch and looks sane to me.


Reviewed 47783-magic_quotes_deprecation-phpmailer.patch and IMO, all references to magic quotes in the encodeFile() function can be removed as get_magic_quotes_runtime() will always return false on PHP 5.4+.
I.e.:

  • The code line now touched + the whole condition block below it.
  • The condition block a few lines further down.

PHPMailer currently has a minimum PHP requirement of PHP 5.5.0, so these changes can be safely made.


Refs:

#7 follow-up: @ayeshrajans
4 months ago

  • Keywords has-patch added

Thank you.
In the PHPMailer patch PHP_VERSION_ID < 70400 is supposed to prevent the get_magic_quotes_runtime() function from being called (so avoid the the deprecation notice), and still keep the $magic_quotes variable false. The idea was to not remove the $magic_quotes variable and two de-slash if blocks below it.

PHPMailer 5.2-stable branch doesn't show a lot of activity, so I tried to keep the PR as minimal as possible. This branch also supports PHP >= 5.0.0 (https://github.com/PHPMailer/PHPMailer/blob/5.2-stable/composer.json#L24). Only 6.x versions require PHP 5.5 minimum, and magic_quotes functionality is gone in those versions.

#8 in reply to: ↑ 7 ; follow-up: @jrf
4 months ago

  • Focuses coding-standards added

Replying to ayeshrajans:

Thank you.
In the PHPMailer patch PHP_VERSION_ID < 70400 is supposed to prevent the get_magic_quotes_runtime() function from being called (so avoid the the deprecation notice), and still keep the $magic_quotes variable false. The idea was to not remove the $magic_quotes variable and two de-slash if blocks below it.

PHPMailer 5.2-stable branch doesn't show a lot of activity, so I tried to keep the PR as minimal as possible. This branch also supports PHP >= 5.0.0 (https://github.com/PHPMailer/PHPMailer/blob/5.2-stable/composer.json#L24). Only 6.x versions require PHP 5.5 minimum, and magic_quotes functionality is gone in those versions.

In that case, as the minimum PHP version for WP is now 5.6, the PHPMailer patch should not be necessary and we should just upgrade PHPMailer to v 6.x.

#9 @jrf
4 months ago

I've done a quick scan and - while already silenced - it looks like the wp-admin/includes/class-plzip.php file, which is also an external dependency, also contains some magic quote related code round lines 5341/5345/5376.

#10 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 5.3

@ayeshrajans
4 months ago

Replaces 47783-magic_quotes_deprecation-wp-core.patch. PHPDoc for map_deep() function is updated to reflect the changes.

@ayeshrajans
4 months ago

Juliette - Thanks you are right PclZip was missing in the patches. This one does it, and it turned out to be a rather big patch because the library had 2 methods to disable and swam back magic quotes functionality. Since this class is from 2009, there is no access indivators. However, I decided to remove the methods themselves and the class property because they started with name priv in properties, and the class property wasn't used anywhere else.

#11 @jrf
3 months ago

  • Keywords php74 added

#12 in reply to: ↑ 8 @chesio
3 months ago

Replying to jrf:

In that case, as the minimum PHP version for WP is now 5.6, the PHPMailer patch should not be necessary and we should just upgrade PHPMailer to v 6.x.

Related: #40472 and #45822

Version 1, edited 3 months ago by chesio (previous) (next) (diff)

#13 @jorbin
2 months ago

In 46105:

GENERAL: Remove magic quote functions

The path to magic quote sanity took a fun and exciting turn: PHP core removed it and WordPress updated the minimum version.

For the formally external pclzip, the code is commented out to make investigating easier and in case we ever need to merge upstream (if that still exists) changes.

Props ayeshrajans, jrf, jorbin.
See #47783.
Fixes #18322.

#14 @jorbin
2 months ago

Keeping this open until we have updated the external libraries.

#15 follow-up: @jrf
2 months ago

FYI: https://github.com/JamesHeinrich/getID3/issues/200

Also: the code in GetID3 master does not contain the magic quotes related code anymore, so no additional patching should be needed once a new version has been tagged.

Last edited 2 months ago by jrf (previous) (diff)

#16 @jrf
2 months ago

GetID3 update ticket: #43836

#17 @jorbin
2 months ago

In 46112:

Update getID3 library to fix issues with PHP7.4

Updates to trunk version that includes fixes for PHP7.4

Changelog:
https://github.com/JamesHeinrich/getID3/compare/v1.9.14...00f3fbfd77e583099ca70a3cf0bc092e113d2b20

See: #47751,#47783.
Fixes: #48040.

#18 @jorbin
2 months ago

In 46113:

Comment out magic quote functions

Follow up to r46112.

See: #47751,#47783, #48040.

#19 in reply to: ↑ 15 @pierlo
2 months ago

Replying to jrf:

Also: the code in GetID3 master does not contain the magic quotes related code anymore, so no additional patching should be needed once a new version has been tagged.

GetID3 v2 now has its own namespace, and relies on Composer for autoloading. I guess a custom autoloader will be needed to use it.

#20 @jrf
8 weeks ago

Summary of current status of this ticket:

  • Everything has been addressed, safe for the PHPMailer issues.
  • PHPMailer has been updated to v 5.2.27 in [46097] via #40472.
  • #41750 is open to update PHPMailer to v 6.x

#41750 is milestoned for WP 5.3. If it lands in time, that should solve the remaining issue. If not, the PHPmailer patch in this ticket may need a refresh and should probably be applied.

#21 @davidbaumwald
7 weeks ago

@jrf Since #41750 was moved to 5.4, should this be punted as well so that they land together?

#22 follow-up: @jrf
7 weeks ago

@davidbaumwald No, this should not be punted.

As #41750 was punted, the 47783-magic_quotes_deprecation-phpmailer.patch patch should be applied for WP 5.3 to allow WP 5.3 to be compatible with PHP 7.4 when it comes out

The patch may need a refresh after the PHPMailer upgrade from #40472 though.

Last edited 7 weeks ago by jrf (previous) (diff)

#23 in reply to: ↑ 22 @davidbaumwald
7 weeks ago

Replying to jrf:

@davidbaumwald No, this should not be punted.

As #41750 was punted, the 47783-magic_quotes_deprecation-phpmailer.patch patch should be applied for WP 5.3 to allow WP 5.3 to be compatible with PHP 7.4 when it comes out

The patch it may need a refresh after the PHPMailer upgrade from #40472 though.

Got it. Thanks for the clarification. ;)

@jrf
7 weeks ago

Refreshed patch for PHPMailer PHP 7.4 compatibility

#24 @jrf
7 weeks ago

The patch I uploaded just now should apply cleanly and should be applied ASAP (as the upgrade to PHPMailer 6.0 won't happen in WP 5.3 anymore).

#25 @desrosj
6 weeks ago

In 46378:

General: Patch PHMailer for PHP 7.4 compatibility.

This patches the PHPMailer library in Core to be PHP 7.4 compatible by adding a version check before calling get_magic_quotes_runtime().

Props ayeshrajans, jrf.
See #47783.

#26 @desrosj
6 weeks ago

  • Keywords close needs-dev-note added

I believe that everything that can be addressed has been addressed here or in a separate ticket. Marking as a close. If someone else could run through and confirm there are no more changes needed and close out.

Also, just marking needs-dev-note because there will be a PHP 7.4 related note.

#27 @jrf
6 weeks ago

@desrosj Confirmed. Everything in this ticket has been addressed AFAICS.

#28 @jrf
6 weeks ago

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

#29 @desrosj
5 weeks ago

  • Keywords has-dev-note added; close needs-dev-note removed
Note: See TracTickets for help on using tickets.