Drupal 10.3 ImageStyleDownloadController::deliver signature change

This will cause a breakage beginning when Drupal 10.3 is released and people start upgrading to it.

I have a merge request with the changes to update the signature and the Drupal core version to ^10.3

That fixed the error on install but images were not rendering for me. I added the required_derivative_scheme route config based on https://www.drupal.org/node/3346038 → and its working now.

It looks like there are a couple test failures compared with the 2.x branch that need to be fixed: https://git.drupalcode.org/issue/remote_stream_wrapper-3437974/-/jobs/18...

I updated the test as I think this is an expected change since its using a core-provided image style (thumbnail): https://www.drupal.org/node/3421405 →

RTBC, any chance we can get this reviewed by a maintainer since this could cause issues once 10.3 is released

There appears to be an incorrect assumption made that the derivative scheme will always be public://.

The new checks in 10.3 enforces that the $required_derivative_scheme to match the scheme of buildUri to assert that the route is responsible for the scheme and that a route for a different scheme isn’t being abused.

https://git.drupalcode.org/issue/remote_stream_wrapper-3437974/-/blob/34...

This is related to #3302994: Bypass of RSW file_managed access protection and arbitrary HTTP HEAD reqeusts → which was part of SA-CORE-2022-012.

What likely needs to happen is:

• RSW obtains the default configured scheme
• Locates the route for that scheme
• Adds the RSW mappings atop those routes.

Alternatively some of these protections might actually be better as part of a Route Subscriber.

Hi @cmlara,

The assumption that the scheme will always be public is based on the routes defined in the Image module. See https://api.drupal.org/api/drupal/core%21modules%21image%21src%21Routing... where the directory path is set by using the method `->getViaScheme('public')`.

It looks to me like this MR fixes the bug introduced by the update to Drupal 10.3. If we need to make other improvements isn't that better to do in the issue you linked?

If we need to make other improvements isn't that better to do in the issue you linked?

That is a possibility as well.

I will note that the Entity itself does not assume that public is used https://git.drupalcode.org/project/remote_stream_wrapper/-/blob/2.x/src/...

I will leave it up to the modules maintainer how they want to split the fix.

It is just important to know the fix as is (even with the update to use the config) does not truly resolve the reasons why the core signature change occurred.

Adding a patch targeting 2.0.0. Not to be used but for anyone required to use only stable versions.

Aaron Gil Martinez → made their first commit to this issue’s fork.

Aaron Gil Martinez → closed merge request !14

#18 fixes the issue when upgrading to 10.3.0. Thx!

I reviewed and approved, though I did, like others, also feel that fixing the default scheme is a bit out of scope here.

Noting also that it's been a year since any maintainer closed or merged an issue on this project, so while this is good work here, we might be out of luck getting this into the project, so thank you to everyone who put up patches. I did write a long time ago asking about co-maintaining, but understandably did not hear back. Perhaps there is someone on this thread who would have the stature in the community to be able to make the same offer so we can address some of the issues that are beginning to stretch this module now with 10.3 out.

Count me in @apotek. I don’t know how much help I will be able to offer but I will definitely try.

https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... → Is the process for taking over an “abandoned” module on Drupal.org.

@vensires and @jrglasgow are users on this thread that have the necessary permission to proceed through the process if they desire to do so.

I just opened 💬 Offering to co-maintain Remote Stream Wrapper Active in case anyone is interested...
(@apotek, I mentioned you too)

Thank you @cmlara for your hint!

Hey sorry all, life has been super hectic the last couple of months.

Merged and releasing!

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