Fix url encoding query and implement FromQueryArgument for Option<T> (#4213)

* Fix url encoding queries

* handle percent encoding hash fragments

* implement fromqueryargument for Option<T>

* fix tests

* fix typo

Cargo.lock

@@ -3675,8 +3675,8 @@ dependencies = [
  "ndk",
  "ndk-context",
  "ndk-sys 0.6.0+11769913",
+ "percent-encoding",
  "thiserror 2.0.12",
- "urlencoding",
 ]
 
 [[package]]
@@ -3985,6 +3985,7 @@ dependencies = [
  "ndk-sys 0.6.0+11769913",
  "objc",
  "objc_id",
+ "percent-encoding",
  "rand 0.8.5",
  "reqwest 0.12.15",
  "rfd",
@@ -4000,7 +4001,6 @@ dependencies = [
  "tokio-tungstenite",
  "tracing",
  "tray-icon",
- "urlencoding",
  "webbrowser",
  "wry",
 ]
@@ -4447,12 +4447,12 @@ dependencies = [
  "dioxus-router",
  "dioxus-router-macro",
  "dioxus-ssr",
+ "percent-encoding",
  "rustversion",
  "serde",
  "tokio",
  "tracing",
  "url",
- "urlencoding",
 ]
 
 [[package]]

Cargo.toml

@@ -316,7 +316,7 @@ tao = { version = "0.33.0", features = ["rwh_05"] }
 webbrowser = "1.0.3"
 infer = "0.19.0"
 dunce = "1.0.5"
-urlencoding = "2.1.3"
+percent-encoding = "2.3.1"
 global-hotkey = "0.7.0"
 rfd = { version = "0.15.2", default-features = false }
 muda = "0.16.1"

packages/asset-resolver/Cargo.toml

@@ -12,7 +12,7 @@ rust-version = "1.79.0"
 
 [dependencies]
 http = { workspace = true }
-urlencoding = { workspace = true }
+percent-encoding = { workspace = true }
 infer = { workspace = true }
 thiserror = { workspace = true }
 dioxus-cli-config = { workspace = true }

packages/asset-resolver/src/lib.rs

@@ -18,7 +18,8 @@ pub fn serve_asset_from_raw_path(path: &str) -> Result<Response<Vec<u8>>, AssetS
     // If the user provided a custom asset handler, then call it and return the response if the request was handled.
     // The path is the first part of the URI, so we need to trim the leading slash.
     let mut uri_path = PathBuf::from(
-        urlencoding::decode(path)
+        percent_encoding::percent_decode_str(path)
+            .decode_utf8()
             .expect("expected URL to be UTF-8 encoded")
             .as_ref(),
     );

packages/desktop/Cargo.toml

@@ -44,7 +44,7 @@ slab = { workspace = true }
 rustc-hash = { workspace = true }
 dioxus-hooks = { workspace = true }
 futures-util = { workspace = true }
-urlencoding = { workspace = true }
+percent-encoding = { workspace = true }
 async-trait = { workspace = true }
 tao = { workspace = true, features = ["rwh_05"] }
 dioxus-history = { workspace = true }

packages/router-macro/src/hash.rs

@@ -28,7 +28,7 @@ impl HashFragment {
             {
                 let __hash = #ident.to_string();
                 if !__hash.is_empty() {
-                    write!(f, "#{}", __hash)?;
+                    write!(f, "#{}", dioxus_router::exports::percent_encoding::utf8_percent_encode(&__hash, dioxus_router::exports::FRAGMENT_ASCII_SET))?;
                 }
             }
         }

packages/router-macro/src/lib.rs

@@ -531,9 +531,17 @@ impl RouteEnum {
                     // Remove any trailing slashes. We parse /route/ and /route in the same way
                     // Note: we don't use trim because it includes more code
                     let route = route.strip_suffix('/').unwrap_or(route);
-                    let query = dioxus_router::exports::urlencoding::decode(query).unwrap_or(query.into());
-                    let hash = dioxus_router::exports::urlencoding::decode(hash).unwrap_or(hash.into());
-                    let mut segments = route.split('/').map(|s| dioxus_router::exports::urlencoding::decode(s).unwrap_or(s.into()));
+                    let query = dioxus_router::exports::percent_encoding::percent_decode_str(query)
+                        .decode_utf8()
+                        .unwrap_or(query.into());
+                    let hash = dioxus_router::exports::percent_encoding::percent_decode_str(hash)
+                        .decode_utf8()
+                        .unwrap_or(hash.into());
+                    let mut segments = route.split('/').map(|s| {
+                        dioxus_router::exports::percent_encoding::percent_decode_str(s)
+                            .decode_utf8()
+                            .unwrap_or(s.into())
+                    });
                     // skip the first empty segment
                     if s.starts_with('/') {
                         let _ = segments.next();

packages/router-macro/src/query.rs

@@ -39,13 +39,8 @@ impl QuerySegment {
             QuerySegment::Segments(segments) => {
                 let mut tokens = TokenStream2::new();
                 tokens.extend(quote! { write!(f, "?")?; });
-                let mut segments_iter = segments.iter();
-                if let Some(first_segment) = segments_iter.next() {
-                    tokens.extend(first_segment.write());
-                }
-                for segment in segments_iter {
-                    tokens.extend(quote! { write!(f, "&")?; });
-                    tokens.extend(segment.write());
+                for (i, segment) in segments.iter().enumerate() {
+                    tokens.extend(segment.write(i == segments.len() - 1));
                 }
                 tokens
             }
@@ -133,7 +128,7 @@ impl FullQuerySegment {
         quote! {
             {
                 let as_string = #ident.to_string();
-                write!(f, "?{}", dioxus_router::exports::urlencoding::encode(&as_string))?;
+                write!(f, "?{}", dioxus_router::exports::percent_encoding::utf8_percent_encode(&as_string, dioxus_router::exports::QUERY_ASCII_SET))?;
             }
         }
     }
@@ -151,18 +146,27 @@ impl QueryArgument {
         let ty = &self.ty;
         quote! {
             let #ident = match split_query.get(stringify!(#ident)) {
-                Some(query_argument) => <#ty as dioxus_router::routable::FromQueryArgument>::from_query_argument(query_argument).unwrap_or_default(),
+                Some(query_argument) => {
+                    use dioxus_router::routable::FromQueryArgument;
+                    <#ty>::from_query_argument(query_argument).unwrap_or_default()
+                },
                 None => <#ty as Default>::default(),
             };
         }
     }
 
-    pub fn write(&self) -> TokenStream2 {
+    pub fn write(&self, trailing: bool) -> TokenStream2 {
         let ident = &self.ident;
+        let write_ampersand = if !trailing {
+            quote! { if !as_string.is_empty() { write!(f, "&")?; } }
+        } else {
+            quote! {}
+        };
         quote! {
             {
-                let as_string = #ident.to_string();
-                write!(f, "{}={}", stringify!(#ident), dioxus_router::exports::urlencoding::encode(&as_string))?;
+                let as_string = dioxus_router::routable::DisplayQueryArgument::new(stringify!(#ident), #ident).to_string();
+                write!(f, "{}", dioxus_router::exports::percent_encoding::utf8_percent_encode(&as_string, dioxus_router::exports::QUERY_ASCII_SET))?;
+                #write_ampersand
             }
         }
     }

packages/router-macro/src/segment.rs

@@ -27,7 +27,7 @@ impl RouteSegment {
             Self::Dynamic(ident, _) => quote! {
                 {
                     let as_string = #ident.to_string();
-                    write!(f, "/{}", dioxus_router::exports::urlencoding::encode(&as_string))?;
+                    write!(f, "/{}", dioxus_router::exports::percent_encoding::utf8_percent_encode(&as_string, dioxus_router::exports::PATH_ASCII_SET))?;
                 }
             },
             Self::CatchAll(ident, _) => quote! { #ident.display_route_segments(f)?; },

packages/router/Cargo.toml

@@ -15,7 +15,7 @@ dioxus-history = { workspace = true }
 dioxus-router-macro = { workspace = true }
 dioxus-fullstack-hooks = { workspace = true, optional = true }
 tracing = { workspace = true }
-urlencoding = { workspace = true }
+percent-encoding = { workspace = true }
 url = { workspace = true }
 dioxus-cli-config = { workspace = true }
 rustversion = { workspace = true }

packages/router/src/lib.rs

@@ -89,5 +89,36 @@ mod utils {
 
 #[doc(hidden)]
 pub mod exports {
-    pub use urlencoding;
+    pub use crate::query_sets::*;
+    pub use percent_encoding;
+}
+
+pub(crate) mod query_sets {
+    //! Url percent encode sets defined [here](https://url.spec.whatwg.org/#percent-encoded-bytes)
+
+    use percent_encoding::AsciiSet;
+
+    /// The ASCII set that must be escaped in query strings.
+    pub const QUERY_ASCII_SET: &AsciiSet = &percent_encoding::CONTROLS
+        .add(b' ')
+        .add(b'"')
+        .add(b'#')
+        .add(b'<')
+        .add(b'>');
+
+    /// The ASCII set that must be escaped in path segments.
+    pub const PATH_ASCII_SET: &AsciiSet = &QUERY_ASCII_SET
+        .add(b'?')
+        .add(b'^')
+        .add(b'`')
+        .add(b'{')
+        .add(b'}');
+
+    /// The ASCII set that must be escaped in hash fragments.
+    pub const FRAGMENT_ASCII_SET: &AsciiSet = &percent_encoding::CONTROLS
+        .add(b' ')
+        .add(b'"')
+        .add(b'<')
+        .add(b'>')
+        .add(b'`');
 }

packages/router/src/routable.rs

@@ -123,7 +123,7 @@ impl<T: for<'a> From<&'a str>> FromQuery for T {
 ///     }
 /// }
 ///
-/// // We also need to implement Display for CustomQuery which will be used to format the query string into the URL
+/// // We also need to implement Display for CustomQuery so that ToQueryArgument is implemented automatically
 /// impl std::fmt::Display for CustomQuery {
 ///     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
 ///         write!(f, "{}", self.count)
@@ -143,7 +143,7 @@ impl<T: for<'a> From<&'a str>> FromQuery for T {
         note = "FromQueryArgument is automatically implemented for types that implement `FromStr` and `Default`. You need to either implement FromStr and Default or implement FromQueryArgument manually."
     )
 )]
-pub trait FromQueryArgument: Default {
+pub trait FromQueryArgument<P = ()>: Default {
     /// The error that can occur when parsing a query argument.
     type Err;
 
@@ -168,6 +168,138 @@ where
     }
 }
 
+/// A marker type for `Option<T>` to implement `FromQueryArgument`.
+pub struct OptionMarker;
+
+impl<T: Default + FromStr> FromQueryArgument<OptionMarker> for Option<T>
+where
+    <T as FromStr>::Err: Display,
+{
+    type Err = <T as FromStr>::Err;
+
+    fn from_query_argument(argument: &str) -> Result<Self, Self::Err> {
+        match T::from_str(argument) {
+            Ok(result) => Ok(Some(result)),
+            Err(err) => {
+                tracing::error!("Failed to parse query argument: {}", err);
+                Err(err)
+            }
+        }
+    }
+}
+
+/// Something that can be formatted as a query argument. This trait must be implemented for any type that is used as a query argument like `#[route("/?:query")]`.
+///
+/// **This trait is automatically implemented for any types that implement [`Display`].**
+///
+/// ```rust
+/// use dioxus::prelude::*;
+///
+/// #[derive(Routable, Clone, PartialEq, Debug)]
+/// enum Route {
+///     // FromQuerySegment must be implemented for any types you use in the query segment
+///     // When you don't spread the query, you can parse multiple values form the query
+///     // This url will be in the format `/?query=123&other=456`
+///     #[route("/?:query&:other")]
+///     Home {
+///         query: CustomQuery,
+///         other: i32,
+///     },
+/// }
+///
+/// // We can derive Default for CustomQuery
+/// // If the router fails to parse the query value, it will use the default value instead
+/// #[derive(Default, Clone, PartialEq, Debug)]
+/// struct CustomQuery {
+///     count: i32,
+/// }
+///
+/// // We implement FromStr for CustomQuery so that FromQuerySegment is implemented automatically
+/// impl std::str::FromStr for CustomQuery {
+///     type Err = <i32 as std::str::FromStr>::Err;
+///
+///     fn from_str(query: &str) -> Result<Self, Self::Err> {
+///         Ok(CustomQuery {
+///             count: query.parse()?,
+///         })
+///     }
+/// }
+///
+/// // We also need to implement Display for CustomQuery so that ToQueryArgument is implemented automatically
+/// impl std::fmt::Display for CustomQuery {
+///     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+///         write!(f, "{}", self.count)
+///     }
+/// }
+///
+/// # #[component]
+/// # fn Home(query: CustomQuery, other: i32) -> Element {
+/// #     unimplemented!()
+/// # }
+/// ```
+pub trait ToQueryArgument<T = ()> {
+    /// Display the query argument as a string.
+    fn display_query_argument(
+        &self,
+        query_name: &str,
+        f: &mut std::fmt::Formatter<'_>,
+    ) -> std::fmt::Result;
+}
+
+impl<T> ToQueryArgument for T
+where
+    T: Display,
+{
+    fn display_query_argument(
+        &self,
+        query_name: &str,
+        f: &mut std::fmt::Formatter<'_>,
+    ) -> std::fmt::Result {
+        write!(f, "{}={}", query_name, self)
+    }
+}
+
+impl<T: Display> ToQueryArgument<OptionMarker> for Option<T> {
+    fn display_query_argument(
+        &self,
+        query_name: &str,
+        f: &mut std::fmt::Formatter<'_>,
+    ) -> std::fmt::Result {
+        if let Some(value) = self {
+            write!(f, "{}={}", query_name, value)
+        } else {
+            Ok(())
+        }
+    }
+}
+
+/// A type that implements [`ToQueryArgument`] along with the query name. This type implements Display and can be used to format the query argument into a string.
+pub struct DisplayQueryArgument<'a, T, M = ()> {
+    /// The query name.
+    query_name: &'a str,
+    /// The value to format.
+    value: &'a T,
+    /// The `ToQueryArgument` marker type, which can be used to differentiate between different types of query arguments.
+    _marker: std::marker::PhantomData<M>,
+}
+
+impl<'a, T, M> DisplayQueryArgument<'a, T, M> {
+    /// Create a new `DisplayQueryArgument`.
+    pub fn new(query_name: &'a str, value: &'a T) -> Self {
+        Self {
+            query_name,
+            value,
+            _marker: std::marker::PhantomData,
+        }
+    }
+}
+
+impl<T: ToQueryArgument<M>, M> Display for DisplayQueryArgument<'_, T, M> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.value.display_query_argument(self.query_name, f)
+    }
+}
+
 /// Something that can be created from an entire hash fragment. This must be implemented for any type that is used as a hash fragment like `#[route("/#:hash_fragment")]`.
 ///
 ///
@@ -406,7 +538,8 @@ where
         for segment in self {
             write!(f, "/")?;
             let segment = segment.to_string();
-            let encoded = urlencoding::encode(&segment);
+            let encoded =
+                percent_encoding::utf8_percent_encode(&segment, crate::query_sets::PATH_ASCII_SET);
             write!(f, "{}", encoded)?;
         }
         Ok(())

packages/router/tests/parsing.rs

@@ -1,5 +1,8 @@
 use dioxus::prelude::*;
-use std::str::FromStr;
+use std::{
+    fmt::{self, Display},
+    str::FromStr,
+};
 
 #[component]
 fn Root() -> Element {
@@ -75,3 +78,87 @@ fn without_trailing_slashes_parse() {
         RouteWithoutTrailingSlash::Dynamic { id: 123 }
     );
 }
+
+// Regression test for https://github.com/DioxusLabs/dioxus/issues/2984
+#[test]
+fn query_segments_parse() {
+    #[derive(Debug, Clone, PartialEq)]
+    enum Query {
+        Id(u64),
+    }
+
+    impl From<&str> for Query {
+        fn from(_: &str) -> Self {
+            // e.g. split query on `&` and split pairs on `=`
+            Query::Id(10)
+        }
+    }
+
+    impl Display for Query {
+        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+            write!(f, "id=10")
+        }
+    }
+
+    #[component]
+    fn Index(query: Query) -> Element {
+        rsx! {
+            h1 { "Index" }
+        }
+    }
+
+    #[derive(Debug, Clone, PartialEq, Routable)]
+    enum Route {
+        #[route("/?:..query")]
+        Index { query: Query },
+    }
+
+    let route = Route::Index {
+        query: Query::Id(10),
+    };
+    assert_eq!(route.to_string(), "/?id=10");
+    let parsed_route = "/?id=10".parse::<Route>().unwrap();
+    assert_eq!(parsed_route, route);
+}
+
+#[test]
+fn optional_query_segments_parse() {
+    #[derive(Debug, Clone, PartialEq, Routable)]
+    enum Route {
+        #[route("/?:query&:other")]
+        Index { query: Option<u64>, other: u64 },
+    }
+
+    #[component]
+    fn Index(query: Option<u64>, other: u64) -> Element {
+        rsx! {
+            h1 { "Index" }
+        }
+    }
+
+    let route = Route::Index {
+        query: Some(10),
+        other: 20,
+    };
+    assert_eq!(route.to_string(), "/?query=10&other=20");
+    let parsed_route = "/?query=10&other=20".parse::<Route>().unwrap();
+    assert_eq!(parsed_route, route);
+
+    let route_without_query = Route::Index {
+        query: None,
+        other: 20,
+    };
+    assert_eq!(route_without_query.to_string(), "/?other=20");
+    let parsed_route_without_query = "/?other=20".parse::<Route>().unwrap();
+    assert_eq!(parsed_route_without_query, route_without_query);
+    let route_without_query_and_other = Route::Index {
+        query: None,
+        other: 0,
+    };
+    assert_eq!(route_without_query_and_other.to_string(), "/?other=0");
+    let parsed_route_without_query_and_other = "/".parse::<Route>().unwrap();
+    assert_eq!(
+        parsed_route_without_query_and_other,
+        route_without_query_and_other
+    );
+}