Fatal Error Media Upload MP3 PHP 8.2 when metadata contains list

[sllimrovert]

When uploading an MP3 file that contains meta data for "Involved People" ( which is a list, not a simple string ), if the server is running PHP 8.2, a fatal error happens when the wp_add_id3_tag_data function calls wp_kses_post. The fatal error happens on vanilla WordPress when uploading the file to the Media Library ( verified in latest WP 6.8.1 ).

In general, wp_kses causes a fatal error if an array is passed into it as $content.

I'll attach the file to this ticket that has caused the error for me. I do not own the copyright on the file.

[sllimrovert]

An MP3 file with metadata for Involved People

[ankitkumarshah]

I was able to successfully reproduce this issue on my test environment.

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.4

Screenshot:

As far as from the error stack and as also mentioned in ticket description it occurs because when wp_add_id3_tag_data() processes the ID3 metadata and passes array values to wp_kses_post(), which expects string input only.

I’d be happy to look into this further.

The wp_add_id3_tag_data() function was passing potentially non-string values to wp_kses_post(), which expects string input only. This change adds type checking and conversion to ensure all values are properly converted to strings before sanitization.

• Add type checking for ID3 metadata values

• Convert array values to space-separated strings

• Cast non-string values to strings

• Maintain existing sanitization and security measures

Trac ticket: https://core.trac.wordpress.org/ticket/63529

[rollybueno]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/8901.diff

Environment

• WordPress: 6.8.1
• PHP: 8.2.27
• Server: nginx/1.26.1
• Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.2.27)
• Browser: Chrome 136.0.0.0
• OS: Linux
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Query Monitor 3.17.2
  • Test Reports 1.2.0
  • User Switching 1.9.2
  • WordPress Beta Tester 3.6.3

Actual Results

1. ✅ Issue resolved with patch.

Supplemental Artifacts

I have recorded my screen that shows the wp_add_id3_tag_data() properly handles array() before passing to the wp_kses_posts(): ​https://drive.google.com/file/d/1eYrnHVaxkuCZDIjr8GJd96abXJ22nG11/view?usp=drive_link

Thanks for the patch! I've tested this fix, and it successfully resolves the fatal error when uploading MP3 files with complex ID3 metadata.

#### Environment tested:

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.4

#### Screencast:
​https://github.com/user-attachments/assets/b7047d5e-aec3-4d96-a914-076a3dc72d87

[wpfy]

Reproduction Report

Description

I was able to successfully reproduce this issue on my test environment.

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.25.4
• Database: mysqli (Server: 8.0.42 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Four 1.3
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

I got this from the browser console async-upload.php -> Network -> Preview

Warning: Array to string conversion in /Users/akramul/Herd/wp-develop/src/wp-includes/kses.php on line 1807

Fatal error: Uncaught TypeError: str_contains(): Argument #1 ($haystack) must be of type string, array given in /Users/akramul/Herd/wp-develop/src/wp-includes/blocks.php:1984 Stack trace: #0 /Users/akramul/Herd/wp-develop/src/wp-includes/blocks.php(1984): str_contains(Array, '<!--') #1 /Users/akramul/Herd/wp-develop/src/wp-includes/formatting.php(5210): filter_block_content(Array, 'post', Array) #2 /Users/akramul/Herd/wp-develop/src/wp-includes/class-wp-hook.php(324): wp_pre_kses_block_attributes(Array, 'post', Array) #3 /Users/akramul/Herd/wp-develop/src/wp-includes/plugin.php(205): WP_Hook->apply_filters(Array, Array) #4 /Users/akramul/Herd/wp-develop/src/wp-includes/kses.php(948): apply_filters('pre_kses', Array, 'post', Array) #5 /Users/akramul/Herd/wp-develop/src/wp-includes/kses.php(754): wp_kses_hook(Array, 'post', Array) #6 /Users/akramul/Herd/wp-develop/src/wp-includes/kses.php(2234): wp_kses(Array, 'post') #7 /Users/akramul/Herd/wp-develop/src/wp-admin/includes/media.php(3537): wp_kses_post(Array) #8 /Users/akramul/Herd/wp-develop/src/wp-admin/includes/media.php(3746): wp_add_id3_tag_data(Array, Array) #9 /Users/akramul/Herd/wp-develop/src/wp-admin/includes/media.php(324): wp_read_audio_metadata('/Users/akramul/...') #10 /Users/akramul/Herd/wp-develop/src/wp-admin/async-upload.php(113): media_handle_upload('async-upload', 0) #11 /Applications/Herd.app/Contents/Resources/valet/server.php(167): require('/Users/akramul/...') #12 {main} thrown in /Users/akramul/Herd/wp-develop/src/wp-includes/blocks.php on line 1984
There has been a critical error on this website. Please check your site admin email inbox for instructions. If you continue to have problems, please try the support forums.

Learn more about troubleshooting WordPress.

​https://share.cleanshot.com/mFhRHvKS

[rollybueno]

Hey @wpfy - would you mind to test the patch that I've submitted 7 hours ago? @ankitkumarshah confirmed on his end, so another set of patch testing would be great!

[rollybueno]

Removing the "needs-testing" since both https://core.trac.wordpress.org/ticket/63529#comment:4 & https://core.trac.wordpress.org/ticket/63529#comment:5 confirms it.

Feel free to re-test the patch more and send feedback

[wpfy]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8901

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.25.4
• Database: mysqli (Server: 8.0.42 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Four 1.3
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

✅ Issue resolved with patch, mp3 with metadata (list) uploaded successfully!

​https://share.cleanshot.com/2vtQKSnx

[hmbashar]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8901

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.25.4
• Database: mysqli (Server: 8.0.41 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: macOS
• Theme: Twenty Eleven 4.9
• MU Plugins: None activated
• Plugins:
  • Revix Reviews ❤️ – All-in-One Business Review Manager 1.2.3
  • Test Reports 1.2.0

Actual Results

1. ✅ Issue resolved with patch.

Supplemental Artifacts

[SirLouen]

Replying to rollybueno:

Removing the "needs-testing" since both https://core.trac.wordpress.org/ticket/63529#comment:4 & https://core.trac.wordpress.org/ticket/63529#comment:5 confirms it.

Feel free to re-test the patch more and send feedback

We have selected this ticket for the Contributors Day in WCEU so it will have enough testing reports

But be aware that #comment 4 is a test from yourself, and reporting a test from yourself your own patch is not useful (we assume that you test your own patches, otherwise you would not send them)

[johnbillion]

The wp_add_id3_tag_data() doesn't yet have any unit test coverage but it would be good to start.

Would somebody like to work on tests for this? The new test class can perhaps go into tests/phpunit/tests/media.

[ankitkumarshah]

Hi @johnbillion,
I’d be happy to work on writing unit tests for this!

[rollybueno]

Of course :)

Replying to SirLouen:

Replying to rollybueno:

Removing the "needs-testing" since both https://core.trac.wordpress.org/ticket/63529#comment:4 & https://core.trac.wordpress.org/ticket/63529#comment:5 confirms it.

Feel free to re-test the patch more and send feedback

We have selected this ticket for the Contributors Day in WCEU so it will have enough testing reports

But be aware that #comment 4 is a test from yourself, and reporting a test from yourself your own patch is not useful (we assume that you test your own patches, otherwise you would not send them)

[sllimrovert]

In my opinion, the choice to flatten arrays into strings for the metadata loses the inherent nature of that meta data. The meta data for "Involved People" is a list and should remain a list so that places that might choose to use it can work with it as a list. There are other examples of metadata that remain an array - like image just a little lower in the function.

The kses is run to ensure no troublesome characters or scripts make it through. I think it would be more robust to walk through the array and run kses on all values. I think the wp_kses_post_deep might be helpful with that.

[huzaifaalmesbah]

Test Report

Description

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8901

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 9.3.0 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅ Issue resolved with patch.

Screenshots

Before Apply Patch	After Apply Patch ✅

[sllimrovert]

I've confirmed locally that swapping:

$metadata[ $key ] = wp_kses_post( reset( $list ) );

for:

$metadata[ $key ] = wp_kses_post_deep( reset( $list ) );

also solves the issue, but keeps the structure of the metadata intact.

[rollybueno]

Hi @ankitkumarshah - I have moved it to separate ticket for better progress tracking. You can check on https://core.trac.wordpress.org/ticket/63539

Replying to ankitkumarshah:

Hi @johnbillion,
I’d be happy to work on writing unit tests for this!

[rollybueno]

Thanks! I think this should be the accepted patch version. I'll confirm it first on my dev, then updating the PR.

Replying to sllimrovert:

I've confirmed locally that swapping:

$metadata[ $key ] = wp_kses_post( reset( $list ) );

for:

$metadata[ $key ] = wp_kses_post_deep( reset( $list ) );

also solves the issue, but keeps the structure of the metadata intact.

Trac ticket: https://core.trac.wordpress.org/ticket/63529

A new patch created as suggested at: https://core.trac.wordpress.org/ticket/63529#comment:18

by replacing
$metadata[ $key ] = wp_kses_post( reset( $list ) );

with:
$metadata[ $key ] = wp_kses_post_deep( reset( $list ) );

[sandeepdahiya]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8926.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Firefox 139.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅ Media file with metadata is uploading as expected.

Additional Notes

• Used this file: -https://core.trac.wordpress.org/attachment/ticket/63529/01%20Back%20in%20Spring_MASTER.mp3

Supplemental Artifacts

Test screenshot: ​https://ibb.co/Rkr2Qw58

[SirLouen]

#63539 was marked as a duplicate.

Trac ticket: 63529

[rollybueno]

Added unit test on ​https://github.com/WordPress/wordpress-develop/pull/8901/files#diff-1828470281074506315d482d58fa75908b8a4b060c83df15db3b6a0deec1ce16 for 12 different test data handling.

This is the command:

npm run test:php -- --filter wpAddId3TagData

Hey @rollybueno,

Thank you for adding the comprehensive unit tests! I've run the test suite and everything looks good 👍

### Test Results:

I'll be closing PR #8935 in favor of this implementation since it includes proper test coverage.

Great work 🎉!

### Test Environment:

• PHPUnit 9.6.23
• Running in Docker environment

[ante1974]

Good day,

Do we know when this fix will be included in a WordPress Update?

Could someone kindly point me to the procedure for raising a related bug?

Thankyou.

[SirLouen]

@ankitkumarshah it's OK to run Unit Tests for a quick check that everything is in order, but running Unit Tests is only useful, when you are actually checking a Testing patch

For this case, manual code review is needed.

In this case, Unit Tests look very redundant, lacking of data providers (that could largely benefit in these tests) and the most important thing: they are not covering the main concern of this topic.

PS: My advice here, if anyone plans to use an LLM for unit tests, better leave these tests undone. You can benefit from an LLM for some inspiration, but not for building the whole testing suite. I've been benchmarking all LLM providers from unit testing for the last 18 months, and they are painfully bad. Even in benchmarks when explaining the test case in detail, they fail miserably. Probably the sole thing that they have not been able to achieve yet with any degree of quality. I will report back when I finally see a grasp of light on quality LLM Unit testing.

[rollybueno]

Updated the test unit to focus on the main issue in this task. The test data tructure will closely resemble the attachment file metadata which has

[
    'album' => 'Map the Dark',
    'title' => 'Back in Spring',
    'artist' => 'Quote the Raven',
    'albumartist' => 'Quote the Raven',
    'tracknumber' => '1/9',
    'isrc' => 'CAFOV2400002',
    'date' => '2024',
    'encoder' => 'WaveLab Pro 11.1.20',
    'involved_people' => [
        [
            'role' => 'Mastered by Justin Perkins @ Mystery Room Mastering',
            'name' => ''
        ]
    ]
]

especially the 'involved_people' key which is an array that causing the fatal error.

​https://github.com/WordPress/wordpress-develop/pull/8901/files#diff-1828470281074506315d482d58fa75908b8a4b060c83df15db3b6a0deec1ce16

### Test Report
Description
---

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8901

Environment
---

• WordPress: 6.9-alpha-20250730.125708
• PHP: 8.0.30
• Server: nginx/1.26.1
• Database: mysqli (Server: 5.7.28 / Client: mysqlnd 8.0.30)
• Browser: Chrome 138.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Five 1.3
• MU Plugins: None activated
• Plugins:
  • Check & Log Email - Easy Email Testing & Mail logging 2.0.8
  • Test Reports 1.2.0

Actual Results
---

1. ✅ Issue resolved with patch.

Note: This is tested on my local working as expected.

Supplemental Artifacts
---

[poojapadamad]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8901

Environment

• WordPress: 6.9-alpha-20250728.130313
• PHP: 7.4.31-dev
• Server: PHP.wasm
• Database: WP_SQLite_Driver (Server: 5.5 / Client: 3.40.1)
• Browser: Chrome 138.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Five 1.3
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅Issue resolved with patch and no error is displayed.

Supplemental Artifacts

Attachment -https://core.trac.wordpress.org/attachment/ticket/63529/New%20Note.jpeg

[joedolson]

In 61098:

Media: Prevent fatal error if metadata is an array.

Check the data type of ID3 data on import before running wp_kses_post or wp_kses_post_deep to fix a fatal error thrown when attempting to run wp_kses_post on non-string content in PHP 8.2+.

Adds unit tests to verify.

Props sllimrovert, ankitkumarshah, rollybueno, wpfy, hmbashar, SirLouen, johnbillion, huzaifaalmesbah, sandeepdahiya, sourabhjain, poojapadamad, joedolson.
Fixes #63529.