From d036a8dd5f8b63468acd721ec626aef04bc37cc8 Mon Sep 17 00:00:00 2001 From: Artemis Date: Sun, 18 Jun 2023 21:43:10 +0100 Subject: [PATCH] Allow responding with named MessagePack data. Closes #2107 --- core/lib/Cargo.toml | 1 + core/lib/src/serde/msgpack.rs | 97 ++++++++++++++++++++++-------- core/lib/tests/msgpack_encoding.rs | 97 ++++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 25 deletions(-) create mode 100644 core/lib/tests/msgpack_encoding.rs diff --git a/core/lib/Cargo.toml b/core/lib/Cargo.toml index 44de9448e3..c2efe8e982 100644 --- a/core/lib/Cargo.toml +++ b/core/lib/Cargo.toml @@ -88,3 +88,4 @@ version_check = "0.9.1" [dev-dependencies] figment = { version = "0.10", features = ["test"] } pretty_assertions = "1" +rmp = "0.8" diff --git a/core/lib/src/serde/msgpack.rs b/core/lib/src/serde/msgpack.rs index bb9867e313..9728f35099 100644 --- a/core/lib/src/serde/msgpack.rs +++ b/core/lib/src/serde/msgpack.rs @@ -43,22 +43,12 @@ pub use rmp_serde::decode::Error; /// /// ## Sending MessagePack /// -/// To respond with serialized MessagePack data, return a `MsgPack` type, -/// where `T` implements [`Serialize`] from [`serde`]. The content type of the -/// response is set to `application/msgpack` automatically. +/// To respond with serialized MessagePack data, return either [`Named`] or +/// [`Compact`] from your handler. `T` must implement [`serde::Serialize`]. /// -/// ```rust -/// # #[macro_use] extern crate rocket; -/// # type User = usize; -/// use rocket::serde::msgpack::MsgPack; -/// -/// #[get("/users/")] -/// fn user(id: usize) -> MsgPack { -/// let user_from_id = User::from(id); -/// /* ... */ -/// MsgPack(user_from_id) -/// } -/// ``` +/// Currently, returning `MsgPack` is equivalent to returning `Compact`, +/// but you should prefer to use an explicit option as this default may change +/// in the future. /// /// ## Receiving MessagePack /// @@ -123,9 +113,61 @@ pub use rmp_serde::decode::Error; /// msgpack = 5242880 /// ``` #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct MsgPack(pub T); +pub struct MsgPack(pub T); + +/// Serializes responses in a compact MesagePack format, where structs are +/// serialized as arrays of their field values. +/// +/// To respond with compact MessagePack data, return a `Compact` type, +/// where `T` implements [`Serialize`] from [`serde`]. The content type of the +/// response is set to `application/msgpack` automatically. +/// +/// ```rust +/// # #[macro_use] extern crate rocket; +/// # type User = usize; +/// use rocket::serde::msgpack; +/// +/// #[get("/users/")] +/// fn user(id: usize) -> msgpack::Compact { +/// let user_from_id = User::from(id); +/// /* ... */ +/// msgpack::MsgPack(user_from_id) +/// } +/// ``` +/// +/// Prefer using [`MsgPack`] for request guards, as the named/compact +/// distinction is not relevant for request data - the correct option is +/// implemented automatically. Using [`Compact`] as a request guard will +/// NOT prevent named requests from being accepted. +pub type Compact = MsgPack; -impl MsgPack { +/// Serializes responses in a named MessagePack format, where structs are +/// serialized as maps of their field names and values. +/// +/// To respond with named MessagePack data, return a `Named` type, +/// where `T` implements [`Serialize`] from [`serde`]. The content type of the +/// response is set to `application/msgpack` automatically. +/// +/// ```rust +/// # #[macro_use] extern crate rocket; +/// # type User = usize; +/// use rocket::serde::msgpack; +/// +/// #[get("/users/")] +/// fn user(id: usize) -> msgpack::Named { +/// let user_from_id = User::from(id); +/// /* ... */ +/// msgpack::MsgPack(user_from_id) +/// } +/// ``` +/// +/// Prefer using [`MsgPack`] for request guards, as the named/compact +/// distinction is not relevant for request data - the correct option is +/// implemented automatically. Using [`Named`] as a request guard will +/// NOT prevent compact requests from being accepted. +pub type Named = MsgPack; + +impl MsgPack { /// Consumes the `MsgPack` wrapper and returns the wrapped item. /// /// # Example @@ -142,7 +184,7 @@ impl MsgPack { } } -impl<'r, T: Deserialize<'r>> MsgPack { +impl<'r, T: Deserialize<'r>, const COMPACT: bool> MsgPack { fn from_bytes(buf: &'r [u8]) -> Result { rmp_serde::from_slice(buf).map(MsgPack) } @@ -163,7 +205,7 @@ impl<'r, T: Deserialize<'r>> MsgPack { } #[crate::async_trait] -impl<'r, T: Deserialize<'r>> FromData<'r> for MsgPack { +impl<'r, T: Deserialize<'r>, const COMPACT: bool> FromData<'r> for MsgPack { type Error = Error; async fn from_data(req: &'r Request<'_>, data: Data<'r>) -> Outcome<'r, Self> { @@ -186,9 +228,14 @@ impl<'r, T: Deserialize<'r>> FromData<'r> for MsgPack { /// Serializes the wrapped value into MessagePack. Returns a response with /// Content-Type `MsgPack` and a fixed-size body with the serialization. If /// serialization fails, an `Err` of `Status::InternalServerError` is returned. -impl<'r, T: Serialize> Responder<'r, 'static> for MsgPack { +impl<'r, T: Serialize, const COMPACT: bool> Responder<'r, 'static> for MsgPack { fn respond_to(self, req: &'r Request<'_>) -> response::Result<'static> { - let buf = rmp_serde::to_vec(&self.0) + let maybe_buf = if COMPACT { + rmp_serde::to_vec(&self.0) + } else { + rmp_serde::to_vec_named(&self.0) + }; + let buf = maybe_buf .map_err(|e| { error_!("MsgPack failed to serialize: {:?}", e); Status::InternalServerError @@ -199,7 +246,7 @@ impl<'r, T: Serialize> Responder<'r, 'static> for MsgPack { } #[crate::async_trait] -impl<'v, T: Deserialize<'v> + Send> form::FromFormField<'v> for MsgPack { +impl<'v, T: Deserialize<'v> + Send, const COMPACT: bool> form::FromFormField<'v> for MsgPack { // TODO: To implement `from_value`, we need to the raw string so we can // decode it into bytes as opposed to a string as it won't be UTF-8. @@ -222,13 +269,13 @@ impl<'v, T: Deserialize<'v> + Send> form::FromFormField<'v> for MsgPack { // } // } -impl From for MsgPack { +impl From for MsgPack { fn from(value: T) -> Self { MsgPack(value) } } -impl Deref for MsgPack { +impl Deref for MsgPack { type Target = T; #[inline(always)] @@ -237,7 +284,7 @@ impl Deref for MsgPack { } } -impl DerefMut for MsgPack { +impl DerefMut for MsgPack { #[inline(always)] fn deref_mut(&mut self) -> &mut T { &mut self.0 diff --git a/core/lib/tests/msgpack_encoding.rs b/core/lib/tests/msgpack_encoding.rs new file mode 100644 index 0000000000..575da5fa69 --- /dev/null +++ b/core/lib/tests/msgpack_encoding.rs @@ -0,0 +1,97 @@ +#![cfg(feature = "msgpack")] + +use rocket::{Rocket, Build}; +use rocket::serde::msgpack; +use rocket::local::blocking::Client; + +#[derive(serde::Serialize, serde::Deserialize, PartialEq, Eq)] +struct Person { + name: String, + age: u8, + gender: Gender, +} + +#[derive(serde::Serialize, serde::Deserialize, PartialEq, Eq)] +#[serde(tag = "gender")] +enum Gender { + Male, + Female, + NonBinary, +} + +#[rocket::post("/age_named", data = "")] +fn named(person: msgpack::MsgPack) -> msgpack::Named { + let person = Person { age: person.age + 1, ..person.into_inner() }; + msgpack::MsgPack(person) +} + +#[rocket::post("/age_compact", data = "")] +fn compact(person: msgpack::MsgPack) -> msgpack::Compact { + let person = Person { age: person.age + 1, ..person.into_inner() }; + msgpack::MsgPack(person) +} + +fn rocket() -> Rocket { + rocket::build() + .mount("/", rocket::routes![named, compact]) +} + +fn read_string(buf: &mut rmp::decode::Bytes) -> String { + let mut string_buf = vec![0; 32]; // Awful but we're just testing. + rmp::decode::read_str(buf, &mut string_buf).unwrap().to_string() +} + +#[test] +fn check_named_roundtrip() { + let client = Client::debug(rocket()).unwrap(); + let person = Person { + name: "Cal".to_string(), + age: 17, + gender: Gender::NonBinary, + }; + let response = client + .post("/age_named") + .body(rmp_serde::to_vec_named(&person).unwrap()) + .dispatch() + .into_bytes() + .unwrap(); + let mut bytes = rmp::decode::Bytes::new(&response); + assert_eq!(rmp::decode::read_map_len(&mut bytes).unwrap(), 3); + assert_eq!(&read_string(&mut bytes), "name"); + assert_eq!(&read_string(&mut bytes), "Cal"); + assert_eq!(&read_string(&mut bytes), "age"); + assert_eq!(rmp::decode::read_int::(&mut bytes).unwrap(), 18); + assert_eq!(&read_string(&mut bytes), "gender"); + // Enums are complicated in serde. In this test, they're encoded like this: + // (JSON equivalent) `{ "gender": "NonBinary" }`, where that object is itself + // the value of the `gender` key in the outer object. `#[serde(flatten)]` + // on the `gender` key in the outer object fixes this, but it prevents `rmp` + // from using compact mode, which would break the test. + assert_eq!(rmp::decode::read_map_len(&mut bytes).unwrap(), 1); + assert_eq!(&read_string(&mut bytes), "gender"); + assert_eq!(&read_string(&mut bytes), "NonBinary"); +} + +#[test] +fn check_compact_roundtrip() { + let client = Client::debug(rocket()).unwrap(); + let person = Person { + name: "Maeve".to_string(), + age: 15, + gender: Gender::Female, + }; + let response = client + .post("/age_compact") + .body(rmp_serde::to_vec(&person).unwrap()) + .dispatch() + .into_bytes() + .unwrap(); + let mut bytes = rmp::decode::Bytes::new(&response); + assert_eq!(rmp::decode::read_array_len(&mut bytes).unwrap(), 3); + assert_eq!(&read_string(&mut bytes), "Maeve"); + assert_eq!(rmp::decode::read_int::(&mut bytes).unwrap(), 16); + // Equivalent to the named representation, gender here is encoded like this: + // `[ "Female" ]`. + assert_eq!(rmp::decode::read_array_len(&mut bytes).unwrap(), 1); + assert_eq!(&read_string(&mut bytes), "Female"); +}