Overview
When the compiled JavaScript and CSS code for code components get written to a file, the filename is calculated using \Drupal\Component\Utility\Crypt::hmacBase64 with Settings::getHashSalt() as the key. Whenever we access the files, the filename is re-calculated on-the-fly. Due to relying on the value of Settings::getHashSalt(), the filenames are not stable.
- Rotating the hash salt is a security best practice;
- Cloning database and files between different environments can happen, and different environments will have different hash salt values.
These scenarios cause the Astro Islands to point at non-existing files.
Proposed resolution
Change the way the filename is calculated or store the calculated filename on the — use the config entity's JavaScriptComponent entity instead of calculating it on-demand.uuid, which is also used for config management/syncing/importing: duplicate UUIDs are refused, and will result in an error like An entity with this UUID already exists but the machine name does not match.
That achieves the same end result, without the per-environment dependency of the hash salt.
More context:
- The code for calculating the filenames was added in commit
9d295in #3498889: ComponentSource plugin for code components. - @longwave suggested if we could "use a more user friendly filename and rely on the Drupal cache busting query string to handle invalidation" while working on #3499933: Storage for CSS shared across in-browser code components (and other use cases in the future), and Wim agreed as a future improvement. Maybe it's time to make that happen?
Issue fork experience_builder-3528318
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #6
balintbrewsCrediting people who participated in the conversation that resulted in this issue.
Comment #7
balintbrewsComment #8
wim leersComment #9
wim leersComment #12
isholgueras commentedI've pushed a proof of concept of removing the hash by the original id.
I've adapted the tests and expected more things to break but they don't.
nachojsand added to components. XB createddefault/files/astro-island/nachojs.jswell.I've run all the tests that I've modified and all pass (except for those hash/filename that I expect), but I'll let the CI to run the whole battery.
Comment #13
mglamanOne thought I had was keeping a hash, but making it a hardcoded salt. So v0, v1, v2, etc. Whenever XB has a breaking change, this could be bumped and an upgrade hook written. No idea _why_ a breaking change could happen, but it is atleast an option to load different files?
Comment #14
isholgueras commentedComment #15
wim leersThis looks surprisingly simple! 🤯
Just one Q. 😊
Comment #16
wim leers🏓
Comment #17
isholgueras commentedComment #18
wim leersThis looks simpler & more elegant than I dared hope! 😄
Comment #20
wim leersThanks!