Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.97%. Comparing base (e793289) to head (c3c2512).
Report is 12 commits behind head on trunk.

@@            Coverage Diff             @@
##            trunk    #1802      +/-   ##
==========================================
+ Coverage   65.92%   65.97%   +0.04%     
==========================================
  Files          88       88              
  Lines        6885     6895      +10     
==========================================
+ Hits         4539     4549      +10     
  Misses       2346     2346              

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

The solution encodes only the filename part of URLs

What about when an internationalized domain name is used? Couldn't this also cause problems with encoding?

Additionally, on multisite subdirectory installs, in theory the path before wp-content could also include non-ASCII chars:

https://testsite.com/חנות/wp-content/uploads/2025/01/example-scaled.avif

@AhmarZaidi Hey, are you intending to pick this up again?

@westonruter Yes, I'll be working on the changes right away.

Instead of parsing the URL and then re-constructing it, what if you just check if the href has any non-ASCII chars and then encode the entire URL

@westonruter I've implemented the changes: If the path contains any non-ascii characters then we encode the whole URL.

However we can use preg_replace_callback to encode only the non-ascii characters in the URL. Let me know if that approach will be better.

What you did looks good. Could you add some test cases for international domain names and non-ASCII paths to ensure the expected output?

@westonruter The old solution was encoding the :// part also, so I've update the code slightly.
Current solution: Encode complete url after :// part.

Potential Issue: This solution converts the slashes (/) to %2F so the file path would be incorrect.
For example https://xn--fsq.com/תמונה.jpg will be encoded to https://xn--fsq.com%2F%D7%AA%D7%9E%D7%95%D7%A0%D7%94.jpg
Note: I've written the tests according to the above output.

If we use something like:

$decoded_url = urldecode( $link['href'] );
$encoded_url = preg_replace_callback(
        '/[^\x00-\x7F]/',
        fn( $matches ) => rawurlencode( $matches[0] ),
        $decoded_url
);

Then https://xn--fsq.com/תמונה.jpg will be encoded to https://xn--fsq.com/%D7%AA%D7%9E%D7%95%D7%A0%D7%94.jpg

Please feel free to let me know if we should do any further changes to the approach.

If we use something like:

That looks good!

Implemented the changes.

So this is no longer a draft and is ready for review, correct?

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: @amitay-elementor.

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: amitay-elementor.

Co-authored-by: AhmarZaidi <[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.

Relevant technical choices

Could you update the description based on the latest revisions?

We forgot about the URLs in the imagesrcset. I've opened #1866 to fix that.

Follow-up issue: #1906