-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add percent encoding of URLs in imagesrcset
param of Link
response header
#1866
base: release/3.9.0
Are you sure you want to change the base?
Conversation
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/3.9.0 #1866 +/- ##
=================================================
+ Coverage 66.64% 66.70% +0.06%
=================================================
Files 88 88
Lines 7015 7029 +14
=================================================
+ Hits 4675 4689 +14
Misses 2340 2340
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
}, | ||
$decoded_url | ||
); | ||
return esc_url_raw( $encoded_url ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This was originally esc_url_raw( $encoded_url ?? '' )
. However, this seems to have just been a way to work around the fact that preg_replace_callback()
can technically return null
when an invalid pattern is provided. However, since we know the pattern is correct based on tests, the use of (string)
cast seems more appropriate.
95ee543
to
e731b4b
Compare
In testing Optimization Detective for the release in 2 days, I discovered a problem when I upload an image file with non-ASCII filename (
البيسون.jpg
):It's returning a 404 because the image URLs in the
imagesrcset
aren't getting the same encoding as is the original URL in thehref
.Link: <https://complicated-lyrebird-c797ca.instawp.xyz/wp-content/uploads/2025/02/%D8%A7%D9%84%D8%A8%D9%8A%D8%B3%D9%88%D9%86-1024x668.jpg>; rel="preload"; fetchpriority="high"; as="image"; imagesrcset="https://complicated-lyrebird-c797ca.instawp.xyz/wp-content/uploads/2025/02/البيسون-1024x668.jpg 1024w, https://complicated-lyrebird-c797ca.instawp.xyz/wp-content/uploads/2025/02/البيسون-300x196.jpg 300w, https://complicated-lyrebird-c797ca.instawp.xyz/wp-content/uploads/2025/02/البيسون-768x501.jpg 768w, https://complicated-lyrebird-c797ca.instawp.xyz/wp-content/uploads/2025/02/البيسون-1536x1002.jpg 1536w, https://complicated-lyrebird-c797ca.instawp.xyz/wp-content/uploads/2025/02/البيسون-2048x1336.jpg 2048w"; imagesizes="(782px < width) 644px, (max-width: 1024px) 100vw, 1024px"; media="screen and (782px < width)"
Expected versus actual:
This is something we missed in #1802 while fixing #1775.
So this PR adds URL encoding of the
imagesrcset
: