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:

  1. The compiled route no longer matches the single-slash URL the site actually
    generates, so derivative requests 404.
  2. Any src/href starting with // is
    parsed by browsers as protocol-relative, so the host becomes
    styles and the request never reaches the site.

Steps to reproduce

  1. Configure a Flysystem scheme with the aws_s3 (or any non-local)
    driver whose public_url_base has no path component, and use it
    to serve public://.
  2. Clear caches and request an image style derivative that has not been
    generated yet.
  3. 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.

CommentFileSizeAuthor
flysystem-04-route-double-slash.patch1.79 KBluigisa

Issue fork flysystem-3616488

Command icon 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

luigisa created an issue. See original summary.

lisa.rae made their first commit to this issue’s fork.

lisa.rae’s picture

Issue summary: View changes

Reviewed 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.

luigisa’s picture

Status: Needs review » Reviewed & tested by the community

I’ve checked it and the patch works as expected.
Thanks Lisa

  • lisa.rae committed 3e8d16ad on 3.0.x
    Issue #3616488 by luigisa, lisa.rae:  Adding a Path Processor for image...

  • lisa.rae committed 662d1484 on 3.0.x authored by luigisa
    Issue #3616488: Collapse double-slash image style routes
    
    When the...
lisa.rae’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

lisa.rae’s picture

Status: Fixed » Closed (fixed)