-
-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an enableIntegrityHashes() method to the public API #522
Conversation
8584814
to
8ab3959
Compare
Slightly changed how the {
"integrity": {
"file1" : { "algorithm": "<alg>", "hash": "<hash>" },
"file2" : { "algorithm": "<alg>", "hash": "<hash>" }
}
} to: {
"integrity": {
"algorithm": "<alg>",
"hashes": {
"file1" : "<hash>",
"file2" : "<hash>"
}
}
} I thought the first version could be useful (for instance if we added a way to use a different algorithm for each file) but after thinking about it for a while I'm not sure that's really needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comments - this is going to be really, really nice. Thanks for the work so far!
lib/plugins/entry-files-manifest.js
Outdated
|
||
if (webpackConfig.integrityAlgorithm) { | ||
for (const file of assets[asset][fileType]) { | ||
if (!(file in integrity.hashes)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is file sometimes already in integrity.hashes
? Like, maybe a shared file that's used by multiple entries?
Also, we can flip this to `if (file in integrity.hashes) { continue; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is file sometimes already in
integrity.hashes
? Like, maybe a shared file that's used by multiple entries?
Yep, the runtime.js
file for instance :)
Also, we can flip this to `if (file in integrity.hashes) { continue; }
Will be part of the next changeset!
lib/plugins/entry-files-manifest.js
Outdated
}, null, 2); | ||
return JSON.stringify({ | ||
entrypoints: assets, | ||
integrity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include the integrity
key at all if the feature is disabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to be able to detect if Encore supported that feature to make the bundle display a relevant error if someone tried to use it with an old version of Encore.
But since I also agree about not making it optional in the bundle anymore (by detecting if integrity
is present in the manifest) that's not needed anymore :)
Ok so, changed the content again to something like: {
"integrity": {
"file1" : "<alg>-<hash1>",
"file2" : "<alg>-<hash2>"
}
} The reason being that it now allows to generate multiple hashes for each file (which is a feature supported by the {
"integrity": {
"file1" : "<alg1>-<hash1> <alg2>-<hash2>",
"file2" : "<alg1>-<hash3> <alg2>-<hash4>"
}
} Which also means that the Encore.enableIntegrityHashes(
true,
['sha256', 'sha384', 'sha512']
); |
Needs a rebase now :) |
370745b
to
fa915aa
Compare
This is going to be awesome! Thank you @Lyrkan! |
…(Lyrkan) This PR was merged into the master branch. Discussion ---------- Add an enableIntegrityHashes() method to the public API This PR adds an `Encore.enableIntegrityHashes()` method that allows to compute integrity hashes of all the files referenced in the `entrypoints.json` (closes #418). For instance: ```js // Enable it for all builds with the // default hash algorithm (sha384) Encore.enableIntegrityHashes(); // Enable it only in production with // a custom hash algorithm Encore.enableIntegrityHashes( Encore.isProduction(), 'sha512' ); ``` And here is the resulting `entrypoints.json` file: ```json { "entrypoints": { "main": { "css": [ "/main.3d1dcb7e.css" ], "js": [ "/main.7c6b7c81.js" ] } }, "integrity": { "/main.3d1dcb7e.css": "sha384-ce7d1nV3CFoSIfinwm53befb9CMHNAAlPEb61deOf3zBvpXK9lct44/U2ieSOKt4", "/main.7c6b7c81.js": "sha384-kHFhNTJlbSuDijSTimOHZGTxKzlLYCWc03AmmRSAE3b173SMlGiQG6uasAzl29+0" } } ``` Commits ------- fa915aa Add an enableIntegrityHashes() method to the public API
Epic! This will be really useful. Thanks @Lyrkan! |
This PR was squashed before being merged into the master branch (closes #42). Discussion ---------- Add support for integrity hashes This PR allows to automatically add `integrity` attributes on `<script>` and `<link>` tags based on the content of the `entrypoints.json` file (related to the following PR on Encore: symfony/webpack-encore#522). It requires the following configuration: ```js // webpack.config.js // Enable it for all builds with the // default hash algorithm (sha384) Encore.enableIntegrityHashes(); // Or enable it only in production // with a custom hash algorithm Encore.enableIntegrityHashes( Encore.isProduction(), 'sha384' ); // Or with multiple hash algorithms Encore.enableIntegrityHashes( Encore.isProduction(), ['sha384','sha512'] ); ``` Then, calling `yarn encore` then generates an entrypoints.json that contains hashes for all the files it references: ```js { "entrypoints": { // (...) }, "integrity": { "/build/runtime.fa8f03f5.js": "sha384-5WSgDNxkAY6j6/bzAcp3v//+PCXLgXCU3u5QgRXWiRfMnN4Ic/a/EF6HJnbRXik8", "/build/0.b70b772e.js": "sha384-FA3+8ecenjmV1Y751s0fKxGBNtyLBA8hDY4sqFoqvsCPOamLlA5ckhRBttBg1esp", // (...) } } ``` And these hashes are automatically added when calling `encore_entry_script_tags` and `encore_entry_link_tags`: ```html <html lang="en"> <head> <!-- ... --> <link rel="stylesheet" href="/build/css/app.2235bc2d.css" integrity="sha384-Jmd35HF93DFCXjisVeMi6U3lniH/mOdAF6wLtOMqhYMh2ZiBRUdtF7jXB55IAKfm"> <!-- ... --> </head> <body id="homepage"> <!-- ... --> <script src="/build/runtime.fa8f03f5.js" integrity="sha384-5WSgDNxkAY6j6/bzAcp3v//+PCXLgXCU3u5QgRXWiRfMnN4Ic/a/EF6HJnbRXik8"></script> <script src="/build/0.b70b772e.js" integrity="sha384-FA3+8ecenjmV1Y751s0fKxGBNtyLBA8hDY4sqFoqvsCPOamLlA5ckhRBttBg1esp"></script> <!-- ... --> </body> </html> ``` An example using Symfony Demo can be found here: Lyrkan/symfony-demo@91a06cd Commits ------- 84c41ed Add support for integrity hashes
@@ -750,6 +752,25 @@ class WebpackConfig { | |||
this.loaderConfigurationCallbacks[name] = callback; | |||
} | |||
|
|||
enableIntegrityHashes(enabled = true, algorithms = ['sha384']) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also forbid passing enabled = true
with an empty array as second argument ? That would actually disable it.
This PR adds an
Encore.enableIntegrityHashes()
method that allows to compute integrity hashes of all the files referenced in theentrypoints.json
(closes #418).For instance:
And here is the resulting
entrypoints.json
file: