browserify / randombytes

random bytes from browserify stand alone
MIT License
99 stars 47 forks source link

Fix crypto global detection #1

Closed feross closed 9 years ago

feross commented 9 years ago

Recent changes to crypto-browserify make browserify tests fail. This fixes them.

jprichardson commented 9 years ago

ACK

calvinmetcalf commented 9 years ago

Yeah looks good, can merge and publish later tonight or first thing tommorow unless somebody beats me to it.

Curious how this gets triggered as this file shouldn't actually be run in a non browser context

dcousens commented 9 years ago

ACK.

This would fail in a browser that doesn't have window.crypto because crypto is undefined and therefore .getRandomValues is also undefined, leading to the undefined error.

calvinmetcalf commented 9 years ago

@dcousens are you able to publish?

feross commented 9 years ago

Right. It fails the browserify tests too because they run the browserified code in node with vm.runInContext where a crypto global is also lacking. On Wed, Jan 21, 2015 at 4:02 PM Calvin Metcalf notifications@github.com wrote:

@dcousens https://github.com/dcousens are you able to publish?

— Reply to this email directly or view it on GitHub https://github.com/crypto-browserify/randombytes/pull/1#issuecomment-70947347 .

dcousens commented 9 years ago

Published as 2.0.1

dcousens commented 9 years ago

@calvinmetcalf we should probably test in the same way, to avoid our dependents finding this out for us after publication.

feross commented 9 years ago

Thanks for the quick fix and merge.