fix(NODE-4211): Do not require crypto in browser builds

[rvanmil]

Description

What is changing?

This adds the @rollup/plugin-replace plugin, which is used to set a process.browser variable, to allow Rollup to create different builds for Node.js and browsers.

The check for process.browser was added to the detectRandomBytes function, which should now only require crypto in the js bundle, when process.browser was set to false while Rollup was running.

Is there new documentation needed for these changes?

I don't think so.

What is the motivation for this change?

The issue fixed by this change is described over here: https://jira.mongodb.org/browse/NODE-4211

WARNING in ./node_modules/bson/dist/bson.browser.esm.js 2196:26-55
Module not found: Error: Can't resolve 'crypto' in '/Users/rvanmil/Development/ctac-web-app-e2e-monitoring/node_modules/bson/dist'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

[durran]

Just a few small changes - thanks for the PR!

[durran]

I've also verified the output and bson.esm.js is the only artifact that correctly contains therequire('crypto') now. Nice work.

[rvanmil]

@durran out of curiosity, why was rollupProcess reverted to proces? Did I overlook something which did not work correctly?

[sagrawal31]

When can we expect a release with this fix?

[sagrawal31]

Thanks for releasing the new version. We are using this library in our code with webpack and we are getting this error/warning-

WARNING in ./node_modules/bson/dist/bson.esm.js 133:34-63
Module not found: Error: Can't resolve 'crypto' in '/Users/shashank/Projects/cooee/web-sdk/node_modules/bson/dist'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
	- install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "crypto": false }
 @ ./src/session/session-manager.ts 5:0-32 73:36-44
 @ ./src/services/safe-http-service.ts 1:0-60 16:30-56
 @ ./src/renderer/in-app-renderer.ts 6:0-64 41:12-39
 @ ./src/index-preview.ts 1:0-59 3:15-28

1 warning has detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

webpack 5.47.1 compiled with 1 warning in 2409 ms

Any idea?

[sagrawal31]

Got the solution. As the error already says, webpack 5 no longer adds the polyfills by default as described here.

So the solution was to add the followings in webpack.config.js at resolve -> fallback-

fallback: {
    crypto: require.resolve('crypto-browserify'),
    stream: require.resolve('stream-browserify'),
    util: require.resolve('util')
}

and install stream-browserify, crypto-browserify & util. Please note- This increases the build size on browsers by ~400 KBs.

The other option is to disable the polyfills completely-

fallback: {
    crypto: false
}

[rvanmil]

@sagrawal31 I think there might be something wrong with your dependencies. This PR fixes the issue described by you and the fix was released with bson version 4.6.5
Maybe you have a nested dependency which still refers to an older version of bson?

[sagrawal31]

Thanks for spotting this @rvanmil. I also had the same concern but I freshly installed the npm packages and looked for bson in package-lock.json but couldn't find any dependency which pulled an old version of bson. https://github.com/letscooee/cooee-web-sdk/blob/main/package-lock.json#L955

[rvanmil]

Your webpack configuration only has es5 as target, which means it will use the Node.js build which does refer to crypto. I think in your case you should change the target to target: ['web', 'es5'].

[sagrawal31]

Thanks, @rvanmil. Making that change no longer shows that warning/error. So far, I was under the impression that target: es5 will still compile it for web but you clarified it. Thanks again!

Do you also mind clarifying two things for me, please-

1. If we do not include the crypto polyfills, do the chances of globally unique object-ids decrease (as we see the warning in the console BSON: No cryptographic implementation for random bytes present, falling back to a less secure implementation.)?
   We have been using the another library so far and it was generating duplicate ObjectId (we spotted when our code was running in a website which was being crawled by Google bot)
2. If the answer to the above question is tend to yes, what if we have to include those polyfills to reduce the chances of duplicity? Should we just get rid of web from target and add all those fallbacks?

[rvanmil]

You're welcome! The other questions I do not know the answer to, perhaps someone from the MongoDB team can answer that? @durran @dariakp

[nbbeeken]

Hi @sagrawal31, the cryptographically secure pseudorandom number generator is attempted to be found in Node.js from require('crypto') and in the browser from window.crypto. If not found it will fall back to Math.random() and emit that warning. However, it appears support is widely available so it's unlikely that you'll run into this warning in practice. I would say the chances of a globally unique object do not decrease any more than a typical usage of random object id generation.

I would consult this first section of the specification of ObjectIds to see if there is a concern for your use case. To quickly summarize, ObjectIds use a combination of second granularity timestamp, unique process id (randomly generated) and a in-memory counter (that starts at a random offset). Hopefully that helps!

[sagrawal31]

Hi, @nbbeeken. Thanks for the detailed response. I have been processing your information and did some tests internally. However, unfortunately, we can still see a lot of duplicates being generated on the browsers when our code is being executed on a website crawled by Google crawlers. We consider this is happening because Google must be using some sort of device blueprints to crawl the applications.

Although we are now stopping our code execution on crawlers and we'll keep an eye on and update here on any further findings.