[Fix] : Buffer explicitly require typo

[pepoviola]

Hi All, first I just want to thanks for the amazing job!!

I found a bug related to this commit that breaks the use of crypto in the browser.

Related to this code

var algorithms = require('./algorithms.json')
Object.keys(algorithms).forEach(function (key) {
  algorithms[key].id = Buffer.from(algorithms[key].id, 'hex')
  algorithms[key.toLowerCase()] = algorithms[key]
})

As this Note ( https://www.npmjs.com/package/buffer#usage )

To require this module explicitly, use require('buffer/') which tells the node.js module lookup algorithm (also used by browserify) to use the npm module named buffer instead of the node.js core module named buffer!

I think the line should be

var Buffer = require('buffer/').Buffer  // note: the trailing slash is important!

( I try this fix and worked as expected )

Again, thanks for this amazing job!

[calvinmetcalf]

@rvagg your thoughts ?

[calvinmetcalf]

@pepoviola where are you running this ?

[pepoviola]

Hi @calvinmetcalf, thanks for your answer. We have a builder called cartero based on browserify and we use the crypto module in some pages and since this change we are getting the above error.

Thanks again for the answer and check this issue.

[rvagg]

Ah, ok so this was my fault. I should have included the buffer package in package.json or not done the explicit import. There’s a tricky choice here:

• Use implicit Buffer but cause problems for Webpack 5+ which doesn’t support Node globals by default
• Use explicit Buffer from a require() and also depend on the ‘buffer’ package https://github.com/feross/buffer

In Node.js, require(‘buffer’) will still pull in the core Buffer API, which is what we want in Node.js (we don’t want buffer/, it’s not going to be as performant in many cases). In the browser, require(‘buffer’) will pull in the ‘buffer’ package.

So IMO the solution here is to add the ‘buffer’ package, I’ll put in a PR to do that as soon as I finish this comment. But the other possible way here is to back out my explicit require(‘buffer’) calls if you want to go that way and deal with the Webpack 5+ pain.

[pepoviola]

Hi @rvagg, thanks for look at this issue. Adding the buffer package will fix my case ( is the workaround I implemented locally ).

Again thanks for take the time to look at this 🙌

[calvinmetcalf]

what about #53 ?

[rvagg]

As I said elsewhere, safe-buffer needs to fade into history, it’s entirely redundant now. But you’re more conscious of the linkages between the various components here and the challenges of moving everything in step so I think you’re best placed to make the call.

safe-buffer still doesn’t explicitly pull in buffer, it expects it to implicitly be there, in the browser or Node.js. So you’re still going to face the challenge when folks start wanting explicit Node.js polyfills included because of Webpack 5 and other tools that are phasing out the implicit polyfills. By using safe-buffer you’re putting the onus onto Feross to deal with it, but maybe he’ll just say “don’t use this package anymore, I’m not updating it”.

If you don’t want to deal with this problem at all, we can just remove the require(‘buffer’) lines in my original code. Then we go back to how it was and everything’s supposed to be implicit. I was just doing a bit of future-proofing. I probably chewed off too much while I was in there.

[calvinmetcalf]

@pepoviola maybe just in the short term add

new webpack.ProvidePlugin({
  Buffer: ['buffer', 'Buffer'],
})

[pepoviola]

Hi @calvinmetcalf, thanks for the answer but we don't use webpack. We just use browserify under the hood and I don't know if we can alias the Buffer module from the browserify options.
Thx!

[calvinmetcalf]

oh then maybe try #53

[pepoviola]

Hi @calvinmetcalf, did you want to I try that fix in my environment?
Thx!

[calvinmetcalf]

yes please

[pepoviola]

Hi @calvinmetcalf, yes works ok with #53 .
Thanks!!

[ljharb]

Seems like #53 solved it.