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

JsValue(SyntaxError: An invalid or illegal string was specified) #2128

Closed
renatoathaydes opened this issue May 7, 2020 · 10 comments
Closed
Labels

Comments

@renatoathaydes
Copy link

renatoathaydes commented May 7, 2020

I am trying to pass an array of strings into a Web API.

I believe I am correctly creating the array using this function:

fn js_array(values: &[&str]) -> JsValue {
    return JsValue::from(values.into_iter()
        .map(|x| JsValue::from_str(x))
        .collect::<Array>());
}

When I log this into the console, it shows the correct array.

Screenshot 2020-05-07 at 17 44 42

When I pass this array into the generate_key_with_str method, it causes a crash in the browser.

The code looks like this:

let window = web_sys::window().expect("no global `window` exists");
let crypto = window.crypto().expect("Crypto does not exist").subtle();
let algs = js_array(&["sign"]);

let key_promise = crypto.generate_key_with_str("RSA-PSS", true, &algs)
        .expect("Key not generated");

let key_pair: CryptoKeyPair = JsFuture::from(key_promise).await.unwrap().into();

The error:

panicked at 'called `Result::unwrap()` on an `Err` value: JsValue(SyntaxError: An invalid or illegal string was specified
init/imports.wbg.__wbg_generateKey_a00ff12147e79f53<@http://localhost:8000/pkg/my_module.js:322:35
handleError/<@http://localhost:8000/pkg/my_module.js:224:22

If I do not unwrap in the last line of the code snippet above, then I still get this logged into the console:

SyntaxError: An invalid or illegal string was specified

The JavaScript code that the browser links to is this:

    imports.wbg.__wbg_generateKey_a00ff12147e79f53 = handleError(function(arg0, arg1, arg2, arg3, arg4) {
        var ret = getObject(arg0).generateKey(getStringFromWasm0(arg1, arg2), arg3 !== 0, getObject(arg4));
        return addHeapObject(ret);
    });
@renatoathaydes
Copy link
Author

Sorry, this was a hard day for me as nothing was working... just found out this error comes from the actual Web API, not from web_sys (it seems the algorithm string was wrong, almost always the object method needs to be called as it requires extra parameters, like a salt). SyntaxError had made me think it was some WASM String conversion issue, but it's definitely not.

@Pauan
Copy link
Contributor

Pauan commented May 7, 2020

This isn't a bug with wasm-bindgen, the same thing happens if you use raw JS code:

crypto.subtle.generateKey("RSA-PSS", true, ["sign"]);

As shown on MDN, you must pass an object as the first argument, not a string:

crypto.subtle.generateKey({
    name: "RSA-PSS",
    modulusLength: 4096,
    publicExponent: new Uint8Array([1, 0, 1]),
    hash: "SHA-512"
}, true, ["sign"]);

Incidentally, this is an excellent example of why we cannot autogenerate good static types for web-sys: the object that you pass as the first argument changes depending on the algorithm.

For the RSA types you need this object, for HMAC you need this object, for ECDH you need this object, and for AES you need this object.

This cannot be automatically generated, it requires careful handcrafted types to represent this.

@renatoathaydes
Copy link
Author

This is the type definition for the method that takes String as the first argument:

Screenshot 2020-05-08 at 09 46 39

Most types are not completely perfect on the limitations they impose, but they can always get a lot closer than the current definitions do... sure, it may require some manual intervention if the Web IDL is very poor, but that's a limitation that only exists because of the Web IDL being incorrect.

@Pauan
Copy link
Contributor

Pauan commented May 8, 2020

sure, it may require some manual intervention if the Web IDL is very poor, but that's a limitation that only exists because of the Web IDL being incorrect.

Even if we made manual changes and fixed the WebIDL, the API would still be very clunky and difficult to work with (because it is a low level web-sys API). So the improvements would be very small: the only improvement is that you wouldn't need to use dyn_into.

So your time would be far better spent trying to get a high level crypto API added into gloo. This API would have good idiomatic Rust types (enums and structs), and it would also be far easier to use than the web-sys API.

@renatoathaydes
Copy link
Author

So your time would be far better spent trying to get a high level crypto API added into gloo....

@Pauan this is basically what I am doing in my project, I would be happy to push the types to gloo if that would help.

One problem I am having that I just can't find a way around is: how to create a JS Uint8Array from Rust.

new Uint8Array([1, 0, 1])

Any idea?

@Pauan
Copy link
Contributor

