Fixes palette-based PNG uploads failing original full-size AVIF/WebP conversion under GD #2024

b1ink0 · 2025-05-27T12:23:03Z

Summary

Fixes #2018, #1561, #1864

Relevant technical choices

This change hooks into wp_handle_upload_prefilter to detect palette-based (indexed-color) PNGs and convert them in-place to truecolor while preserving transparency before any AVIF/WebP encoding takes place. It preserves all existing upload metadata and only affects PNGs handled by the GD editor.

codecov · 2025-05-27T12:32:11Z

Codecov Report

Attention: Patch coverage is 73.52941% with 9 lines in your changes missing coverage. Please review.

Project coverage is 67.77%. Comparing base (05444d4) to head (9ff40f2).
Report is 87 commits behind head on trunk.

Files with missing lines	Patch %	Lines
plugins/webp-uploads/hooks.php	73.52%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk    #2024      +/-   ##
==========================================
- Coverage   68.06%   67.77%   -0.30%     
==========================================
  Files          92       93       +1     
  Lines        7626     7698      +72     
==========================================
+ Hits         5191     5217      +26     
- Misses       2435     2481      +46

Flag	Coverage Δ
multisite	`67.77% <73.52%> (-0.30%)`	⬇️
single	`36.69% <0.00%> (-0.34%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…te PNGs to truecolor

github-actions · 2025-05-30T12:39:08Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @IlyaZha, @mytory, @chimok.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: IlyaZha, mytory, chimok.

Co-authored-by: b1ink0 <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: adamsilverstein <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: benniledl <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

felixarntz

@b1ink0 Thank you, this looks great to me so far, just one comment.

It would be great to get @adamsilverstein's review on this as well.

felixarntz · 2025-05-30T16:02:17Z

plugins/webp-uploads/hooks.php

+
+	return $file;
+}
+add_filter( 'wp_handle_upload_prefilter', 'webp_uploads_convert_palette_png_to_truecolor' );


You're already using this in the test, but wouldn't it make sense to also add the same function as a hook callback to wp_handle_sideload_prefilter?

Yes, it makes sense. At first, I thought wp_handle_sideload and media_handle_sideload, which trigger the wp_handle_sideload_prefilter, were not used in core, but after some digging, I found that core does trigger this filter.

Done in 8ad675c

… hooks

plugins/webp-uploads/hooks.php

westonruter · 2025-06-04T21:23:59Z

plugins/webp-uploads/hooks.php

+		return $file;
+	}
+
+	if ( 'png' !== strtolower( pathinfo( $file['name'], PATHINFO_EXTENSION ) ) ) {


Should this look at the attachment metadata instead to see if the MIME type is image/png?

Yes, we can just check if $file['type'] is image/png.

Done in 7443e68.

Co-authored-by: Weston Ruter <[email protected]>

adamsilverstein · 2025-06-10T12:08:28Z

Hi @b1ink0 and thanks for the PR.

detect palette-based (indexed-color) PNGs and convert them in-place to truecolor while preserving transparency before any AVIF/WebP encoding takes place

Good catch! I understand the technical requirement for GD here and agree we need to address them to resolve this issue.

My only concern is that by converting a palette image (up to 8 bit) to truecolor (eg. 24 bit), we might inadvertently bloat the image size. We tried to address this for Imagick in https://core.trac.wordpress.org/ticket/36477. I'd love to see some before/after size numbers for indexed PNG uploads converted to WebP and AVIF to confirm that isn't the case. I suspect we are ok given the lossy compression of WebP/AVIF vs the lossless PNG compression, but it would be worth checking and perhaps adding a unit test for this aspect.

plugins/webp-uploads/hooks.php

adamsilverstein · 2025-06-10T12:11:17Z

plugins/webp-uploads/hooks.php

+/**
+ * Converts palette PNG images to true color PNG images.
+ *
+ * GD cannot convert palette-based PNG to WebP/AVIF formats, causing conversion failures.


I wonder if this fix belongs in core itself sice we inherently support this type of conversion with a simple filter. If we don't fix automatically, at the very least, we should inform the user why the upload failed and how to fix it.

I located the Trac ticket: #59339 "Conversion to webp causes fatal error when original image is a palette image (as opposed to truecolor)"

Should we post an update there and start the discussion on where to go next?

adamsilverstein · 2025-06-10T12:16:42Z

plugins/webp-uploads/hooks.php

+
+	// Check if required GD functions exist.
+	if (
+		! function_exists( 'imagecreatefrompng' ) ||


+1 to defensive programming; however all of these functions have been around since PHP4/5 as as far as I know should be available to all WordPress installs with GD. So maybe we can remove this check?

That would address the code coverage issue as well, if it is impossible to create a scenario where any of these functions don't exist.

Oh, but actually will these functions be available if GD is not installed? If PHP is not compiled with --with-gd then I don't think they will.

If PHP is not compiled with GD, then these functions will not be available. I compiled PHP with and without GD and tested it; the version without GD did not have these functions.

adamsilverstein · 2025-06-10T12:21:14Z

plugins/webp-uploads/hooks.php

+	}
+
+	$image = imagecreatefrompng( $file['tmp_name'] );
+	if ( false === $image || imageistruecolor( $image ) ) {


I'm confused by this check. If $image is false, the conversion failed. In that case, don't we want to continue to the processing below?

When imagecreatefrompng() returns false, it means the creation of the PNG failed. In that case, we can't convert it to truecolor because we need a valid GDImage resource for imagepalettetotruecolor() to work properly. Returning early in this scenario is the best choice since there's no valid image resource to operate on.

I also combined two different checks in that condition, which I realize makes the logic less clear. Separate those out in 7f757c7 to improve readability.

adamsilverstein · 2025-06-10T12:21:45Z

plugins/webp-uploads/hooks.php

+	}
+	imagedestroy( $image );
+
+	return $file;


Should we set $file['name'] to the new file name before returning $file?

If you're asking whether the conversion has changed the name of the file, it has not - we simply overwrote the same file with the converted version.

However, if you're suggesting that users should see a different filename when an image is converted to truecolor, we could add a suffix like -truecolor to the filename. But I'm not sure whether we should do that, since this conversion is meant to be a transparent internal optimization rather than a user-visible change.

adamsilverstein · 2025-06-10T12:22:46Z

plugins/webp-uploads/tests/test-load.php

+				$modern_image_format_path = get_attached_file( $attachment_id );
+				$this->assertNotFalse( $modern_image_format_path );
+				$this->assertFileExists( $modern_image_format_path );
+				$this->assertGreaterThan( 0, (int) filesize( $modern_image_format_path ) );


can we also add an assertion that the new image filesize isn't larger than the original image?

Done in 56e8dac.

adamsilverstein · 2025-06-10T12:24:53Z

plugins/webp-uploads/tests/test-load.php

+				'image_path'       => $non_palette_png,
+				'expected_changed' => false,
+				'set_up'           => function (): void {
+					add_filter( 'wp_image_editors', array( $this, 'use_gd_image_editor' ) );


can the test just add this filter always, without needing a "set_up" attribute?

Done in e646941.

adamsilverstein · 2025-06-10T12:25:40Z

plugins/webp-uploads/tests/test-load.php

+				'set_up'           => function (): void {
+					add_filter( 'wp_image_editors', array( $this, 'use_gd_image_editor' ) );
+				},
+				'tear_down'        => function (): void {


tear_down should not be required as all hooks should be reset between test runs as far as I know. Can you try removing?

Yeah, we can remove this. I had added them because I was also testing using the ImageMagick editor, and since filters don't get removed between iterations of the data provider, I had to add setup and teardown.

But I removed the test for ImageMagick as I noticed ImageMagick wasn't working correctly when I ran the tests. When I tried to log Imagick::queryFormats(), it returned an empty array. I'm not sure why - I tried running php -r "print_r(Imagick::queryFormats());" in the wp-env created tests-wordpress container and it gave an array with all supported formats, but when I ran the same command in tests-cli it gave an empty array. I'm not sure - do tests run in the tests-cli container and not the tests-wordpress container?

Done in e646941.

Co-authored-by: Adam Silverstein <[email protected]>

…and teardown parameters

b1ink0 · 2025-06-17T09:53:26Z

I'd love to see some before/after size numbers for indexed PNG uploads converted to WebP and AVIF to confirm that isn't the case. I suspect we are ok given the lossy compression of WebP/AVIF vs the lossless PNG compression, but it would be worth checking and perhaps adding a unit test for this aspect.

I ran some tests on a few indexed PNGs. As expected, converting them to truecolor PNGs increased the file size up to 2.5x in some cases. However, when these converted truecolor PNGs were then encoded to AVIF or WebP, the resulting file sizes were consistently smaller than the original indexed PNGs encoded directly to AVIF/WebP.

Indexed PNG Size	Truecolor PNG Size	AVIF Size (from Truecolor PNG)	WebP Size (from Truecolor PNG)
7 KB	15 KB	3 KB	2 KB
65 KB	116 KB	4 KB	3 KB
41 KB	87 KB	26 KB	16 KB
1.4 MB	3.8 MB	674 KB	628 KB
177 KB	426 KB	149 KB	107 KB

cc: @adamsilverstein

Add function to convert palette PNGs to true color during upload

4d54810

b1ink0 added this to the webp-uploads n.e.x.t milestone May 27, 2025

b1ink0 added [Type] Bug An existing feature is broken [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) labels May 27, 2025

This was linked to issues May 27, 2025

Fatal error: Paletter image not supported by webp #1561

Open

Transparent .pngs with colour type Palette cannot be converted #1864

Open

b1ink0 added 2 commits May 27, 2025 18:29

Refactor to use imagepalettetotruecolor() to convert uploaded palet…

82cf154

…te PNGs to truecolor

Add test for converting palette PNG to truecolor

cf97730

b1ink0 marked this pull request as ready for review May 30, 2025 12:38

b1ink0 requested review from adamsilverstein, felixarntz and westonruter as code owners May 30, 2025 12:38

felixarntz reviewed May 30, 2025

View reviewed changes

b1ink0 added 2 commits May 30, 2025 23:35

Add hooks for palette PNG to truecolor conversion and tests for those…

8ad675c

… hooks

Merge branch 'trunk' into fix/palette-pngs-to-avif-webp-conversion

85482bc

westonruter reviewed Jun 4, 2025

View reviewed changes

b1ink0 and others added 2 commits June 6, 2025 00:25

Add type safety guard for palette PNG conversion filter

ef9a9f9

Co-authored-by: Weston Ruter <[email protected]>

Improve PNG type checking

7443e68

adamsilverstein reviewed Jun 10, 2025

View reviewed changes

plugins/webp-uploads/hooks.php Outdated Show resolved Hide resolved

adamsilverstein reviewed Jun 10, 2025

View reviewed changes

Fix typo in function description "true color" to "truecolor"

731d3b0

Co-authored-by: Adam Silverstein <[email protected]>

b1ink0 added 2 commits June 11, 2025 16:20

Separate image creation failure and truecolor checks for clarity

7f757c7

Refactor test for palette PNG conversion to remove unnecessary setup …

e646941

…and teardown parameters

Add test to assert converted image filesize does not exceed original

56e8dac

b1ink0 requested review from westonruter, adamsilverstein and felixarntz June 17, 2025 10:21

Remove unused import

9ff40f2

Fixes palette-based PNG uploads failing original full-size AVIF/WebP conversion under GD #2024

Are you sure you want to change the base?

Fixes palette-based PNG uploads failing original full-size AVIF/WebP conversion under GD #2024

Uh oh!

Conversation

b1ink0 commented May 27, 2025

Summary

Relevant technical choices

Uh oh!

codecov bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unlinked Accounts

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b1ink0 Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsilverstein commented Jun 10, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b1ink0 Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b1ink0 commented Jun 17, 2025

Uh oh!

Uh oh!

codecov bot commented May 27, 2025 •

edited

Loading

github-actions bot commented May 30, 2025 •

edited

Loading

b1ink0 Jun 5, 2025 •

edited

Loading

b1ink0 Jun 11, 2025 •

edited

Loading