From 96b518cafaeb5fbf01e7649a6880a515bd830152 Mon Sep 17 00:00:00 2001 From: Connor Slade Date: Tue, 19 Dec 2023 18:27:16 -0500 Subject: [PATCH] Cleanup Query struct Fixes #54 --- Changelog.md | 8 +- .../old-examples/application_quote_book.rs | 2 +- examples/old-examples/basic/data.rs | 2 +- examples/paste_bin/src/routes/post_paste.rs | 2 +- lib/extensions/logger.rs | 2 +- lib/internal/handle.rs | 13 +- lib/proto/http/query.rs | 126 +++++++++++------- lib/request.rs | 2 +- 8 files changed, 92 insertions(+), 65 deletions(-) diff --git a/Changelog.md b/Changelog.md index 45632fb..3c46bd5 100644 --- a/Changelog.md +++ b/Changelog.md @@ -70,7 +70,11 @@ Coming Soon - Add a Stream trait to allow using different socket impls - Allow using custom event loop - The custom event loop with the Stream trait should allow a separate crate to add tls support to an afire server - +- Cleanup Query struct + - Now stores a set of QueryParameter structs instead of [String; 2] + - Implement Debug for Query + - Rename `Query::from_body` to `Query::from_str` +- Made internal::handle::handle function public for use in custom event loops. # 2.2.1 @@ -211,7 +215,7 @@ Apr 10, 2022 - Add SocketHandler struct to hold socket interacting functions - Fix Path Traversal Exploit O_O -# 1.0.0! +# 1.0.0 Mar 14, 2022 diff --git a/examples/old-examples/application_quote_book.rs b/examples/old-examples/application_quote_book.rs index a1076f8..3c00449 100644 --- a/examples/old-examples/application_quote_book.rs +++ b/examples/old-examples/application_quote_book.rs @@ -48,7 +48,7 @@ fn main() { // Route to handle creating new quotes. // After successful creation the user will be redirected to the new quotes page. server.stateful_route(Method::POST, "/api/new", |app, req| { - let form = Query::from_body(&String::from_utf8_lossy(&req.body)); + let form = Query::from_str(&String::from_utf8_lossy(&req.body)); let name = url::decode(form.get("author").expect("No author supplied")).expect("Invalid author"); let body = diff --git a/examples/old-examples/basic/data.rs b/examples/old-examples/basic/data.rs index 5a1a23c..13b5d0d 100644 --- a/examples/old-examples/basic/data.rs +++ b/examples/old-examples/basic/data.rs @@ -38,7 +38,7 @@ impl Example for Data { // The body of requests is not part of the req.query // Instead it is part of the req.body but as a string // We will need to parse it get it as a query - let body_data = Query::from_body(&String::from_utf8_lossy(&req.body)); + let body_data = Query::from_str(&String::from_utf8_lossy(&req.body)); let name = body_data.get("name").unwrap_or("Nobody"); let text = format!("

Hello, {}

