From 6a3d1ac1d56f6f025a04f81105902b0accea3a89 Mon Sep 17 00:00:00 2001
From: Sergio Benitez <sb@sergio.bz>
Date: Wed, 30 Jun 2021 06:24:00 -0700
Subject: [PATCH] Fix 'UriDisplay<Query>' 'Json', 'MsgPack', impls.

As 'FromForm' doesn't provide access to the raw, undecoded string,
'MsgPack' cannot implement 'FromForm::from_value()'. This means that it
is not presently possible to parse a MessagePack form from a query
string. As such, the 'UriDisplay<Query>' implementation was removed.

The 'UriDisplay<Query>' for JSON was fixed such that a round-trip of a
'Json<T>' as a form works as expected.
---
 core/codegen/Cargo.toml           |  2 +-
 core/codegen/tests/uri_display.rs | 51 +++++++++++++++++++++++++++++--
 core/lib/src/serde/json.rs        |  5 +--
 core/lib/src/serde/msgpack.rs     | 17 +++++++----
 4 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/core/codegen/Cargo.toml b/core/codegen/Cargo.toml
index b046102e..56523799 100644
--- a/core/codegen/Cargo.toml
+++ b/core/codegen/Cargo.toml
@@ -25,7 +25,7 @@ unicode-xid = "0.2"
 glob = "0.3"
 
 [dev-dependencies]
-rocket = { version = "0.5.0-rc.1", path = "../lib", features = ["json"] }
+rocket = { version = "0.5.0-rc.1", path = "../lib", features = ["json", "msgpack"] }
 pretty_assertions = "0.7"
 version_check = "0.9"
 trybuild = "1.0"
diff --git a/core/codegen/tests/uri_display.rs b/core/codegen/tests/uri_display.rs
index 95593935..cc031090 100644
--- a/core/codegen/tests/uri_display.rs
+++ b/core/codegen/tests/uri_display.rs
@@ -1,14 +1,28 @@
 #[macro_use] extern crate rocket;
 
 use rocket::http::uri::fmt::{UriDisplay, Query, Path};
+use rocket::serde::{Serialize, Deserialize};
 
 macro_rules! assert_uri_display_query {
-    ($v:expr, $s:expr) => (
+    ($v:expr, $expected:expr) => (
         let uri_string = format!("{}", &$v as &dyn UriDisplay<Query>);
-        assert_eq!(uri_string, $s);
+        assert_eq!(uri_string, $expected);
     )
 }
 
+macro_rules! assert_query_value_roundtrip {
+    ($T:ty, $v:expr) => ({
+        use rocket::form::{Form, Strict};
+        use rocket::http::RawStr;
+
+        let v = $v;
+        let string = format!("={}", &v as &dyn UriDisplay<Query>);
+        let raw = RawStr::new(&string);
+        let value = Form::<Strict<$T>>::parse_encoded(raw).map(|s| s.into_inner());
+        assert_eq!(value.expect("form parse"), v);
+    })
+}
+
 #[derive(UriDisplayQuery, Clone)]
 enum Foo<'r> {
     First(&'r str),
@@ -194,3 +208,36 @@ fn uri_display_path() {
     assert_uri_display_path!(BamP(BazP(&100)), "100");
     assert_uri_display_path!(BopP(FooP("bop foo")), "bop%20foo");
 }
+
+#[test]
+fn uri_display_serde() {
+    use rocket::serde::json::Json;
+
+    #[derive(Debug, PartialEq, Clone, UriDisplayQuery, Deserialize, Serialize)]
+    #[serde(crate = "rocket::serde")]
+    struct Bam {
+        foo: String,
+        bar: Option<usize>,
+        baz: Result<String, usize>,
+    }
+
+    #[derive(Debug, PartialEq, FromForm, UriDisplayQuery)]
+    struct JsonFoo(Json<Bam>);
+
+    let bam = Bam {
+        foo: "hi[]=there.baz !?".into(),
+        bar: None,
+        baz: Ok("what is baz, anyway?".into()),
+    };
+
+    assert_query_value_roundtrip!(JsonFoo, JsonFoo(Json(bam.clone())));
+
+    // TODO: This requires `MsgPack` to parse from value form fields.
+    //
+    // use rocket::serde::msgpack::MsgPack;
+    //
+    // #[derive(Debug, PartialEq, FromForm, UriDisplayQuery)]
+    // struct MsgPackFoo(MsgPack<Bam>);
+    //
+    // assert_query_value_roundtrip!(MsgPackFoo, MsgPackFoo(MsgPack(bam)));
+}
diff --git a/core/lib/src/serde/json.rs b/core/lib/src/serde/json.rs
index 451b85b8..006edddc 100644
--- a/core/lib/src/serde/json.rs
+++ b/core/lib/src/serde/json.rs
@@ -208,9 +208,10 @@ impl<'r, T: Serialize> Responder<'r, 'static> for Json<T> {
     }
 }
 
-impl<T: fmt::UriDisplay<fmt::Query>> fmt::UriDisplay<fmt::Query> for Json<T> {
+impl<T: Serialize> fmt::UriDisplay<fmt::Query> for Json<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_, fmt::Query>) -> std::fmt::Result {
-        self.0.fmt(f)
+        let string = to_string(&self.0).map_err(|_| std::fmt::Error)?;
+        f.write_value(&string)
     }
 }
 
diff --git a/core/lib/src/serde/msgpack.rs b/core/lib/src/serde/msgpack.rs
index 57047969..48d727bb 100644
--- a/core/lib/src/serde/msgpack.rs
+++ b/core/lib/src/serde/msgpack.rs
@@ -32,7 +32,7 @@ use crate::data::{Limits, Data, FromData, Outcome};
 use crate::response::{self, Responder, content};
 use crate::http::Status;
 use crate::form::prelude as form;
-use crate::http::uri::fmt;
+// use crate::http::uri::fmt;
 
 use serde::{Serialize, Deserialize};
 
@@ -200,6 +200,9 @@ impl<'r, T: Serialize> Responder<'r, 'static> for MsgPack<T> {
 
 #[crate::async_trait]
 impl<'v, T: Deserialize<'v> + Send> form::FromFormField<'v> for MsgPack<T> {
+    // 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.
+
     async fn from_data(f: form::DataField<'v, '_>) -> Result<Self, form::Errors<'v>> {
         Self::from_data(f.request, f.data).await.map_err(|e| {
             match e {
@@ -211,11 +214,13 @@ impl<'v, T: Deserialize<'v> + Send> form::FromFormField<'v> for MsgPack<T> {
     }
 }
 
-impl<T: fmt::UriDisplay<fmt::Query>> fmt::UriDisplay<fmt::Query> for MsgPack<T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_, fmt::Query>) -> std::fmt::Result {
-        self.0.fmt(f)
-    }
-}
+// impl<T: Serialize> fmt::UriDisplay<fmt::Query> for MsgPack<T> {
+//     fn fmt(&self, f: &mut fmt::Formatter<'_, fmt::Query>) -> std::fmt::Result {
+//         let bytes = to_vec(&self.0).map_err(|_| std::fmt::Error)?;
+//         let encoded = crate::http::RawStr::percent_encode_bytes(&bytes);
+//         f.write_value(encoded.as_str())
+//     }
+// }
 
 impl<T> From<T> for MsgPack<T> {
     fn from(value: T) -> Self {