Support absolute URLs that point to public stream wrapper files in FileUrlGenerator

I've attached a POC patch.

Before using in production it needs a bit more checking and maybe improving and definitely tests. Concept seems OK - it just converts back what looks like a public URL into a public:// stream wrapper, which regardless of input should be safe as anything public:// is public. Flagging for review for the premise and can look at tests if the approach is OK.

Huh!?

I’d swear that the Crop + CDN modules worked together just fine in the past, and I do not recall the CDN module rewriting already absolute file URLs? 😅

Also: why do stream wrappers not support query strings? 🙃 That seems like a bug in core. I’ll investigate … eventually.

Currently absolutely no time for the CDN module, I have my hands more than full with the Experience Builder module 😇

Heh yeh. It never has. The getExternalUrl() on the PublicWrapper encodes the target so if it contains a query string it becomes encoded in the path like a path component containing %3D. It seems it's been like this since it was originally written for the first Drupal 8. Likely it could be sorted but similarly I didn't have time to dig through Drupal on the potential breaks if there are any places assuming a path - and I'm pretty sure there are some - most places assume that public://xxxx means you can just access the local filesystem on public-directory/xxxx - and call realpath and all kinds on it. So the impact of allowing a query string on public://, could be, (I haven't dug too far), a very large BC break.

What if … the Crop module detected the presence of the CDN module, and if installed, it would just apply the query string to the public:// file URI? Then the CDN module wouldn't have to do reverse parsing.

Implemented in https://git.drupalcode.org/project/cdn/-/merge_requests/32/diffs?commit_....

Then if/when this is fixed in core, the logic that that commit added could simply be deleted.

@wim-leers Sounds like a plan! I will look at dropping a ticket across to Crop to not convert to absolute URI if CDN is installed. Then similarly to here, once core is updated, they can also drop their absolute URI completely.

Just in case anyone looks at my patch, I've hidden it because it needs work - it causes double encoding because the conversion of the absolute URI back to public:// doesn't decode path components so https://xxx/sites/default/files/file%20space.png becomes public://file%20space.png but actually, and this is probably ANOTHER thing Core needs to sort out, the original would have been "public://file space.png" - so effectively you end up with CDN creating a URL from public://file%20space.png of https://xxx/sites/default/files/file%2520space.png which of course then 404s... Good fun!