Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for integrity hashes #42

Closed
wants to merge 2 commits into from
Closed

Add support for integrity hashes #42

wants to merge 2 commits into from

Conversation

Lyrkan
Copy link
Contributor

@Lyrkan Lyrkan commented Feb 14, 2019 •
edited
Loading

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:

// 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:

{
  "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 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

@weaverryan
Copy link
Member

@Lyrkan What do you think about not making this optional? What I mean is, we always look for the integrity key. And if it is present, we add the integrity attribute. If not, we skip it. I think we can avoid the added option, because you've already "opted into it" by activating it in Encore.

@Lyrkan Lyrkan force-pushed the use-integrity-hashes branch from e023d90 to 7faf283 Compare March 1, 2019 21:30
@Lyrkan
Copy link
Contributor Author

Lyrkan commented Mar 1, 2019

@weaverryan I removed the toggle and no integrity key is now the same thing than integrity: {}.

I also made some changes in order to support the latest version of symfony/webpack-encore#522 (that allows to generate multiple hashes per file) and updated the demo link of my first post :)

weaverryan
weaverryan reviewed Mar 29, 2019
View reviewed changes
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor stuff! Then it'll be ready for merge!

src/Asset/TagRenderer.php Outdated Show resolved Hide resolved
src/Asset/IntegrityDataProviderInterface.php Show resolved Hide resolved
src/Asset/TagRenderer.php Outdated Show resolved Hide resolved
@weaverryan
Copy link
Member

Awesome! Thank you @Lyrkan!

@endelwar
Copy link

I'm getting a little warning in PHPStorm about boolean!= bool
Schermata 2019-03-29 alle 15 53 07

@Lyrkan
Copy link
Contributor Author

Lyrkan commented Mar 29, 2019

@endelwar My bad for that one, wrong JSDoc type!

I just pushed a fix in Encore, it'll be part of the next release (but you can safely ignore that warning for now).

@PhilETaylor PhilETaylor mentioned this pull request Apr 1, 2019
Closed
@udf2457 udf2457 mentioned this pull request Jul 3, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weaverryan weaverryan weaverryan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Lyrkan @weaverryan @endelwar