Pauan commented May 8, 2020

@renatoathaydes You just call Uint8Array::new:

Uint8Array::new(&[1, 0, 1].iter().map(|x| JsValue::from(*x)).collect::<Array>())

Or alternatively:

Uint8Array::new(&Array::of3(&JsValue::from(1), &JsValue::from(0), &JsValue::from(1)))

@renatoathaydes
Copy link
Author

I had trouble reading the code of js_sys that creates Uint8Array and there was no example anywhere I could find. The type is created in the arrays! macro, when I understood that, then I was able to figure out how that is done. It's quite impressive, but hard to follow. I think I got the object build correctly now. Doesn't look pretty, but it works:

fn rsa_hashed_key_params(name: &str,
                         modulus_len: i32,
                         public_exp: &[u8],
                         hash: &str) -> Object {
    let key_params = RsaHashedKeyGenParams::from(JsValue::from(Object::new()));
    key_params.set_name(&name.into());
    key_params.set_modulus_length(&JsValue::from(modulus_len));
    let pexp = js_sys::Uint8Array::new_with_length(public_exp.len() as u32);
    for i in 0..public_exp.len() {
        pexp.set_index(i as u32, public_exp[i]);
    }
    key_params.set_public_exponent(&pexp);
    key_params.set_hash(&hash.into());
    let js = JsValue::from(key_params);
    web_sys::console::log_2(&"key params".into(), &js);
    js.into()
}

#[wasm_bindgen]
extern "C" {
    pub type RsaHashedKeyGenParams;

    #[wasm_bindgen(structural, method, setter)]
    pub fn set_name(this: &RsaHashedKeyGenParams, value: &JsValue);

    #[wasm_bindgen(structural, method, setter)]
    pub fn set_modulus_length(this: &RsaHashedKeyGenParams, value: &JsValue);

    #[wasm_bindgen(structural, method, setter)]
    pub fn set_public_exponent(this: &RsaHashedKeyGenParams, value: &JsValue);

    #[wasm_bindgen(structural, method, setter)]
    pub fn set_hash(this: &RsaHashedKeyGenParams, value: &JsValue);
}

Which prints this:

Screenshot 2020-05-08 at 15 29 18

Do you have any feedback on the above? Is it the "right" way to do it?

Will see if I can adapt your snippet above to create the u8 array.

@renatoathaydes
Copy link
Author

The RsaHashedKeyGenParams type seems pretty useless as is... I could use Object and Reflect to create the untyped JS Object as that's all that the generate_key method needs... not sure how that would be handled in a library like gloo though.

@renatoathaydes
Copy link
Author

The case-style of the object keys was wrong... I fixed it like this:

    #[wasm_bindgen(structural, method, setter, js_name = "modulusLength")]
    pub fn set_modulus_length(this: &RsaHashedKeyGenParams, value: &JsValue);

I had assumed it would automatically change the case.

@Pauan
Copy link
Contributor

Pauan commented May 8, 2020

I had trouble reading the code of js_sys that creates Uint8Array and there was no example anywhere I could find.

The web-sys and js-sys APIs are generally one-to-one with JS, so it's straightforward to take JS code and translate it into Rust. Also the docs always link to the relevant MDN documentation.

Is it the "right" way to do it?

Generally speaking, yes. Using Object and Reflect::set is another equally valid option.

not sure how that would be handled in a library like gloo though.

I imagine it would just use normal structs/enums:

enum Hash {
    Sha256,
    Sha384,
    Sha512,
}

struct RsaOptions {
    modulus_length: u32,
    hash: Hash,
    public_exponent: u64,
}

enum EcCurve {
    P256,
    P384,
    P521,
}

struct EcOptions {
    named_curve: EcCurve,
}

struct HmacOptions {
    hash: Hash,
}

enum AesLength {
    Bits128,
    Bits192,
    Bits256,
}

struct AesOptions {
    length: AesLength,
}

enum Algorithm {
    RsaSsaPkcs(RsaOptions),
    RsaPss(RsaOptions),
    RsaOaep(RsaOptions),
    Ecdsa(EcOptions),
    Ecdh(EcOptions),
    Hmac(HmacOptions),
    AesCtr(AesOptions),
    AesCbc(AesOptions),
    AesGcm(AesOptions),
    AesKw(AesOptions),
}

Then it would internally create JS objects based on those structs/enums.

I had assumed it would automatically change the case.

We have an issue for that: #1818

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants