prost icon indicating copy to clipboard operation
prost copied to clipboard

Providing own existing types instead of generated ones for better message validation

Open Victor-N-Suadicani opened this issue 2 years ago • 4 comments

Is it possible to provide an existing type as the decoding type for a protobuf message? Say I have this protobuf:

message FloatMsg {
    float x = 1;
}

Using prost-build, this would give me

pub struct FloatMsg {
    #[prost(float, tag="1")]
    pub x: f32,
}

However, what if, for my purposes, I want to ensure that x is never NaN (quite a common requirement)? Of course I could define my own NotNanFloatMsg { pub x: NotNan } using NotNan from the ordered_float crate and then provide a TryFrom<FloatMsg> for NotNanFloatMsg implementation.

However, when I receive a message I would then have to both call decode to get the protobuf and then try_into to get my properly validated type. And I would also have to convert it back again if I have another message in my protobuf file that uses FloatMsg.

My question is: Is there/could there be a way to merge these two operations (decode and try_into) into one? This would fit nicely with the principle of "Parse, don't validate", and would ensure statically that I would never have a NaN (or whatever my validation requirements are).

One way I imagine this could work is that one could somehow tell prost-build, "don't generate a FloatMsg, use this crate::FloatMsg instead" where crate::FloatMsg could be defined however you want (for instance, with NotNaN). crate::FloatMsg would then have to provide a way to be decoded from the protobuf - how exactly this should be done I'm not sure. Perhaps a derive of Message and a provided TryFrom<(f32,)>?

This would be very similar to the use case of serde's try_from attribute. This attribute basically allows you to perform a little bit of extra validation during deserialization. I'm basically looking for a way to do something similar in prost.

Victor-N-Suadicani avatar Aug 09 '22 12:08 Victor-N-Suadicani

At this point unless you can capture thetry_into into the Message impl of your type it won't be possible. That said, I do think separating decoding & validation is an okay boundary and you can always write a fn locally that triggers both methods. It is also possible to use the derive macros manually though I am not sure how smooth that experience is.

LucioFranco avatar Aug 09 '22 16:08 LucioFranco

It is also possible to use the derive macros manually though I am not sure how smooth that experience is.

The thing that ruins the experience of this the most is that you can't tell prost-build to skip a message that you will provide yourself. So even if you use the derive macros yourself, you still have to convert back and forth (because the message probably appears as part of other protobuf messages and prost-build will make it reference its own generated version).

If, for example, the Config struct had a replace(proto_msg: &str, rust_type: &str) method, that could solve all of this, though not in a super ergonomic way. Basically you would be able to say replace("some_proto_package.FloatMsg", "crate::FloatMsg") and prost-build would just skip generating any code for some_proto_package.FloatMsg and for all the places in the proto where that message is used, it would use crate::FloatMsg as the type instead. Does that make sense?

Given this, I guess I could expand the derive macros and manually modify the decode behaviour in a way that achieves the decode + try_into behaviour I wanted.

Victor-N-Suadicani avatar Aug 10 '22 07:08 Victor-N-Suadicani

I think adding that config item makes a lot of sense and adds flexibility. I know its not the most ergonomic for your use case but I could see it maybe being useful for others, so maybe that seems like a good addition.

LucioFranco avatar Aug 10 '22 16:08 LucioFranco

I just realized that #427 is actually a very similar, if not identical issue to this that I hadn't seen previously.

Victor-N-Suadicani avatar Aug 30 '22 10:08 Victor-N-Suadicani