waysact / webpack-subresource-integrity

Webpack plugin for enabling Subresource Integrity.
MIT License
357 stars 46 forks source link

Webpack 5.47.0 fails with v5.0.0-alpha.5 #162

Closed jens-duttke closed 3 years ago

jens-duttke commented 3 years ago

After updating to Webpack 5.47.0, the browser throws a lot of integrity violations:

Failed to find a valid digest in the 'integrity' attribute for resource '<URL>' with computed SHA-256 integrity 'NjqZnazu3EZwz5SxDoeaTBQCHB2YupOpWOsv+BvMxIA='. The resource has been blocked.
Failed to find a valid digest in the 'integrity' attribute for resource '<URL>' with computed SHA-256 integrity 'XJd+zgxjvQP6sPOr0yrqgFiMY0IVs8D08WN8wWQZdVU='. The resource has been blocked.
Failed to find a valid digest in the 'integrity' attribute for resource '<URL>' with computed SHA-256 integrity 'GQRbuQl4o18G0DTYVd2Had4NAXld25014og8rkIgqyw='. The resource has been blocked.
Failed to find a valid digest in the 'integrity' attribute for resource '<URL>' with computed SHA-256 integrity 'cTI1Gy+xILiHMtr3wlqDUEcSPwWNAReoR9u5uF4I7K4='. The resource has been blocked.

With Webpack 5.46.0 everything worked fine.

Is that a Webpack issue, or does this plugin use some internal APIs which have been changed in Webpack 5.47.0?

jscheid commented 3 years ago

I need to look into it more but at first glance it looks like it might be a problem with long-term caching. It seems that Webpack 5.47 generates different content for the same contenthash. Different content means that the SRI value is different, but same contenthash means that the browser will reuse the same outdated content from the cache. Can you confirm that this is the case by testing with an empty cache?

jscheid commented 3 years ago

I have a hunch it might be related to https://github.com/webpack/webpack/commit/0bd1e789d48f9b3e3ca38d6c48db7c7be5380bcd, where possibly the newer Webpack version omits use strict without it affecting the hash, but I haven't been able to reproduce it so far. It would be useful to see your webpack.config, in particular your optimization settings.

jens-duttke commented 3 years ago

@jscheid My optimization settings looks like this:

optimization: {
    minimizer: [
        new TerserPlugin({
            terserOptions: {
                output: {
                    // @todo max_line_len: 10 disabled for now because of a bug in Safari 11.0.3
                }
            }
        })
    ],
    moduleIds: 'deterministic',
    splitChunks: {
        chunks: 'all',
        cacheGroups: {
            i18n: {
                test: /[\\/]json[\\/]locales[\\/].+?\.json$/u,
                enforce: true,
                priority: 3,
                name (module) {
                    return `locales/${path.basename(module.resource, path.extname(module.resource))}`;
                }
            },
            ...generateCacheGroupsPaths({
                'bootstrap': '/src/scss/bootstrap.scss',
                'error-reporting': '/src/ts/errorReporting.ts',
                'startup-error': '/src/scss/startup-error.scss',
                'context': '/src/ts/context/',
                'dataTypes': '/src/ts/dataTypes/',
                'editor': '/src/ts/editor/',
                'ui': '/src/ts/ui/',
                'utils': '/src/ts/utils/'
            }, { priority: 2 }),
            ...generateCacheGroupsVendors({
                'vendors/core-js': ['core-js'],
                'vendors/hash-wasm': ['hash-wasm'],
                'vendors/react': ['react', 'react-dom']
            }, { priority: 1, name: 'vendors/commons' })
        }
    }
},

Can you confirm that this is the case by testing with an empty cache?

If I run the same page in Incognito mode the page is loading correctly. So I assume it's really the same contenthash for different content.

jscheid commented 3 years ago

I've managed to reproduce this with a test case, which generates files with the same name and different contents between Webpack 5.46 and 5.47. The diff in question looks like this:

diff -ru dist-162-webpack-5-46/9396385c6109d27a1121.chunk.js dist-162-webpack-5-47/9396385c6109d27a1121.chunk.js
--- dist-162-webpack-5-46/9396385c6109d27a1121.chunk.js 2021-07-29 22:29:05.000000000 +1200
+++ dist-162-webpack-5-47/9396385c6109d27a1121.chunk.js 2021-07-29 22:31:45.000000000 +1200
@@ -1 +1 @@
-(self.webpackChunkissue_162=self.webpackChunkissue_162||[]).push([[364],{364:s=>{"use strict";s.exports={}}}]);
\ No newline at end of file
+"use strict";(self.webpackChunkissue_162=self.webpackChunkissue_162||[]).push([[364],{364:s=>{s.exports={}}}]);
\ No newline at end of file

What this (and also #152) shows is that there is a class of "benign" changes in file contents (with the file name hash staying the same) that don't matter without SRI but are fatal when SRI is used. A white space-only change is probably the best example, or quoting style as in #152, and this particular case might qualify as well.

The ideal solution, from the perspective of this plugin, would be for Webpack to adopt some kind of test harness that guarantees (with reasonable confidence) that even benign changes will cause the hash to update.

Without such a guarantee, we need to add a section to the README explaining the pitfalls, and we should probably also trigger a warning (or even an error) when a "dangerous" filename construct is used.

@sokra do you have any thoughts on this? In this particular case I've only been able to reproduce with chunkhash, but in #152 it was contenthash. Do you agree that we need to treat any kind of hash in output filenames as dangerous when used with SRI?

jscheid commented 3 years ago

@jens-duttke could you also share the output section of your config?

jens-duttke commented 3 years ago

@jens-duttke could you also share the output section of your config?

Sure:

output: {
    path: path.join(__dirname, 'build'),
    clean: true,
    publicPath: '/',
    filename: devMode ? 'js/[name].min.js' : 'js/[name].[contenthash].min.js',
    chunkFilename: devMode ? 'js/[name].min.js' : 'js/[name].[chunkhash].min.js',
    hashFunction: MD4Base62Hasher,
    hashDigest: 'base62',
    hashDigestLength: 8,
    crossOriginLoading: 'use-credentials',
    globalObject: 'this'
},
jscheid commented 3 years ago

Thanks for the report @jens-duttke. After discussing this with @sokra I've changed the plugin to output a warning when the output filenames are configured in a way that can cause this situation. This change is released in 5.0.0-rc.1.