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

Proposal for bindgen changes #102

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,14 @@ wasi = "0.5"

[target.wasm32-unknown-unknown.dependencies]
wasm-bindgen = { version = "0.2.29", optional = true }
js-sys = { version = "0.3.27", optional = true }
web-sys = { version = "0.3.27", optional = true, features = ["Window", "WorkerGlobalScope", "Crypto"] }
stdweb = { version = "0.4.18", optional = true }

[features]
std = []
# enables dummy implementation for unsupported targets
dummy = []
bindgen = ["wasm-bindgen", "js-sys", "web-sys"]
Copy link
Member

Choose a reason for hiding this comment

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

Why not rename the package, like I did here? This removes what is otherwise a breaking change to the API (probably resulting in a compile error if someone tries using the wasm-bindgen feature).

To add: might also be worth adding a comment against the optional crates which users should not use as features.

# Unstable feature to support being a libstd dependency
rustc-dep-of-std = ["compiler_builtins", "core"]
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ reporting will be improved on some platforms.
For the `wasm32-unknown-unknown` target, one of the following features should be
enabled:

- [`wasm-bindgen`](https://crates.io/crates/wasm_bindgen)
- [`bindgen`](https://crates.io/crates/wasm_bindgen)
- [`stdweb`](https://crates.io/crates/stdweb)

By default, compiling `getrandom` for an unsupported target will result in
Expand Down
14 changes: 8 additions & 6 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,11 @@ pub(crate) const SEC_RANDOM_FAILED: Error = internal_error!(3);
pub(crate) const RTL_GEN_RANDOM_FAILED: Error = internal_error!(4);
pub(crate) const FAILED_RDRAND: Error = internal_error!(5);
pub(crate) const NO_RDRAND: Error = internal_error!(6);
pub(crate) const BINDGEN_CRYPTO_UNDEF: Error = internal_error!(7);
pub(crate) const BINDGEN_GRV_UNDEF: Error = internal_error!(8);
pub(crate) const STDWEB_NO_RNG: Error = internal_error!(9);
pub(crate) const STDWEB_RNG_FAILED: Error = internal_error!(10);
pub(crate) const BINDGEN_WEB_CRYPTO: Error = internal_error!(7);
pub(crate) const BINDGEN_WEB_FAILED: Error = internal_error!(8);
pub(crate) const BINDGEN_NODE_FAILED: Error = internal_error!(9);
Copy link
Member

Choose a reason for hiding this comment

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

Better not to re-use error codes (except where equivalent)? We aren't short on them, and doing so could cause confusion.

pub(crate) const STDWEB_NO_RNG: Error = internal_error!(10);
pub(crate) const STDWEB_RNG_FAILED: Error = internal_error!(11);

fn internal_desc(error: Error) -> Option<&'static str> {
match error {
Expand All @@ -148,8 +149,9 @@ fn internal_desc(error: Error) -> Option<&'static str> {
RTL_GEN_RANDOM_FAILED => Some("RtlGenRandom: call failed"),
FAILED_RDRAND => Some("RDRAND: failed multiple times: CPU issue likely"),
NO_RDRAND => Some("RDRAND: instruction not supported"),
BINDGEN_CRYPTO_UNDEF => Some("wasm-bindgen: self.crypto is undefined"),
BINDGEN_GRV_UNDEF => Some("wasm-bindgen: crypto.getRandomValues is undefined"),
BINDGEN_WEB_CRYPTO => Some("wasm-bindgen: self.crypto is unavalible"),
BINDGEN_WEB_FAILED => Some("wasm-bindgen: crypto.getRandomValues failed"),
BINDGEN_NODE_FAILED => Some("wasm-bindgen: Node.js crypto.randomFillSync failed"),
STDWEB_NO_RNG => Some("stdweb: no randomness source available"),
STDWEB_RNG_FAILED => Some("stdweb: failed to get randomness"),
_ => None,
Expand Down
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
//! Getrandom also supports `wasm32-unknown-unknown` by directly calling
//! JavaScript methods. Rust currently has two ways to do this: [bindgen] and
//! [stdweb]. Getrandom supports using either one by enabling the
//! `wasm-bindgen` or `stdweb` crate features. Note that if both features are
//! enabled, `wasm-bindgen` will be used. If neither feature is enabled, calls
//! `bindgen` or `stdweb` crate features. Note that if both features are
//! enabled, `bindgen` will be used. If neither feature is enabled, calls
//! to `getrandom` will always fail at runtime.
//!
//! [bindgen]: https://github.com/rust-lang/rust-bindgen
Expand Down Expand Up @@ -234,7 +234,7 @@ cfg_if! {
#[path = "rdrand.rs"] mod imp;
} else if #[cfg(all(target_arch = "wasm32", target_os = "unknown"))] {
cfg_if! {
if #[cfg(feature = "wasm-bindgen")] {
if #[cfg(feature = "bindgen")] {
#[path = "wasm32_bindgen.rs"] mod imp;
} else if #[cfg(feature = "stdweb")] {
#[path = "wasm32_stdweb.rs"] mod imp;
Expand Down
119 changes: 33 additions & 86 deletions src/wasm32_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,107 +7,54 @@
// except according to those terms.

//! Implementation for WASM via wasm-bindgen
extern crate std;
use js_sys::global;
use wasm_bindgen::{prelude::*, JsCast};
use web_sys::{window, Crypto, WorkerGlobalScope};

use core::cell::RefCell;
use core::mem;
use std::thread_local;

use wasm_bindgen::prelude::*;

use crate::error::{BINDGEN_CRYPTO_UNDEF, BINDGEN_GRV_UNDEF};
use crate::Error;

#[derive(Clone, Debug)]
enum RngSource {
Node(NodeCrypto),
Browser(BrowserCrypto),
}

// JsValues are always per-thread, so we initialize RngSource for each thread.
// See: https://github.com/rustwasm/wasm-bindgen/pull/955
thread_local!(
static RNG_SOURCE: RefCell<Option<RngSource>> = RefCell::new(None);
);
use crate::util::LazyBool;
use crate::{error, Error};

pub fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> {
assert_eq!(mem::size_of::<usize>(), 4);

RNG_SOURCE.with(|f| {
let mut source = f.borrow_mut();
if source.is_none() {
*source = Some(getrandom_init()?);
static IS_NODE: LazyBool = LazyBool::new();
if IS_NODE.unsync_init(|| node_crypto().is_some()) {
if node_crypto().unwrap().random_fill_sync(dest).is_err() {
Copy link
Member

Choose a reason for hiding this comment

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

Why .unwrap() and not .ok_or(..)? ?

return Err(error::BINDGEN_NODE_FAILED);
}

match source.as_ref().unwrap() {
RngSource::Node(n) => n.random_fill_sync(dest),
RngSource::Browser(n) => {
// see https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues
//
// where it says:
//
// > A QuotaExceededError DOMException is thrown if the
// > requested length is greater than 65536 bytes.
for chunk in dest.chunks_mut(65536) {
n.get_random_values(chunk)
}
} else {
let crypto = browser_crypto().ok_or(error::BINDGEN_WEB_CRYPTO)?;
// https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues
// > A QuotaExceededError DOMException is thrown if the
// > requested length is greater than 65536 bytes.
for chunk in dest.chunks_mut(65536) {
if crypto.get_random_values_with_u8_array(chunk).is_err() {
return Err(error::BINDGEN_WEB_FAILED);
}
};
Ok(())
})
}

fn getrandom_init() -> Result<RngSource, Error> {
if let Ok(self_) = Global::get_self() {
// If `self` is defined then we're in a browser somehow (main window
// or web worker). Here we want to try to use
// `crypto.getRandomValues`, but if `crypto` isn't defined we assume
// we're in an older web browser and the OS RNG isn't available.

let crypto = self_.crypto();
if crypto.is_undefined() {
return Err(BINDGEN_CRYPTO_UNDEF);
}
}
Ok(())
}

// Test if `crypto.getRandomValues` is undefined as well
let crypto: BrowserCrypto = crypto.into();
if crypto.get_random_values_fn().is_undefined() {
return Err(BINDGEN_GRV_UNDEF);
}
fn node_crypto() -> Option<NodeCrypto> {
node_require("crypto").ok()
}

return Ok(RngSource::Browser(crypto));
fn browser_crypto() -> Option<Crypto> {
// Support calling self.crypto in the main window or a Web Worker.
if let Some(window) = window() {
return window.crypto().ok();
}

return Ok(RngSource::Node(node_require("crypto")));
let worker = global().dyn_into::<WorkerGlobalScope>().ok()?;
worker.crypto().ok()
}

#[wasm_bindgen]
extern "C" {
type Global;
#[wasm_bindgen(getter, catch, static_method_of = Global, js_class = self, js_name = self)]
fn get_self() -> Result<Self_, JsValue>;

type Self_;
#[wasm_bindgen(method, getter, structural)]
fn crypto(me: &Self_) -> JsValue;

#[derive(Clone, Debug)]
type BrowserCrypto;

// TODO: these `structural` annotations here ideally wouldn't be here to
// avoid a JS shim, but for now with feature detection they're
// unavoidable.
#[wasm_bindgen(method, js_name = getRandomValues, structural, getter)]
fn get_random_values_fn(me: &BrowserCrypto) -> JsValue;
#[wasm_bindgen(method, js_name = getRandomValues, structural)]
fn get_random_values(me: &BrowserCrypto, buf: &mut [u8]);

#[wasm_bindgen(js_name = require)]
fn node_require(s: &str) -> NodeCrypto;
#[wasm_bindgen(catch, js_name = require)]
fn node_require(s: &str) -> Result<NodeCrypto, JsValue>;

#[derive(Clone, Debug)]
type NodeCrypto;

#[wasm_bindgen(method, js_name = randomFillSync, structural)]
fn random_fill_sync(me: &NodeCrypto, buf: &mut [u8]);
#[wasm_bindgen(catch, method, js_name = randomFillSync)]
fn random_fill_sync(me: &NodeCrypto, buf: &mut [u8]) -> Result<(), JsValue>;
}
2 changes: 1 addition & 1 deletion tests/wasm_bindgen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ license = "MIT/Apache-2.0"
crate-type = ["cdylib"]

[dependencies]
getrandom = { path = "../..", features = ["wasm-bindgen"] }
getrandom = { path = "../..", features = ["bindgen"] }
wasm-bindgen = "0.2"
wasm-bindgen-test = "0.2"