RReverser/serde-wasm-bindgen

`serde-wasm-bindgen` doesn't handle `serde(deny_unknown_fields)` properly

sergeyboyko0791 opened this issue · 3 comments

An example:

use wasm_bindgen_test::*;
use serde::{Deserialize, Serialize};

wasm_bindgen_test_configure!(run_in_browser);

#[wasm_bindgen_test]
fn test_deny_unknown_fields() {
    #[derive(Debug, Deserialize, Serialize)]
    #[serde(deny_unknown_fields)]
    pub struct RPCErrorExtra {
        pub code: i32,
        pub message: String,
        pub data: Option<serde_json::Value>,
        pub stack: Option<serde_json::Value>,
    }

    let serializer = serde_wasm_bindgen::Serializer::json_compatible();

    let json = r#"{"code": 1, "message": "", "extra": true}"#;
    let json_value = serde_json::from_str::<serde_json::Value>(json).unwrap();
    let js_value = json_value.serialize(&serializer).unwrap();
    // This should fail, but it doesn't.
    serde_wasm_bindgen::from_value::<RPCErrorExtra>(js_value).unwrap_err();
}

Panics with:

panicked at 'called Result::unwrap_err() on an Ok value: RPCErrorExtra { code: 1, message: "", data: None, stack: None }

Yeah I'm afraid you'll have to manually capture extra fields like here: https://serde.rs/attr-flatten.html#capture-additional-fields

The reason is that, unlike flatten, deny_unknown_fields doesn't switch deserialization from structs to maps, and we have no way of knowing whether Serde will want those extra fields or not.

Being able to pick only known properties from JS object instead of iterating over its all key-value pairs as a map is a significant perf optimisation, and I wouldn't want to remove it for the relatively rare attribute.

Ideally Serde upstream should switch to deserialize_map when deny_unknown_fields is used, but I'm not sure if they'd be open to do so.

The easiest solution for now might be to use a helper like this:

fn deserialize_deny_unknown_fields<'de, T: Deserialize<'de>>(value: JsValue) -> Result<T, serde_wasm_bindgen::Error> {
    #[derive(Deserialize)]
    struct Wrap<T> {
        #[serde(flatten)]
        result: T,
        #[serde(flatten)]
        extra_fields: HashMap<String, serde::de::IgnoredAny>,
    }

    let Wrap {
        result,
        extra_fields,
    } = serde_wasm_bindgen::from_value(value)?;
    if !extra_fields.is_empty() {
        return Err(serde::de::Error::custom(format_args!(
            "Unexpected fields: {:?}",
            extra_fields.keys().collect::<Vec<_>>()
        )));
    }
    Ok(result)
}

@RReverser thank you for the answer! Will follow your advice.