Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#49945 closed defect (bug) (fixed)

PHP 7.4+ notice in getid3_mp3::MPEGaudioHeaderValid()

Reported by: schlessera's profile schlessera Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: has-dev-note has-patch
Focuses: Cc:

Description

The getid3_mp3::MPEGaudioHeaderValid() method checks for an array key right at the beginning for an early return:

<?php
public static function MPEGaudioHeaderValid($rawarray, $echoerrors=false, $allowBitrate15=false) {
        if (($rawarray['synch'] & 0x0FFE) != 0x0FFE) {
                return false;
        }

As it can receive its data from the MPEGaudioHeaderDecode() method, which returns array|false, a corrupted MP3 stream will cause this check throw a notice:

Notice: Trying to access array offset on value of type bool in wp-includes/ID3/module.audio.mp3.php on line 1782

This was already fixed upstream:
https://github.com/JamesHeinrich/getID3/blob/master/getid3/module.audio.mp3.php#L1794

This fix needs to be ported over to Core for PHP 7.4+ support.

Attachments (1)

49945.1.patch (36.1 KB) - added by ayeshrajans 4 years ago.
A patch to update all existing files in ID3 library to its current version. No new files are added, only modified local files. I doubt we have tests for them though.

Download all attachments as: .zip

Change History (19)

#1 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.5

#2 @SergeyBiryukov
4 years ago

  • Component changed from Media to External Libraries

#3 @ayeshrajans
4 years ago

There seems to be other fixes in the 1.9.19 release. I'll send a patch to update the version.

@ayeshrajans
4 years ago

A patch to update all existing files in ID3 library to its current version. No new files are added, only modified local files. I doubt we have tests for them though.

#4 @SergeyBiryukov
4 years ago

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#5 @SergeyBiryukov
4 years ago

It looks like version 1.9.19 (released on Dec 18, 2019) does not include the PHP 7.4+ fix in question (committed on Dec 22, 2019), so it will have to be backported separately.

#6 follow-up: @SergeyBiryukov
4 years ago

In 47601:

External Libraries: Update getID3() to 1.9.19.

Changelog: https://github.com/JamesHeinrich/getID3/compare/v1.9.18...v1.9.19

Props ayeshrajans, schlessera.
See #49945.

#7 @SergeyBiryukov
4 years ago

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

In 47602:

External Libraries: Backport a commit from getID3() trunk to fix a PHP 7.4+ notice.

This addresses a "Trying to access array offset on value of type bool" notice in the getid3_mp3::MPEGaudioHeaderValid() method.

Props schlessera.
Fixes #49945.

#8 in reply to: ↑ 6 ; follow-up: @SergeyBiryukov
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[47601] caused some failures on Travis:

Deprecated: Function get_magic_quotes_runtime() is deprecated in /var/www/build/wp-includes/ID3/getid3.php on line 303
Deprecated: Function get_magic_quotes_gpc() is deprecated in /var/www/build/wp-includes/ID3/getid3.php on line 309
Deprecated: Function get_magic_quotes_runtime() is deprecated in /var/www/build/wp-includes/ID3/getid3.php on line 303
Deprecated: Function get_magic_quotes_gpc() is deprecated in /var/www/build/wp-includes/ID3/getid3.php on line 309

Those lines have a version_compare(PHP_VERSION, '7.4.0', '<') check above them, but for some reason are still executed in the PHP 7.4 job on Travis. It looks like php --version on all jobs reports PHP 7.2.15.

Those lines were previously commented out in [46113], but that was reverted in [47601] as seemingly not necessary with the version_compare() check brought from upstream.

I guess we'll have to comment them out again for the time being, until we figure out why version_compare() doesn't work.

#9 @SergeyBiryukov
4 years ago

In 47603:

External Libraries: Comment out magic quote functions in getID3().

For some reason, the version_compare() check does not work as expected on Travis.

Follow-up to [46113], [47601].

See #49945.

#10 in reply to: ↑ 8 ; follow-up: @SergeyBiryukov
4 years ago

Replying to SergeyBiryukov:

Those lines have a version_compare(PHP_VERSION, '7.4.0', '<') check above them, but for some reason are still executed in the PHP 7.4 job on Travis. It looks like php --version on all jobs reports PHP 7.2.15.

Per @ocean90, the PHP version of Travis doesn't matter as we're using Docker. The issue seems to be that the PHP version of the Docker image is still 7.4.0RC6.

#11 in reply to: ↑ 10 @ocean90
4 years ago

Replying to SergeyBiryukov:

The issue seems to be that the PHP version of the Docker image is still 7.4.0RC6.

https://github.com/WordPress/wpdev-docker-images/pull/26 should fix this.

#12 @SergeyBiryukov
4 years ago

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

In 47604:

External Libraries: Revert [47603].

With the Docker image for PHP 7.4 updated to stable version, this should no longer be necessary.

See https://github.com/WordPress/wpdev-docker-images/pull/26

Props ocean90.
Fixes #49945.

#13 @desrosj
4 years ago

  • Keywords has-dev-note added

#14 @Hareesh Pillai
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

There was a new version (v1.9.20) of getID3 released yesterday. Let us try to update to this version for WP 5.5

Last edited 4 years ago by Hareesh Pillai (previous) (diff)

This ticket was mentioned in PR #376 on WordPress/wordpress-develop by desrosj.


4 years ago
#15

  • Keywords has-patch added

#16 @desrosj
4 years ago

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

In 48278:

External Libraries: Update getID3 to version 1.9.20.

A full list of changes in this update can be found on GitHub: https://github.com/JamesHeinrich/getID3/compare/v1.9.19...v1.9.20.

Props hareesh-pillai, desrosj.
Previously [47601-47604].
Fixes #49945.

desrosj commented on PR #376:


4 years ago
#17

This was merged into core in https://core.trac.wordpress.org/changeset/48278.

#18 @SergeyBiryukov
4 years ago

#50582 was marked as a duplicate.

Note: See TracTickets for help on using tickets.