winksaville / exper_inter_process_channel

Experiment with inter-process channels
MIT License
0 stars 0 forks source link

MsgHeader::id is not used to validate deserialization #1

Open winksaville opened 1 year ago

winksaville commented 1 year ago

I've stumbled across a critical issue, the serialization output of two different messages can be identical.

This happened to me when I serde Msg1 and then copied/pasted the code to test Msg2 and didn't change the type from serde_json::from_str::<Msg1> to <Msg2>. Because the serialization of Msg1 and Msg2 are identical deserialization works just fine. But the rust TypeId is Msg1 while the MsgHeader::id field is the MSG2_ID value.

See the test_identical_json which shows that serde_json::from_str succeeds when it would be preferable that it fail.

I searched for how to validate during serde::from_string and issue serde issue 939 may help. Of course there may be better solutions too :)

use std::any::{Any, TypeId};

use msg1::Msg1;
use msg2::{Msg2, MSG2_ID};

#[test]
/// CAREFUL: Deserializing a Msg2 to a Msg1 "works",
/// but it shouldn't as they have different MsgIds.
fn test_identical_json() {
    // Create a Box<Msg2>
    let msg2 = Box::<Msg2>::default();
    let ser_msg2 = serde_json::to_string(&msg2).unwrap();

    // Deserialize to Msg1, this should fail but currently it succeeds
    let deser_bad_msg2 = serde_json::from_str::<Msg1>(&ser_msg2);
    match deser_bad_msg2 {
        Ok(bad_msg2) => {
            println!("test test_identical_json: `serde_json::from_str::<Msg1>(&ser_msg2)` should fail as id is MSG2_ID not MSG1_ID");

            // Rust thinks this is a Msg1
            assert_eq!(TypeId::of::<Msg1>(), bad_msg2.type_id());
            // But the header.id is MSG2_ID
            assert_eq!(bad_msg2.header.id, MSG2_ID);
        }
        Err(_) => panic!("This is expected"),
    }
}

The --nocapture output indicates this "should fail":

$ cargo test -- --nocapture
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests src/main.rs (target/debug/deps/exper_inter_process_channel-8d5194b92dbbae52)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/test.rs (target/debug/deps/test-1ac436d00ca818e3)

running 1 test
test test_identical_json: `serde_json::from_str::<Msg1>(&ser_msg2)` should fail as id is MSG2_ID not MSG1_ID
test test_identical_json ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
winksaville commented 1 year ago

I've now got fn get_id_str_from_buf: https://github.com/winksaville/exper_inter_process_channel/blob/0eb7a71ae3c48e359e4aefc129ed87aca557d7fc/msg_serde_json/src/lib.rs#L6-L25 And use it in msg1: https://github.com/winksaville/exper_inter_process_channel/blob/0eb7a71ae3c48e359e4aefc129ed87aca557d7fc/msg1/src/lib.rs#L49-L53 and msg_serded_macro: https://github.com/winksaville/exper_inter_process_channel/blob/0eb7a71ae3c48e359e4aefc129ed87aca557d7fc/msg_serde_macro/src/lib.rs#L43-L46 to validate a msg is being deserialized properly.


I've also added fn get_msg_id_from_boxed_msg_any: https://github.com/winksaville/exper_inter_process_channel/blob/0eb7a71ae3c48e359e4aefc129ed87aca557d7fc/msg_header/src/lib.rs#L33-L38 and it can be used to get a MsgId from BoxedMsgAny and then use a hashmap to convert a BoxedMsgAny directly to a serialized format using a HashMap<<MsgId>, ToSerdeJsonBuf> such as I've done here: https://github.com/winksaville/exper_inter_process_channel/blob/0eb7a71ae3c48e359e4aefc129ed87aca557d7fc/src/main.rs#L208-L216