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.

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 support for md4 in Node >=18. #17628

Merged
merged 2 commits into from
Sep 6, 2023

Conversation

iclanton
Copy link

@iclanton iclanton commented Aug 24, 2023

Summary

🤖 Generated by Copilot at 6f6ae98

This pull request adds WebAssembly-based md4 hashing for Node.js 18 or higher, and implements a BatchedHash class that improves the performance of hashing large data. It also refactors the createHash function and removes unused code from the createHash.js module. The changes involve adding new modules lib/util/hash/BatchedHash.js, lib/util/hash/md4.js, and lib/util/hash/wasm-hash.js, and modifying lib/util/createHash.js.

Details

🤖 Generated by Copilot at 6f6ae98

  • Remove Hash class and HashConstructor type exports from createHash.js module (link)
  • Add variables to lazily require crypto, md4, and BatchedHash modules in createHash.js module (link)
  • Modify createHash function to handle md4 and native-md4 algorithms differently depending on Node.js version and use BatchedHash class for md4 WebAssembly hash in createHash.js module (link)
  • Add BatchedHash class that extends Hash class and batches data chunks for underlying hash implementation in BatchedHash.js module (link)
  • Add md4 function that creates WasmHash instance using md4 WebAssembly module and md4 instances pool in md4.js module (link)
  • Add WasmHash class that implements hash interface using WebAssembly module, memory buffer, chunk size, and digest size in wasm-hash.js module (link)
  • Add create function that creates WasmHash instance using WebAssembly module, instances pool, chunk size, and digest size in wasm-hash.js module (link)
  • Export MAX_SHORT_STRING constant from wasm-hash.js module and use it in BatchedHash class (link, link)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 24, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

Why not upgrade the webpack version to v5?

@iclanton
Copy link
Author

iclanton commented Sep 6, 2023

@snitin315 - There are a number of breaking changes between Webpack 4 and 5, making upgrading large and complex projects time-consuming.

Webpack 4 currently has many more weekly downloads on npmjs.org than Webpack 5 does, but Webpack 4 only works with Node 16 and earlier because of the issue being addressed in this PR. Node 16 is EOL on 2023-09-11 (next Monday).

@snitin315
Copy link
Member

snitin315 commented Sep 6, 2023

@iclanton Shouldn't webpack upgrade be considered as the migration effort for your app to Node v18? You can anyway use NODE_OPTIONS=--openssl-legacy-provider when using webpack v4 with Node.js v17+, and put a TODO to upgrade webpack v5 later. CMIIW, here we are using this function just to create a hash in filenames, so there's no such security threat.

@TheLarkInn
Copy link
Member

TheLarkInn commented Sep 6, 2023

@alexander-akait @snitin315 and myself discussed this further. Because Node 16 EOL is next week, we do not want our ~12 million webpack 4 consumers to be stuck with what will be flagged as a security vulnerability not updating Node. It would be ideal not to block them here.

Copy link
Member

@TheLarkInn TheLarkInn left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution.

@TheLarkInn TheLarkInn merged commit 7395af8 into webpack:webpack-4 Sep 6, 2023
16 checks passed
@iclanton iclanton deleted the webpack4-md4-hash branch September 6, 2023 17:22
chadlwilson added a commit to gocd/gocd that referenced this pull request Sep 17, 2023
This was only needed for webpack 4 due to use of md4 hashes, support for which has been backported in webpack 4.47.0 removing need for use of the old/insecure Node crypto provider. See webpack/webpack#17628
@chadlwilson
Copy link

chadlwilson commented Sep 18, 2023

Thanks so much for this @iclanton . I see you've been doing some work around the ecosystem to help with this. Is terser-webpack-plugin on your radar? I opened webpack-contrib/terser-webpack-plugin#578 to see if the maintainers are interested in addressing a similar issue there. (edit: but this may be more to do with the transitive dependency to terser ^1.4.3 within webpack 4 itself.)

"terser-webpack-plugin": "^1.4.3",

@Shubhanshu88
Copy link

@chadlwilson I guess this PR by iclanton : #17659 is for the terser-webpack-plugin bump

@chadlwilson
Copy link

I totally missed that, thanks @Shubhanshu88 - I feel stupid now!

plietar added a commit to mrc-ide/mint that referenced this pull request Dec 12, 2023
This is a mostly straightforward upgrade. Just a few notes:
- The lockfile format has changed so gets rewritten entirely.
- webpack is bumped to the next dot release. This adds a workaround
  caused by Node switching to OpenSSL 3, which drops md4 as a hash
  function (webpack/webpack#17628).
- node-sass did not build on the new node so had to be updated.
- I've updated playwright to take advantage of its added support for
  installing dependencies on Debian, removing the hardcoded list of
  packages we had to install.

There's a whole bunch of updates we could do to our NodeJS dependencies,
but I've left that for a later point since they aren't urgent or
necessarily trivial.
plietar added a commit to mrc-ide/mint that referenced this pull request Dec 14, 2023
This is a mostly straightforward upgrade. Just a few notes:
- The lockfile format has changed so gets rewritten entirely.
- webpack is bumped to the next dot release. This adds a workaround
  caused by Node switching to OpenSSL 3, which drops md4 as a hash
  function (webpack/webpack#17628).
- node-sass did not build on the new node so had to be updated.
- I've updated playwright to take advantage of its added support for
  installing dependencies on Debian, removing the hardcoded list of
  packages we had to install.

There's a whole bunch of updates we could do to our NodeJS dependencies,
but I've left that for a later point since they aren't urgent or
necessarily trivial.
rap1ds added a commit to sharetribe/flex-sdk-js that referenced this pull request May 13, 2024
yarn run build failed to run, due to following error:

    ERROR in sharetribe-flex-sdk-web.js from Terser
    Error: error:0308010C:digital envelope routines::unsupported

Read more here: webpack/webpack#14532

To fix the issue:

- Upgrade webpack to 4.47.0 (see: webpack/webpack#17628)
- Add `resolutions` block for terser-webpack-plugin until this PR is
  merged (webpack/webpack#17659)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants