Provide BC for ImageStyleDownloadController

(Bringing Slack notes into the issue)

Lack of BC was intentional.

As noted in 🐛 ImageStyleDownloadController routes do not limit schemes served Fixed the change was for security reasons related to SA-CORE-2022-012 → .

The class and method in question are 100% internal. Per Drupal API policy no one should be extending, calling, or otherwise using the method unless they are willing to accept breaking changes in point releases.

A lot of this happens in Drupal Core with module maintainers not realizing that their Core Compatibility should actually be "<=X.Y.Z" or at most "~X.Y.0" rather than the much more common "^X"

Setting back to NW to prevent an accidental fast-track of this into core.

I'm going to take a moment to plug ✨ Split ImageStyle into the config entity and a separate event-based image processing service Needs work as a likely long term solution for many modules that are currently providing their own ImageStyleDownloadController class.

I understand the intentional lack of BC and that this is not actually part of the BC promise. The affected contrib modules will need to effectively provide the BC support themselves to support both 10.2 and 10.3, and can potentially start a new branch that is only 10.3+ that drops BC. This also highlights the benefit of OPT_IN_TEST_NEXT_MINOR as all of these modules would have failed immediately.

I think this issue in itself is probably wontfix. As noted, this is an intended hard break.

However, a lot of work could be done to better explain in the release note of 🐛 ImageStyleDownloadController routes do not limit schemes served Fixed . I vote for reopening that for a better release note instead.

IMHO this should have never been committed. Policy or not, public method signatures shouldn't be changed like they were here in a minor revision without regard to BC. Several modules are extending this controller, causing some pretty big headaches going to the next minor revision.

A lot of this happens in Drupal Core with module maintainers not realizing that their Core Compatibility should actually be "<=X.Y.Z" or at most "~X.Y.0" rather than the much more common "^X"

Disagree. This pattern becomes an unmaintainable mess across the contrib world. Maintainers expect that core methods stay consistent from minor to minor. The "~X.Y.0" pattern has been a pain point for our update processes and unique to the modules you maintain.

Marking RTBC, because the MR should have been added to the original commit and should be fast tracked into 10.3.1 to mitigate the damage already done by people trying to upgrade to 10.3 and facing broken sites due to modules not being updated fast enough... partially due to this change.

Still -1 on the grounds it would be adding a known security fault into the code.

It is now also possible some code has already gone as far as to remove any protections they have had added for older releases (<10.3) now that this safety measure is in place and we could be exposing those site owners to renewed risk.

Policy or not, public method signatures shouldn't be changed like they were here in a minor revision without regard to BC.

Disagree, public just means the method can be called from another class, it has no implication on the guarantee that method signature will not change. The only “guarantee” of a method not changing is if it’s declared formal @api.

The "~X.Y.0" pattern has been a pain point for our update processes not something maintainers should be doing.

What we really shouldn’t be doing is modifying or utilizing non-api code (however this is near impossible for many modules since typehinting an interface is API but implementing an interface is not) which would allow us as maintainers (with reasonable assurance) to just declare “^X”.

Back to NR for #8

Can the issue summary be updated to include why the backwards compatibility is more important then the security protection. Maybe which contrib modules are affected that haven't been fixed.