Cleanup randomFill. NFC (#23260)

Under node we can rely on `randomFillSync` always being available since its been around since v9.0.0: https://nodejs.org/api/crypto.html#cryptorandomfillsyncbuffer-offset-size Also, remove the return value from this function since it always fills the entire buffer. https://caniuse.com/getrandomvalues
emscripten-core · Dec 30, 2024 · 5e3fab4 · 5e3fab4
1 parent 5a8d9e5
commit 5e3fab4
Show file tree

Hide file tree

Showing 35 changed files with 51 additions and 70 deletions.
diff --git a/src/closure-externs/node-externs.js b/src/closure-externs/node-externs.js
@@ -137,3 +137,5 @@ path.isAbsolute;
  * @type {Object.<string,*>}
  */
 path.posix;
+
+crypto.randomFillSync;
diff --git a/src/library_fs.js b/src/library_fs.js
@@ -1411,7 +1411,8 @@ FS.staticInit();
       var randomBuffer = new Uint8Array(1024), randomLeft = 0;
       var randomByte = () => {
         if (randomLeft === 0) {
-          randomLeft = randomFill(randomBuffer).byteLength;
+          randomFill(randomBuffer);
+          randomLeft = randomBuffer.byteLength;
         }
         return randomBuffer[--randomLeft];
       };

diff --git a/src/library_wasi.js b/src/library_wasi.js
@@ -561,53 +561,35 @@ var WasiLibrary = {
   // random.h
 
   $initRandomFill: () => {
-    if (typeof crypto == 'object' && typeof crypto['getRandomValues'] == 'function') {
-      // for modern web browsers
+    if (typeof crypto == 'object' && typeof crypto.getRandomValues == 'function') {
 #if SHARED_MEMORY
       // like with most Web APIs, we can't use Web Crypto API directly on shared memory,
       // so we need to create an intermediate buffer and copy it to the destination
-      return (view) => (
-        view.set(crypto.getRandomValues(new Uint8Array(view.byteLength))),
-        // Return the original view to match modern native implementations.
-        view
-      );
+      return (view) => view.set(crypto.getRandomValues(new Uint8Array(view.byteLength)));
 #else
       return (view) => crypto.getRandomValues(view);
 #endif
-    } else
+    }
+
 #if ENVIRONMENT_MAY_BE_NODE
     if (ENVIRONMENT_IS_NODE) {
-      // for nodejs with or without crypto support included
-      try {
-        var crypto_module = require('crypto');
-        var randomFillSync = crypto_module['randomFillSync'];
-        if (randomFillSync) {
-          // nodejs with LTS crypto support
-          return (view) => crypto_module['randomFillSync'](view);
-        }
-        // very old nodejs with the original crypto API
-        return (view) => (
-          view.set(crypto_module['randomBytes'](view.byteLength)),
-          // Return the original view to match modern native implementations.
-          view
-        );
-      } catch (e) {
-        // nodejs doesn't have crypto support
-      }
+      return (view) => require('crypto').randomFillSync(view);
     }
 #endif // ENVIRONMENT_MAY_BE_NODE
-    // we couldn't find a proper implementation, as Math.random() is not suitable for /dev/random, see emscripten-core/emscripten/pull/7096
+
+    // we couldn't find a proper implementation, as Math.random() is not
+    // suitable for /dev/random, see emscripten-core/emscripten/pull/7096
 #if ASSERTIONS
-    abort('no cryptographic support found for randomDevice. consider polyfilling it if you want to use something insecure like Math.random(), e.g. put this in a --pre-js: var crypto = { getRandomValues: (array) => { for (var i = 0; i < array.length; i++) array[i] = (Math.random()*256)|0 } };');
+    abort('no cryptographic support found for random function. consider polyfilling it if you want to use something insecure like Math.random(), e.g. put this in a --pre-js: var crypto = { getRandomValues: (array) => { for (var i = 0; i < array.length; i++) array[i] = (Math.random()*256)|0 } };');
 #else
-    abort('initRandomDevice');
+    abort();
 #endif
   },
 
   $randomFill__deps: ['$initRandomFill'],
   $randomFill: (view) => {
     // Lazily init on the first invocation.
-    return (randomFill = initRandomFill())(view);
+    (randomFill = initRandomFill())(view);
   },
 
   random_get__proxy: 'none',

diff --git a/test/other/codesize/test_codesize_cxx_ctors1.gzsize b/test/other/codesize/test_codesize_cxx_ctors1.gzsize
@@ -1 +1 @@
-8405
+8365
diff --git a/test/other/codesize/test_codesize_cxx_ctors1.jssize b/test/other/codesize/test_codesize_cxx_ctors1.jssize
@@ -1 +1 @@
-20468
+20373
diff --git a/test/other/codesize/test_codesize_cxx_ctors1.size b/test/other/codesize/test_codesize_cxx_ctors1.size
@@ -1 +1 @@
-128894
+128914
diff --git a/test/other/codesize/test_codesize_cxx_ctors2.gzsize b/test/other/codesize/test_codesize_cxx_ctors2.gzsize
@@ -1 +1 @@
-8390
+8347
diff --git a/test/other/codesize/test_codesize_cxx_ctors2.jssize b/test/other/codesize/test_codesize_cxx_ctors2.jssize
@@ -1 +1 @@
-20436
+20341
diff --git a/test/other/codesize/test_codesize_cxx_ctors2.size b/test/other/codesize/test_codesize_cxx_ctors2.size
@@ -1 +1 @@
-128343
+128363
diff --git a/test/other/codesize/test_codesize_cxx_except.gzsize b/test/other/codesize/test_codesize_cxx_except.gzsize
@@ -1 +1 @@
-9412
+9369
diff --git a/test/other/codesize/test_codesize_cxx_except.jssize b/test/other/codesize/test_codesize_cxx_except.jssize
@@ -1 +1 @@
-24237
+24141
diff --git a/test/other/codesize/test_codesize_cxx_except.size b/test/other/codesize/test_codesize_cxx_except.size
@@ -1 +1 @@
-170928
+170948
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm.gzsize b/test/other/codesize/test_codesize_cxx_except_wasm.gzsize
@@ -1 +1 @@
-8370
+8329
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm.jssize b/test/other/codesize/test_codesize_cxx_except_wasm.jssize
@@ -1 +1 @@
-20361
+20266
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm.size b/test/other/codesize/test_codesize_cxx_except_wasm.size
@@ -1 +1 @@
-142143
+142163
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize b/test/other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize
@@ -1 +1 @@
-8370
+8329
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm_exnref.jssize b/test/other/codesize/test_codesize_cxx_except_wasm_exnref.jssize
@@ -1 +1 @@
-20361
+20266
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm_exnref.size b/test/other/codesize/test_codesize_cxx_except_wasm_exnref.size
@@ -1 +1 @@
-144730
+144750
diff --git a/test/other/codesize/test_codesize_cxx_lto.gzsize b/test/other/codesize/test_codesize_cxx_lto.gzsize
@@ -1 +1 @@
-8404
+8363
diff --git a/test/other/codesize/test_codesize_cxx_lto.jssize b/test/other/codesize/test_codesize_cxx_lto.jssize
@@ -1 +1 @@
-20492
+20397
diff --git a/test/other/codesize/test_codesize_cxx_lto.size b/test/other/codesize/test_codesize_cxx_lto.size
@@ -1 +1 @@
-121844
+121864
diff --git a/test/other/codesize/test_codesize_cxx_mangle.gzsize b/test/other/codesize/test_codesize_cxx_mangle.gzsize
@@ -1 +1 @@
-9416
+9375
diff --git a/test/other/codesize/test_codesize_cxx_mangle.jssize b/test/other/codesize/test_codesize_cxx_mangle.jssize
@@ -1 +1 @@
-24237
+24142
diff --git a/test/other/codesize/test_codesize_cxx_mangle.size b/test/other/codesize/test_codesize_cxx_mangle.size
@@ -1 +1 @@
-232437
+232457
diff --git a/test/other/codesize/test_codesize_cxx_noexcept.gzsize b/test/other/codesize/test_codesize_cxx_noexcept.gzsize
@@ -1 +1 @@
-8405
+8365
diff --git a/test/other/codesize/test_codesize_cxx_noexcept.jssize b/test/other/codesize/test_codesize_cxx_noexcept.jssize
@@ -1 +1 @@
-20468
+20373
diff --git a/test/other/codesize/test_codesize_cxx_noexcept.size b/test/other/codesize/test_codesize_cxx_noexcept.size
@@ -1 +1 @@
-131701
+131721
diff --git a/test/other/codesize/test_codesize_cxx_wasmfs.gzsize b/test/other/codesize/test_codesize_cxx_wasmfs.gzsize
@@ -1 +1 @@
-3646
+3600
diff --git a/test/other/codesize/test_codesize_cxx_wasmfs.jssize b/test/other/codesize/test_codesize_cxx_wasmfs.jssize
@@ -1 +1 @@
-7892
+7794
diff --git a/test/other/codesize/test_codesize_cxx_wasmfs.size b/test/other/codesize/test_codesize_cxx_wasmfs.size
@@ -1 +1 @@
-168848
+168868
diff --git a/test/other/codesize/test_codesize_files_js_fs.gzsize b/test/other/codesize/test_codesize_files_js_fs.gzsize
@@ -1 +1 @@
-7699
+7656
diff --git a/test/other/codesize/test_codesize_files_js_fs.jssize b/test/other/codesize/test_codesize_files_js_fs.jssize
@@ -1 +1 @@
-18962
+18867
diff --git a/test/other/codesize/test_codesize_files_wasmfs.gzsize b/test/other/codesize/test_codesize_files_wasmfs.gzsize
@@ -1 +1 @@
-2880
+2837
diff --git a/test/other/codesize/test_codesize_files_wasmfs.jssize b/test/other/codesize/test_codesize_files_wasmfs.jssize
@@ -1 +1 @@
-6180
+6083
diff --git a/test/test_other.py b/test/test_other.py
@@ -15235,18 +15235,14 @@ def test_js_only_settings(self):
   def test_uuid(self):
     # We run this test in Node/SPIDERMONKEY and browser environments because we
     # try to make use of high quality crypto random number generators such as
-    # crypto.getRandomValues or randomBytes (if available).
-
-    # Use closure compiler so we can check that require('crypto').randomBytes and
-    # window.crypto.getRandomValues doesn't get minified out.
+    # `getRandomValues` or `randomFillSync`.
     self.do_runf('test_uuid.c', emcc_args=['-O2', '--closure=1', '-luuid'])
 
+    # Check that test.js compiled with --closure 1 contains `randomFillSync` and
+    # `getRandomValues`
     js_out = read_file('test_uuid.js')
-
-    # Check that test.js compiled with --closure 1 contains ".randomBytes(" and
-    # ".getRandomValues("
-    self.assertContained(".randomBytes(", js_out)
-    self.assertContained(".getRandomValues(", js_out)
+    self.assertContained('.randomFillSync(', js_out)
+    self.assertContained('.getRandomValues(', js_out)
 
   def test_wasm64_no_asan(self):
     err = self.expect_fail([EMCC, test_file('hello_world.c'), '-sMEMORY64', '-fsanitize=address'])