Exclude 'remixicon.symbol.svg' from asset pipeline #5878
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Referencing SVG icons from an external host using `<use xlink:href="">` is not supported, for CORS (cross-origin request scripting) security reasons. This doesn't come up often in development but can rear its head in a production deployment. To avoid this, we'll always provide an empty host. This results in the generated paths being protocol-prefixed relative URLs (i.e. "http:/assets/solidus_admin/remixicon.symbol.svg") but this seems to work OK.
|
Interesting. In Alchemy we have the same issue and we tried to fix it with a preload tag
I have no assets host that I can test this with, though I trust your approach. But, can we try to preload the asset and verify if it works as well or not? |
jarednorman
approved these changes
Oct 21, 2024
There was a problem hiding this comment.
I tend to think this approach is fine (rather than preloads) because:
- It's in the admin and serving one asset in the admin from the app server isn't a performance concern.
- Keeping preloads in sync with usage seems like a maintenance burden. It would be pretty easy for people to change something and miss that.
MadelineCollier
approved these changes
Oct 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #5657 - when a Solidus store has
config.action_controller.asset_hostconfigured to point to a different hostname than the Rails application (i.e. an assets CDN, in production), the admin menu icons do not render as expected.This is because for security reasons, referencing SVG icons from an external file (using
<use xlink:href="">) is not possible when the asset is served from a different domain to the one requesting it.Providing an empty
hostoption allows us to skip Rails' built-in asset-path helper methods that are falling back to the globalasset_hostconfig, and correct the issue.Note: this change does result in the generated asset path being a protocol-prefixed relative URL (i.e.
"http:/assets/solidus_admin/remixicon.symbol-[SHA].svg"), but this seems to work OK from my testing.