Structs with nested flattens cannot be deserialized if deny_unknown_fields is set

[sebastianblunt]

Minimal repro, returns

thread 'main' panicked at 'called Result::unwrap() on an Err value: Error("unknown field c", line: 2, column: 24)', src/libcore/result.rs:999:5

Cargo.toml:

[dependencies]
serde = { version = "1", features = ["derive"] }
serde_json = "1"

main.rs

use serde::Deserialize;

fn main() {
    let x = r#"
    {"c": "Some string"}
    "#;

    let x: A = serde_json::from_str(x).unwrap();

    eprintln!("{:?}", x);
}

#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)] // Remove this line and it works
struct A {
    #[serde(flatten)]
    a: B,
}

#[derive(Debug, Deserialize)]
struct B {
    #[serde(flatten)]
    b: C,
}

#[derive(Debug, Deserialize)]
struct C {
    c: String,
}

Removing the #[serde(deny_unknown_fields)] line causes it to work.

Possibly related to #1358?

[Kestrer]

The cause for this is that the way #[serde(flatten)] works is that it deserializes the container as a map instead of a struct, and holds a map of unknown entries as it goes through. Then at the end it uses the field's Deserialize implementation to deserialize from said map.

Now, if the flattened type then deserializes with a struct, Serde knows the fields that the type needs. It supplies the type with only those fields, and it removes them from the map of unknown entries. However, if the type deserializes as map, Serde does not know the fields the type needs and so it gives all the fields to it, but since it doesn't know whether the type has used all the fields it has given it, it doesn't remove those entries from the map. And as I said above, using #[serde(flatten)] in a field causes it to be deserialized as a map.

As a side effect, this does work, but shouldn't:

#[derive(Deserialize)]
struct AInner {
    a: i32,
}
#[derive(Deserialize)]
struct A {
    #[serde(flatten)]
    inner: AInner,
}

#[derive(Deserialize)]
struct ShouldntDeserialize {
    a: i32,
}

#[derive(Deserialize)]
struct Container {
    #[serde(flatten)]
    a: A,
    #[serde(flatten)]
    shouldnt_deserialize: ShouldntDeserialize,
}

AInner should in theory "consume" a and not allow it to be readable after that, but since A is deserialized as a map it can't and so ShouldntDeserialize is able to access a.

Anyway back to the original example, Serde's deserialized most of A and just needs to run the deny_unknown_fields check. So are there any extra fields in the unknown fields map? Yes, there is: c, since it wasn't removed when deserializing B, since B contains #[serde(flatten)] and therefore deserializes as a map.

I don't think this problem is solvable without a breaking change. #[serde(flatten)] will have to be completely reworked, and possibly lots of Deserializer too if we need to accomodate it.

[Mingun]

This is indeed a duplicate of #1358