", name); diff --git a/examples/paste_bin/src/routes/post_paste.rs b/examples/paste_bin/src/routes/post_paste.rs index 755dde0..a85027b 100644 --- a/examples/paste_bin/src/routes/post_paste.rs +++ b/examples/paste_bin/src/routes/post_paste.rs @@ -64,7 +64,7 @@ fn post_api(ctx: &Context) -> Result<(), Box> { fn post_form(ctx: &Context) -> Result<(), Box> { // Parse the query from the request body - let query = Query::from_body(&ctx.req.body_str()); + let query = Query::from_str(&ctx.req.body_str()); // Pull out the name and paste from the query // (automatically url-decoded) let name = query.get("title").context("Missing name")?; diff --git a/lib/extensions/logger.rs b/lib/extensions/logger.rs index 755c363..5072692 100644 --- a/lib/extensions/logger.rs +++ b/lib/extensions/logger.rs @@ -151,7 +151,7 @@ impl Logger { // Format Query as string let mut query = "".to_string(); for i in req.query.iter() { - query += &format!("{}: {}, ", i[0], i[1]); + query += &format!("{}: {}, ", i.key, i.value); } if query.len() >= 2 { query = query[0..query.len() - 2].to_string() diff --git a/lib/internal/handle.rs b/lib/internal/handle.rs index b4eea2f..b7f08b3 100644 --- a/lib/internal/handle.rs +++ b/lib/internal/handle.rs @@ -1,9 +1,4 @@ -use std::{ - cell::RefCell, - io::Read, - net::{Shutdown, TcpStream}, - sync::Arc, -}; +use std::{cell::RefCell, io::Read, net::Shutdown, sync::Arc}; use crate::{ context::ContextFlag, @@ -23,11 +18,11 @@ pub(crate) type Writeable = Box>; /// Handles a socket. /// /// -pub(crate) fn handle(stream: Arc, this: Arc>) +pub fn handle(stream: Arc, this: Arc>) where State: 'static + Send + Sync, { - let mut socket = stream.force_lock(); + let socket = stream.force_lock(); trace!( Level::Debug, "Opening socket {:?}", @@ -35,7 +30,7 @@ where ); socket.set_timeout(this.socket_timeout).unwrap(); drop(socket); - + 'outer: loop { let mut keep_alive = false; let mut req = Request::from_socket(stream.clone()); diff --git a/lib/proto/http/query.rs b/lib/proto/http/query.rs index 82410d6..52e6b9b 100644 --- a/lib/proto/http/query.rs +++ b/lib/proto/http/query.rs @@ -10,21 +10,19 @@ use crate::internal::encoding::url; /// Collection of query parameters. /// Can be made from the query string of a URL, or the body of a POST request. /// Similar to [`crate::header::Headers`]. -#[derive(Debug, Hash, PartialEq, Eq, Clone)] -pub struct Query(Vec<[String; 2]>); - -impl Deref for Query { - type Target = Vec<[String; 2]>; - - fn deref(&self) -> &Self::Target { - &self.0 - } +#[derive(Hash, PartialEq, Eq, Clone)] +pub struct Query { + inner: Vec, } -impl DerefMut for Query { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } +/// An individual query parameter. +/// Key and value are both automatically url decoded. +#[derive(Debug, Hash, PartialEq, Eq, Clone)] +pub struct QueryParameter { + /// The key of the query parameter. + pub key: String, + /// The value of the query parameter. + pub value: String, } impl Query { @@ -33,15 +31,15 @@ impl Query { /// ```rust /// # use afire::Query; /// # use std::str::FromStr; - /// # let query = Query::from_body("foo=bar&nose=dog"); + /// # let query = Query::from_str("foo=bar&nose=dog"); /// # assert!(query.has("foo")); /// if query.has("foo") { /// println!("foo exists"); /// } /// ``` pub fn has(&self, key: impl AsRef) -> bool { - let key = key.as_ref().to_owned(); - self.iter().any(|i| *i[0] == key) + let key = key.as_ref(); + self.inner.iter().any(|i| i.key == key) } /// Adds a new key-value pair to the collection with the specified key and value. @@ -54,7 +52,10 @@ impl Query { /// query.add("foo", "bar"); /// # } pub fn add(&mut self, key: impl Into, value: impl Into) { - self.push([key.into(), value.into()]); + self.inner.push(QueryParameter { + key: key.into(), + value: value.into(), + }); } /// Get a value from a key. @@ -63,62 +64,67 @@ impl Query { /// ``` /// # use afire::Query; /// # use std::str::FromStr; - /// let query = Query::from_body("foo=bar&nose=dog"); + /// let query = Query::from_str("foo=bar&nose=dog"); /// assert_eq!(query.get("foo"), Some("bar")); /// ``` pub fn get(&self, key: impl AsRef) -> Option<&str> { - let key = key.as_ref().to_owned(); - self.iter() - .find(|i| *i[0] == key) - .map(|i| &i[1]) - .map(|x| x.as_str()) + let key = key.as_ref(); + self.inner + .iter() + .find(|i| i.key == key) + .map(|i| i.value.as_ref()) } /// Gets a value of the specified key as a mutable reference. /// This will return None if the key does not exist. /// See [`Query::get`] for the non-mutable version. pub fn get_mut(&mut self, key: impl AsRef) -> Option<&mut String> { - let key = key.as_ref().to_owned(); - self.iter_mut().find(|i| *i[0] == key).map(|i| &mut i[1]) + let key = key.as_ref(); + self.inner + .iter_mut() + .find(|i| i.key == key) + .map(|i| &mut i.value) } - /// Adds a new key-value pair to the collection from a `[String; 2]`. - /// See [`Query::add`] for adding to the collection with a key and value. + /// Adds a new parameter to the collection with a QueryParameter struct. + /// See [`Query::add`] for adding a key-value pair with string keys and values. /// ## Example /// ```rust - /// # use afire::Query; + /// # use afire::proto::http::query::{QueryParameter, Query}; /// # fn test(query: &mut Query) { - /// query.add_query(["foo".to_owned(), "bar".to_owned()]); + /// query.add_query(QueryParameter { + /// key: "foo".into(), + /// value: "bar".into(), + /// }); /// # } - pub fn add_query(&mut self, query: [String; 2]) { - self.push(query); + pub fn add_query(&mut self, query: QueryParameter) { + self.inner.push(query); } /// Gets the key-value pair of the specified key. /// If the key does not exist, this will return None. - pub fn get_query(&self, key: impl AsRef) -> Option<&[String; 2]> { - let key = key.as_ref().to_owned(); - self.iter().find(|i| *i[0] == key) + pub fn get_query(&self, key: impl AsRef) -> Option<&QueryParameter> { + let key = key.as_ref(); + self.inner.iter().find(|i| i.key == key) } /// Get the key-value pair of the specified key as a mutable reference. /// If the key does not exist, this will return None. - pub fn get_query_mut(&mut self, key: impl AsRef) -> Option<&mut [String; 2]> { - let key = key.as_ref().to_owned(); - self.iter_mut().find(|i| *i[0] == key) + pub fn get_query_mut(&mut self, key: impl AsRef) -> Option<&mut QueryParameter> { + let key = key.as_ref(); + self.inner.iter_mut().find(|i| i.key == key) } /// Create a new Query from a Form POST body /// ## Example /// ``` /// # use afire::Query; - /// let query = Query::from_body("foo=bar&nose=dog"); + /// let query = Query::from_str("foo=bar&nose=dog"); /// ``` - pub fn from_body(body: &str) -> Self { + pub fn from_str(body: &str) -> Self { let mut data = Vec::new(); - let parts = body.split('&'); - for i in parts { + for i in body.split('&') { let mut sub = i.splitn(2, '='); let Some(key) = sub.next().map(url::decode) else { @@ -129,14 +135,30 @@ impl Query { continue; }; - data.push([key, value]) + data.push(QueryParameter { + key: key.into(), + value: value.into(), + }); } - Query(data) + Query { inner: data } + } +} + +impl Deref for Query { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl DerefMut for Query { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner } } -// Implement fmt::Display for Query impl fmt::Display for Query { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if self.is_empty() { @@ -144,21 +166,27 @@ impl fmt::Display for Query { } let mut output = String::from("?"); - for i in &self.0 { - output.push_str(&format!("{}={}&", i[0], i[1])); + for i in &self.inner { + output.push_str(&format!("{}={}&", i.key, i.value)); } f.write_str(&output[..output.len() - 1]) } } +impl fmt::Debug for Query { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("Query").field("inner", &self.inner).finish() + } +} + #[cfg(test)] mod test { use super::Query; #[test] fn test_from_str() { - let query = Query::from_body("foo=bar&nose=dog"); + let query = Query::from_str("foo=bar&nose=dog"); assert_eq!(query.get("foo"), Some("bar")); assert_eq!(query.get("nose"), Some("dog")); assert_eq!(query.get("bar"), None); @@ -166,7 +194,7 @@ mod test { #[test] fn test_get() { - let query = Query::from_body("foo=bar&nose=dog"); + let query = Query::from_str("foo=bar&nose=dog"); assert_eq!(query.get("foo"), Some("bar")); assert_eq!(query.get("nose"), Some("dog")); assert_eq!(query.get("bar"), None); @@ -174,7 +202,7 @@ mod test { #[test] fn test_get_mut() { - let mut query = Query::from_body("foo=bar&nose=dog"); + let mut query = Query::from_str("foo=bar&nose=dog"); assert_eq!(query.get_mut("bar"), None); query.get_mut("foo").unwrap().push_str("bar"); assert_eq!(query.get("foo"), Some("barbar")); diff --git a/lib/request.rs b/lib/request.rs index d1ea6a8..790aa74 100644 --- a/lib/request.rs +++ b/lib/request.rs @@ -188,7 +188,7 @@ pub(crate) fn parse_request_line(bytes: &[u8]) -> Result<(Method, String, Query, } } - let query = Query::from_body(&final_query); + let query = Query::from_str(&final_query); let version = parts .next() .ok_or(Error::Parse(ParseError::NoVersion))?