dtolnay / erased-serde

Type-erased Serialize, Serializer and Deserializer traits
Apache License 2.0
709 stars 36 forks source link

Stack overflow when using serialize_trait_object on trait that does not have Serialize as supertrait #51

Closed dtolnay closed 2 years ago

dtolnay commented 2 years ago
use erased_serde::serialize_trait_object;
use serde::Serialize;

pub trait MyTrait {}

serialize_trait_object!(MyTrait);

impl MyTrait for i32 {}

fn main() {
    let trait_object: &dyn MyTrait = &0i32;
    let _ = Serialize::serialize(trait_object, serde_json::value::Serializer);
}
$ cargo run
thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

The serialize_trait_object! call is expanding to something like:

impl<'erased> ::serde::Serialize for dyn MyTrait + 'erased {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: ::serde::Serializer,
    {
        ::erased_serde::serialize(self, serializer)
    }
}

which calls https://docs.rs/erased-serde/0.3.18/erased_serde/fn.serialize.html. The first argument of that function call is constrained by a trait bound T: erased_serde::Serialize. The serialize_trait_object! macro is counting on dyn MyTrait to have a compiler-generated impl erased_serde::Serialize for dyn MyTrait because it's expecting trait MyTrait: erased_serde::Serialize at the trait definition.

However, in the case that the caller forgot their erased_serde::Serialize supertrait, the impl being selected instead is erased-serde's impl<T> erased_serde::Serialize for T where T: ?Sized + serde::Serialize, which involves calling the serde Serialize impl. But the serde Serialize impl in this case is what's shown above, impl<'erased> serde::Serialize for dyn MyTrait + 'erased, which involves calling the erased-serde impl. Thus cycle and stack overflow.

The fact that there are 2 potentially applicable impls of erased_serde::Serialize for dyn MyTrait (one compiler-generated and one handwritten blanket impl in the erased-serde crate) is related to the soundness issue https://github.com/rust-lang/rust/issues/57893 but is not unsound in our case because erased_serde::Serialize has no associated types.

The fix will be for serialize_trait_object! to verify that the given trait has the required erased_serde::Serialize supertrait, for example by doing:

fn require<T: ?Sized + erased_serde::Serialize>() {}
fn check<T: ?Sized + MyTrait>() { require::<T>() }