Skip to content

[6.x] Encode asset URLs - #15146

Open
duncanmcclean wants to merge 2 commits into
6.xfrom
encode-asset-urls
Open

[6.x] Encode asset URLs#15146
duncanmcclean wants to merge 2 commits into
6.xfrom
encode-asset-urls

Conversation

@duncanmcclean

Copy link
Copy Markdown
Member

This pull request fixes an issue where asset URLs weren't encoded, so filenames containing spaces or accented characters produced invalid URLs.

This was happening because Asset::url() and Asset::absoluteUrl() assembled the URL from the raw path, leaving characters like ú and spaces untouched. An asset called Dún Laoghaire_18 2.jpg came back as https://ams3.digitaloceanspaces.com/mywebsite/Dún Laoghaire_18 2.jpg, which Str::isUrl() doesn't consider a URL. Browsers percent-encode a raw src attribute for you, so images still rendered — the breakage only showed up server-side, where passing the URL into something like the Glide tag would send it down the wrong branch.

This PR fixes it by running the path through URL::encode() when assembling both URLs. This PR also decodes the path in AssetRepository::findByUrl(), so encoded URLs still resolve back to their asset.

Fixes #5593

duncanmcclean and others added 2 commits August 10, 2026 09:42
`Asset::url()` and `Asset::absoluteUrl()` returned the raw path, so filenames
containing spaces or accents produced invalid urls.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Keeps encoded urls resolving back to their asset.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@jasonvarga jasonvarga left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

URL::encode() (src/Facades/Endpoint/URL.php:326-346) was built for encoding a URL that may already be partially percent-encoded — it rawurlencode()s and then maps a set of sequences (%2F, %40, %3A, ... %25%) back to their literal characters so it doesn't double-encode an already-encoded URL. Reusing it here on a raw filesystem path (Asset.php's url()/absoluteUrl()) breaks that assumption: if a filename already contains a literal %XX-looking substring, it gets treated as "already encoded" and passes through untouched.

Repro: an asset with the literal filename photo%20one.jpg:

  • Asset::url() encodes it to .../photo%20one.jpg — unchanged, since the existing %20 is preserved as if it were already an encoded space.
  • AssetRepository::findByUrl()'s new rawurldecode() then turns that into photo one.jpg, which doesn't match the actual stored path photo%20one.jpg. The asset becomes unresolvable via the url()findByUrl() round trip.

This is narrow (filenames containing literal percent-encoded-looking sequences), but it's a real regression — these assets resolved fine before this change since neither side encoded/decoded. None of the added test cases (nothing to encode, spaces, accents, spaces in folders) cover a literal %XX substring, so it's not caught by CI.

Needs fixing before merge: don't reuse URL::encode() (designed for full URLs) on raw asset paths. A plain per-segment rawurlencode() — without the "preserve already-encoded" table — would round-trip correctly through rawurldecode() for all inputs, since it never assumes the raw path is pre-encoded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Asset url encoding issues: spaces and accents

2 participants