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

deny_unknown_fields incorrectly fails with flattened untagged enum #1600

Open
ggriffiniii opened this issue Aug 13, 2019 · 4 comments
Open

Comments

@ggriffiniii
Copy link

If #[serde(deny_unkown_fields)] is applied to a container that contains a flattened untagged enum it will fail to deserialize inputs even if the input does not contain any unknown fields.

use serde::Deserialize;
#[derive(Debug,Deserialize)]
#[serde(deny_unknown_fields)]
struct Outer {
    #[serde(flatten)]
    inner: UntaggedEnum,
}

#[derive(Debug,Deserialize)]
#[serde(untagged)]
enum UntaggedEnum {
    A(A),
    B(B),
}

#[derive(Debug,Deserialize)]
struct A {
    a_attr: i64,
}

#[derive(Debug,Deserialize)]
struct B {
    b_attr: i64,
}

fn main() {
    let out: Outer = serde_json::from_str(r#"{"b_attr":100}"#).unwrap();
    println!("{:#?}", out);
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=300860522d5f6c7b5b83f7e9b3b95157

@ghost
Copy link

ghost commented Sep 2, 2019

@ggriffiniii It looks like what you want to achieve can be done by moving deny_unknown_fields from the outer struct to the inner enum:

use serde::Deserialize;
#[derive(Debug,Deserialize)]
struct Outer {
    #[serde(flatten)]
    inner: UntaggedEnum,
}

#[derive(Debug,Deserialize)]
#[serde(untagged)]
enum UntaggedEnum {
    A(A),
    B(B),
}

#[derive(Debug,Deserialize)]
#[serde(deny_unknown_fields)]
struct A {
    a_attr: i64,
}

#[derive(Debug,Deserialize)]
#[serde(deny_unknown_fields)]
struct B {
    b_attr: i64,
}

fn main() {
    let out: Outer = serde_json::from_str(r#"{"b_attr":100}"#).unwrap();
    println!("{:#?}", out);
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8da23d148ab4db0d4d7cc9c22be18c7a

All fields unknown by the outer struct would be passed to the inner structs and then rejected.

However, this approach would not work with multiple inner enums.

@optozorax
Copy link

Same problem here, but with enums:

use serde::{Serialize, Deserialize};
use serde_json::Value as Json;

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
// #[serde(deny_unknown_fields)] // This line causes invalid errors in serialization
#[serde(tag = "msg")]
enum Msg {
    A {
        #[serde(flatten)]
        a: AEnum,
    },

    B {
        b: String,
    },
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
#[serde(deny_unknown_fields)]
#[serde(untagged)]
enum AEnum {
    One {
        one: String,
    },
    Two {
        two: String,
    }
}

fn main() {
    // This works fine, but try to uncomment line 5, and this causes a error
    assert_eq!(
        serde_json::from_str::<Msg>(r#"{"msg": "A", "one": "1"}"#).unwrap(), 
        Msg::A { a: AEnum::One { one: "1".to_string() } }
    );
    assert_eq!(
        serde_json::from_str::<Msg>(r#"{"msg": "A", "two": "2"}"#).unwrap(), 
        Msg::A { a: AEnum::Two { two: "2".to_string() } }
    );

    // `deny_unknown_fields` here is working nice
    assert!(serde_json::from_str::<Msg>(r#"{"msg": "A", "one": "1", "field": 1}"#).is_err());
    assert!(serde_json::from_str::<Msg>(r#"{"msg": "A", "two": "2", "other": 2}"#).is_err());

    assert_eq!(
        serde_json::from_str::<Msg>(r#"{"msg": "B", "b": "1"}"#).unwrap(), 
        Msg::B { b: "1".to_string() }
    );

    // `deny_unknown_fields` here not working, assert fails
    assert!(serde_json::from_str::<Msg>(r#"{"msg": "B", "b": "2", "c": 3}"#).is_err());
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=86f6f21557f361288427131716d3c53f

@optozorax
Copy link

optozorax commented Aug 19, 2020

I solve this problem by introduce new struct, and manually match to this struct from old with Option fields:

diff --git a/a.rs b/b.rs
index 536b12c..b9f82f8 100644
--- a/a.rs
+++ b/b.rs
@@ -1,12 +1,23 @@
-use serde::{Serialize, Deserialize};
-use serde_json::Value as Json;
+use serde::Deserialize;
+use std::convert::{TryFrom, TryInto};
 
-#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
-// #[serde(deny_unknown_fields)] // This line causes invalid errors in serialization
+#[derive(Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
 #[serde(tag = "msg")]
+enum MsgDe {
+    A {
+        one: Option<String>,
+        two: Option<String>,
+    },
+
+    B {
+        b: String,
+    },
+}
+
+#[derive(Debug, Eq, PartialEq)]
 enum Msg {
     A {
-        #[serde(flatten)]
         a: AEnum,
     },
 
@@ -15,9 +26,7 @@ enum Msg {
     },
 }
 
-#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
-#[serde(deny_unknown_fields)]
-#[serde(untagged)]
+#[derive(Debug, Eq, PartialEq)]
 enum AEnum {
     One {
         one: String,
@@ -27,26 +36,50 @@ enum AEnum {
     }
 }
 
+impl TryFrom<MsgDe> for Msg {
+    type Error = String;
+
+    fn try_from(value: MsgDe) -> Result<Self, Self::Error> {
+        match value {
+            // Process complicated cases
+            MsgDe::A {
+                one: Some(one),
+                two: None,
+            } => Ok(Msg::A { a: AEnum::One { one } }),
+            MsgDe::A {
+                one: None,
+                two: Some(two),
+            } => Ok(Msg::A { a: AEnum::Two { two } }),
+            
+            // Boilerplate for simple fields
+            MsgDe::B { b } => Ok(Msg::B { b }),
+
+            // Error if complicated cases can't match
+            other => Err(format!("Don't know how to convert: {:?}", other)),
+        }
+    }
+}
+
 fn main() {
-    // This works fine, but try to uncomment line 5, and this causes a error
+    // This works
     assert_eq!(
-        serde_json::from_str::<Msg>(r#"{"msg": "A", "one": "1"}"#).unwrap(), 
-        Msg::A { a: AEnum::One { one: "1".to_string() } }
+        serde_json::from_str::<MsgDe>(r#"{"msg": "A", "one": "1"}"#).unwrap().try_into(), 
+        Ok(Msg::A { a: AEnum::One { one: "1".to_string() } })
     );
     assert_eq!(
-        serde_json::from_str::<Msg>(r#"{"msg": "A", "two": "2"}"#).unwrap(), 
-        Msg::A { a: AEnum::Two { two: "2".to_string() } }
+        serde_json::from_str::<MsgDe>(r#"{"msg": "A", "two": "2"}"#).unwrap().try_into(), 
+        Ok(Msg::A { a: AEnum::Two { two: "2".to_string() } })
     );
 
     // `deny_unknown_fields` here is working nice
-    assert!(serde_json::from_str::<Msg>(r#"{"msg": "A", "one": "1", "field": 1}"#).is_err());
-    assert!(serde_json::from_str::<Msg>(r#"{"msg": "A", "two": "2", "other": 2}"#).is_err());
+    assert!(serde_json::from_str::<MsgDe>(r#"{"msg": "A", "one": "1", "field": 1}"#).is_err());
+    assert!(serde_json::from_str::<MsgDe>(r#"{"msg": "A", "two": "2", "other": 2}"#).is_err());
 
     assert_eq!(
-        serde_json::from_str::<Msg>(r#"{"msg": "B", "b": "1"}"#).unwrap(), 
-        Msg::B { b: "1".to_string() }
+        serde_json::from_str::<MsgDe>(r#"{"msg": "B", "b": "1"}"#).unwrap().try_into(), 
+        Ok(Msg::B { b: "1".to_string() })
     );
 
-    // `deny_unknown_fields` here not working, assert fails
-    assert!(serde_json::from_str::<Msg>(r#"{"msg": "B", "b": "2", "c": 3}"#).is_err());
+    // This works too
+    assert!(serde_json::from_str::<MsgDe>(r#"{"msg": "B", "b": "2", "c": 3}"#).is_err());
 }

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=479f996a6295c8b2da59f5c706ca45bc

@Mingun
Copy link
Contributor

Mingun commented Aug 11, 2023

This is duplicate of #1358.

fleupold added a commit to cowprotocol/services that referenced this issue Jan 2, 2024
# Description
Notification calls from the driver to solver engines are currently
failing due to:

> Failed to deserialize the JSON body into the target type: invalid
type: map, expected variant identifier at line 1 column 143

I broke this with #2192 where I missed adding the flatten directive on
the driver dto.

# Changes
- [x] Fix dto on both sides (we can no longer deny_unknow_fields due to
this [serde bug](serde-rs/serde#1600))
- [x] Warn if the notify call is not successful

## How to test
Run e.g. the partial fill colocation test case. See warning before
adjusting the dto and now no longer see it.
sunce86 pushed a commit to cowprotocol/services that referenced this issue Jan 6, 2024
# Description
Notification calls from the driver to solver engines are currently
failing due to:

> Failed to deserialize the JSON body into the target type: invalid
type: map, expected variant identifier at line 1 column 143

I broke this with #2192 where I missed adding the flatten directive on
the driver dto.

# Changes
- [x] Fix dto on both sides (we can no longer deny_unknow_fields due to
this [serde bug](serde-rs/serde#1600))
- [x] Warn if the notify call is not successful

## How to test
Run e.g. the partial fill colocation test case. See warning before
adjusting the dto and now no longer see it.
flavio added a commit to flavio/policy-server that referenced this issue Jul 17, 2024
There seeems to be issues with the usage of the `deny_unknown_fields`
and untagged enumerations.
serde ends up confused and complains about perfectly valid files.

See serde-rs/serde#1600

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants