Skip to content

Conversation

@b1ink0
Copy link
Contributor

@b1ink0 b1ink0 commented Mar 25, 2025

Summary

Fixes #1952

Relevant technical choices

This PR addresses an issue where the Image Prioritizer plugin fails to validate background image URLs when the server returns a Content-Type of application/octet-stream instead of an expected image/* Content-Type.

@b1ink0 b1ink0 added [Type] Bug An existing feature is broken [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) labels Mar 25, 2025
@b1ink0 b1ink0 added this to the image-prioritizer n.e.x.t milestone Mar 25, 2025
@b1ink0 b1ink0 changed the title Allow AVIF and other valid image URLs with incorrect application/octet-stream Content-Type Allow AVIF and other valid background image URLs with incorrect Content-Type Mar 25, 2025
@codecov
Copy link

codecov bot commented Mar 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.26%. Comparing base (217fda0) to head (0c6ff67).
Report is 29 commits behind head on trunk.

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk    #1956   +/-   ##
=======================================
  Coverage   72.25%   72.26%           
=======================================
  Files          85       85           
  Lines        6968     6958   -10     
=======================================
- Hits         5035     5028    -7     
+ Misses       1933     1930    -3     
Flag Coverage Δ
multisite 72.26% <100.00%> (+<0.01%) ⬆️
single 40.71% <100.00%> (+0.13%) ⬆️

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.

@b1ink0 b1ink0 marked this pull request as ready for review March 25, 2025 17:14
@github-actions
Copy link

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.

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

Co-authored-by: b1ink0 <[email protected]>
Co-authored-by: westonruter <[email protected]>

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

$file_extension = pathinfo( (string) wp_parse_url( $url, PHP_URL_PATH ), PATHINFO_EXTENSION );
$valid_image_file_extensions = array( 'jpg', 'jpeg', 'png', 'gif', 'webp', 'avif', 'heic', 'heif', 'svg' );
if ( '' !== $file_extension && in_array( strtolower( $file_extension ), $valid_image_file_extensions, true ) ) {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right because then the $content_length check below is not being accounted for.

Comment on lines 236 to 246
$content_type = (array) wp_remote_retrieve_header( $r, 'content-type' );
if ( ! is_string( $content_type[0] ) || ! str_starts_with( $content_type[0], 'image/' ) ) {
// This is a special case for images that are not served with the correct Content-Type.
if ( is_string( $content_type[0] ) && 'application/octet-stream' === $content_type[0] ) {
$file_extension = pathinfo( (string) wp_parse_url( $url, PHP_URL_PATH ), PATHINFO_EXTENSION );
$valid_image_file_extensions = array( 'jpg', 'jpeg', 'png', 'gif', 'webp', 'avif', 'heic', 'heif', 'svg' );
if ( '' !== $file_extension && in_array( strtolower( $file_extension ), $valid_image_file_extensions, true ) ) {
return true;
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like this instead?

Suggested change
$content_type = (array) wp_remote_retrieve_header( $r, 'content-type' );
if ( ! is_string( $content_type[0] ) || ! str_starts_with( $content_type[0], 'image/' ) ) {
// This is a special case for images that are not served with the correct Content-Type.
if ( is_string( $content_type[0] ) && 'application/octet-stream' === $content_type[0] ) {
$file_extension = pathinfo( (string) wp_parse_url( $url, PHP_URL_PATH ), PATHINFO_EXTENSION );
$valid_image_file_extensions = array( 'jpg', 'jpeg', 'png', 'gif', 'webp', 'avif', 'heic', 'heif', 'svg' );
if ( '' !== $file_extension && in_array( strtolower( $file_extension ), $valid_image_file_extensions, true ) ) {
return true;
}
}
$content_type = (array) wp_remote_retrieve_header( $r, 'content-type' );
if ( 'application/octet-stream' === $content_type[0] ) {
// This is a special case for images that are not served with the correct Content-Type.
$file_extension = pathinfo( (string) wp_parse_url( $url, PHP_URL_PATH ), PATHINFO_EXTENSION );
$valid_image_file_extensions = array( 'jpg', 'jpeg', 'png', 'gif', 'webp', 'avif', 'heic', 'heif', 'svg' );
$is_valid_content_type = in_array( strtolower( $file_extension ), $valid_image_file_extensions, true );
} else {
$is_valid_content_type = is_string( $content_type[0] ) && str_starts_with( $content_type[0], 'image/' );
}
if ( ! $is_valid_content_type ) {

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@westonruter westonruter changed the title Allow AVIF and other valid background image URLs with incorrect Content-Type Allow background image URLs for file types the web server doesn't know about, e.g. when AVIF is sent as application/octet-stream Mar 26, 2025
@westonruter westonruter merged commit f751ae2 into WordPress:trunk Mar 26, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Type] Bug An existing feature is broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image Prioritizer Not Saving URL Metrics for AVIF Background Images

2 participants