Stage file proxy attempts to fetch aggregated JS/CSS.

Stage file proxy is using StageFileProxySubscriber and it doesn't check config related to excluded extensions.
Here is a MR that fixes it.

Testing steps:

• Apply MR
• Set excluded extensions in the settings page (/admin/config/system/stage_file_proxy) - "js, css"
• Clear cache

The fix looks good on paper and passes automated tests. What's the best way to test this?

Just started a 2.1.x branch for php8 only need to make sure it works there first.

@smustgrave Do you need this merge request to be rebased onto 2.1? The bug is being experienced in production on the 2.0 branch. So maybe we need two patches. Currently both 2.0 and 2.1 support D10, which is where the bug appears.

Would need one for both.

But have you checked to see if this is a core issue? https://www.drupal.org/project/issues/search/drupal?text=aggregation&ass... →

Maybe instead of MR the module can exclude path which is public://styles/?

updated IS

It isn't a core issue. I applied patch that resolves the core issue and the issue was still present.
Also, without this MR you are not able to excluded extensions. There is a config available for it, but not the code that will support it.

I will rebase in on 2.1.x branch.

Target branch changed from 2.0.x to 2.1.x. Please review.

This happens on both Drupal 9 and 10. Not sure about the proposed fix as this would still proxy .css.gz and .js.gz assets.

IMO SFP should not proxy anything for assets (D10) or anything in public://css and public://js (D9).

Note: This issue seems to be mitigated by enabling the new file_assets_path setting → and moving JS/CSS aggregation files out of public://.

Is the MR necessary? StageFileProxySubscriber could exclude paths already when initializing the AlterExcludedPathsEvent.

It is necessary. It fixes another issue 🐛 Stage file proxy attempts to fetch aggregated JS/CSS. Fixed

Could you please move inability to exclude extensions to a separate issue? Marking this needs work to exclude local CSS/JS assets. We've scheduled time tomorrow to provide a patch.

Attaching changes from MR as a patch, so it can be safely applied via composer.

I agree with Heine, this this doesn't solve the problem. Even if it could solve the problems with aggregation, it has problem.

1. it requires you _want_ to exclude all css and js files. I don't have a use case but if have css files you do want to be able to stage, this would block that use case
2. it requires additional configuration to get working
3. as Heine noted, it doesn't address all use cases or possibly more complicated aggregation scenarios in the future

Here's a patch that seems to addresses excluding aggregation specifically. The assumptions in StageFileProxySubscriber around paths are a little weird and the logic for the asset path pretty complicated but this addresses it as well as imagecache(Styles) so I think its probably fine for the current state of the module.

Actually got bit by this issue myself. Going to try out #21 but would like a 2nd plus +1 before merging if possible

I had the same issue, tested my project with this patch and it is working fine now. Once someone reviews it, I think we can move it to the next stage.

Committed to 2.1.x. Should this be backported to 2.0.x without the usage of str_starts_with?

I don't think so. The change in core only landed in 10.1 I believe.

I haven't confirmed it but from #15 it affects 9 but we're only really noticing it because of the 10.1 changes? Maybe Heine can provide more context.

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

Hi there,

I've provided this new patch, that creates a new type of event "AlterExcludedExactPathsEvent" and runs it separately after the old event "AlterExcludedPathsEvent" in order to allow backwards compatibility. This new event disallows /js and /css paths as a root argument for the public file directory (
/js/ or
/css/).

Please let me know what you make of it.

Cheers!

smustgrave → closed merge request !46