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...
Comment | File | Size | Author |
---|---|---|---|
#10 | dummy_patch.patch | 0 bytes | tunic |
#7 | disable_route_normalizer-8-1.patch | 545 bytes | patrickfweston |
#2 | disable_route_normalizer.patch | 572 bytes | agoradesign |
Comments
Comment #2
agoradesign CreditAttribution: agoradesign commentedhere it is...
Comment #4
agoradesign CreditAttribution: agoradesign commentedI really doubt that this patch has something to do with the failing test...
Comment #5
michaellander CreditAttribution: michaellander at Elevated Third commentedThis patch worked for me. Thank you!
Comment #6
agoradesign CreditAttribution: agoradesign commentedYou'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
Comment #7
patrickfwestonThis 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).
Comment #8
fagoI verify the patch works as it should. I doubt as well that it makes tests fail.
Comment #9
akalam CreditAttribution: akalam at Metadrop commentedThe patch #7 work for us. Thanks as it let us use remote_stream_wrapper in conjunction with redirect module.
Comment #10
tunicThe 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.
Comment #12
Dave ReidI have committed #2 to 8.x-1.x. Thank you!
Comment #13
agoradesign CreditAttribution: agoradesign commentedHey Dave, you owe me one authorship in a subsequent commit ;)) :D
Glad to see this in now :)
Comment #14
tunicUps, sorry agoradesign, I didn't want to hijack your patch.
Comment #15
agoradesign CreditAttribution: agoradesign commented:D no problem :)
Comment #16
Dave ReidComment #17
Dave ReidGah, I'm sorry agoradesign. Silly Drupal.org defaults to the last person with the patch.
Comment #18
agoradesign CreditAttribution: agoradesign commentedDon't worry :)