Problem/Motivation
FlysystemRouteSubscriber builds the image style route paths by
concatenating a leading slash with the public stream wrapper's directory path:
'/' . $directory_path . '/styles/{image_style}/{scheme}'
and it also rewrites core's image.style_public route the same
way.
When the public scheme is itself Flysystem-backed and its
public_url_base is a bare host with no path component (a CDN or
bucket domain such as https://cdn.example.com),
getDirectoryPath() legitimately returns '' and the
route path becomes //styles/{image_style}/{scheme}.
Two things break:
-
The compiled route no longer matches the single-slash URL the site actually
generates, so derivative requests 404. -
Any
src/hrefstarting with//is
parsed by browsers as protocol-relative, so the host becomes
stylesand the request never reaches the site.
Steps to reproduce
-
Configure a Flysystem scheme with the
aws_s3(or any non-local)
driver whosepublic_url_basehas no path component, and use it
to servepublic://. -
Clear caches and request an image style derivative that has not been
generated yet. -
404.
drush router:info(or the route dump) shows the route path
as//styles/....
Proposed resolution
Collapse repeated leading slashes when building both route paths:
$public_route->setPath(preg_replace('#^/+#', '/', $public_route->getPath())); // ... preg_replace('#^/+#', '/', '/' . $directory_path . '/styles/{image_style}/{scheme}')
Remaining tasks
- Review.
-
Kernel/unit coverage for the route subscriber with
getDirectoryPath()returning''.
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| flysystem-04-route-double-slash.patch | 1.79 KB | luigisa |
Issue fork flysystem-3616488
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
lisa.rae commentedReviewed the pull request, and made some changes:
* Added a Path Processor for image styles to check for streaming from a CDN or bucket domain
* Added tests to support this Path Processor
* Added tests for "the route subscriber with getDirectoryPath() returning ‘'"
Please review and test, and provide feedback. If in agreement, after testing please mark RTBC, otherwise provide feedback or additional code changes and I’ll re-review afterwards.
Comment #5
luigisa commentedI’ve checked it and the patch works as expected.
Thanks Lisa
Comment #8
lisa.rae commentedComment #10
lisa.rae commented