Today I've spent a lot of time on debugging and investigating the following problem:

I'm using remote_stream_wrapper in a client project to attach images during a migrate import. This works fine and the images are showing up, as they should - at least if you render them without image style.

As soon as I tried to render them via image style, I got access denied errors. On further investigation, I found out that there's actually a redirect happening, resulting in the second request not having the import "file" query attribute set.

Finally I found out, that the presence of the redirect module caused the problem.

The solution is finally very trivial: the image style routes must have set '_disable_route_normalizer' to TRUE in order to prevent that redirect's route normalizer is causing a redirect. Redirect itself is altering the core 'image.style_public' and 'system.files' routes in its \Drupal\redirect\Routing\RouteSubscriber class.

The remaining question is, which of the two modules should do this integration work. Even if in a default setup without redirect module, the presence of '_disable_route_normalizer' is unimportant (but also does no harm), I was tending to say, remote_stream_wrapper should take care of this.

I've contacted the maintainers of both modules, @davereid and @berdir on Twitter. berdir mentionend a pending core patch, that the route normalizer is based upon and so proposed the same: "The route normalizer is based on a core patch that will eventually get in, so I think other contrib modules should add it themselves" (https://twitter.com/berdir/status/861930414117707780)

So I'll propose a patch here...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agoradesign created an issue. See original summary.

agoradesign’s picture

here it is...

Status: Needs review » Needs work

The last submitted patch, 2: disable_route_normalizer.patch, failed testing.

agoradesign’s picture

I really doubt that this patch has something to do with the failing test...

michaellander’s picture

This patch worked for me. Thank you!

agoradesign’s picture

You're welcome.

Would be great, if someone could look at the module's tests. I'm pretty sure, that they will fail with or without that patch here

patrickfweston’s picture

This patch works for me as well. However, I'm on the 8.x-1.0 release and the patch above fails. I've included a patch for 8.x-1.0 here (just a change in the line number).

fago’s picture

Status: Needs work » Needs review

I verify the patch works as it should. I doubt as well that it makes tests fail.

akalam’s picture

The patch #7 work for us. Thanks as it let us use remote_stream_wrapper in conjunction with redirect module.

tunic’s picture

The patch is working ok in a local box. All tests run ok in a local box with the patch applied. It seems test runner hit some strange condition that makes tests fail.

Let's try with an empty patch.

  • Dave Reid committed 60ec85c on 8.x-1.x authored by tunic
    Issue #2877055 by agoradesign, patrickfweston: Disable route normalizer...
Dave Reid’s picture

Status: Needs review » Fixed

I have committed #2 to 8.x-1.x. Thank you!

agoradesign’s picture

Hey Dave, you owe me one authorship in a subsequent commit ;)) :D

Glad to see this in now :)

tunic’s picture

Ups, sorry agoradesign, I didn't want to hijack your patch.

agoradesign’s picture

:D no problem :)

Dave Reid’s picture

Dave Reid’s picture

Gah, I'm sorry agoradesign. Silly Drupal.org defaults to the last person with the patch.

agoradesign’s picture

Don't worry :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.