Add percent encoding of URLs in imagesrcset param of Link response header#1866
Add percent encoding of URLs in imagesrcset param of Link response header#1866westonruter merged 1 commit intorelease/3.9.0from
imagesrcset param of Link response header#1866Conversation
|
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.
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
felixarntz
left a comment
There was a problem hiding this comment.
@westonruter Great catch, LGTM!
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
imagesrcsetaren'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: