From ca76b67e985583ebc4276d6dce9dc74fde3af3bc Mon Sep 17 00:00:00 2001 From: Vika Date: Tue, 3 Dec 2024 08:29:38 +0300 Subject: kittybox-util: bump to 0.3.0 Changed micropub::Error's description to Option> to allow for that sweet sweet memory savings from not having to heap-allocate strings for static errors. Change-Id: Ic82e5ad5cacea766ea0a7e8677ce6a7f16ae8668 --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/media/mod.rs | 32 ++++----- src/micropub/mod.rs | 173 +++++++++++++++++++++++------------------------ templates-neo/Cargo.toml | 2 +- templates/Cargo.toml | 2 +- util/Cargo.toml | 2 +- util/src/micropub.rs | 33 ++++++--- 8 files changed, 131 insertions(+), 117 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9f8ac2..f45a2bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1860,7 +1860,7 @@ dependencies = [ [[package]] name = "kittybox-util" -version = "0.2.0" +version = "0.3.0" dependencies = [ "axum-core", "futures-util", diff --git a/Cargo.toml b/Cargo.toml index 001f2b1..fd6311f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,7 @@ required-features = ["sqlparser"] members = [".", "./util", "./templates", "./indieauth", "./templates-neo", "./tower-watchdog"] default-members = [".", "./util", "./templates", "./indieauth"] [dependencies.kittybox-util] -version = "0.2.0" +version = "0.3.0" path = "./util" features = ["fs", "axum"] [dependencies.kittybox-frontend-renderer] diff --git a/src/media/mod.rs b/src/media/mod.rs index 32b8405..3ed6810 100644 --- a/src/media/mod.rs +++ b/src/media/mod.rs @@ -13,10 +13,10 @@ pub use storage::file::FileStore; impl From for MicropubError { fn from(err: MediaStoreError) -> Self { - Self { - error: ErrorType::InternalServerError, - error_description: format!("{}", err) - } + Self::new( + ErrorType::InternalServerError, + format!("media store error: {}", err) + ) } } @@ -27,25 +27,25 @@ pub(crate) async fn upload( mut upload: Multipart ) -> Response { if !user.check_scope(&Scope::Media) { - return MicropubError { - error: ErrorType::NotAuthorized, - error_description: "Interacting with the media storage requires the \"media\" scope.".to_owned() - }.into_response(); + return MicropubError::from_static( + ErrorType::NotAuthorized, + "Interacting with the media storage requires the \"media\" scope." + ).into_response(); } let host = user.me.authority(); let field = match upload.next_field().await { Ok(Some(field)) => field, Ok(None) => { - return MicropubError { - error: ErrorType::InvalidRequest, - error_description: "Send multipart/form-data with one field named file".to_owned() - }.into_response(); + return MicropubError::from_static( + ErrorType::InvalidRequest, + "Send multipart/form-data with one field named file" + ).into_response(); }, Err(err) => { - return MicropubError { - error: ErrorType::InternalServerError, - error_description: format!("Error while parsing multipart/form-data: {}", err) - }.into_response(); + return MicropubError::new( + ErrorType::InternalServerError, + format!("Error while parsing multipart/form-data: {}", err) + ).into_response(); }, }; let metadata: Metadata = (&field).into(); diff --git a/src/micropub/mod.rs b/src/micropub/mod.rs index 9838e5a..663702f 100644 --- a/src/micropub/mod.rs +++ b/src/micropub/mod.rs @@ -28,13 +28,13 @@ pub struct MicropubQuery { impl From for MicropubError { fn from(err: StorageError) -> Self { - Self { - error: match err.kind() { + Self::new( + match err.kind() { crate::database::ErrorKind::NotFound => ErrorKind::NotFound, _ => ErrorKind::InternalServerError, }, - error_description: format!("Backend error: {}", err), - } + format!("backend error: {}", err) + ) } } @@ -246,11 +246,10 @@ pub(crate) async fn _post( // Security check! Do we have an OAuth2 scope to proceed? if !user.check_scope(&Scope::Create) { - return Err(MicropubError { - error: ErrorKind::InvalidScope, - error_description: "Not enough privileges - try acquiring the \"create\" scope." - .to_owned(), - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidScope, + "Not enough privileges - try acquiring the \"create\" scope." + )); } // Security check #2! Are we posting to our own website? @@ -261,18 +260,18 @@ pub(crate) async fn _post( .iter() .any(|url| !url.as_str().unwrap().starts_with(user.me.as_str())) { - return Err(MicropubError { - error: ErrorKind::Forbidden, - error_description: "You're posting to a website that's not yours.".to_owned(), - }); + return Err(MicropubError::from_static( + ErrorKind::Forbidden, + "You're posting to a website that's not yours." + )); } // Security check #3! Are we overwriting an existing document? if db.post_exists(&uid).await? { - return Err(MicropubError { - error: ErrorKind::AlreadyExists, - error_description: "UID clash was detected, operation aborted.".to_owned(), - }); + return Err(MicropubError::from_static( + ErrorKind::AlreadyExists, + "UID clash was detected, operation aborted." + )); } // Save the post tracing::debug!("Saving post to database..."); @@ -365,20 +364,20 @@ impl MicropubUpdate { if add.iter().map(|(k, _)| k.as_str()).any(|k| { k.to_lowercase().as_str() == "uid" }) { - return Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Update cannot modify the post UID".to_owned() - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Update cannot modify the post UID" + )); } } if let Some(replace) = &self.replace { if replace.iter().map(|(k, v)| k.as_str()).any(|k| { k.to_lowercase().as_str() == "uid" }) { - return Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Update cannot modify the post UID".to_owned() - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Update cannot modify the post UID" + )); } } let iter = match &self.delete { @@ -392,10 +391,10 @@ impl MicropubUpdate { }; if let Some(mut iter) = iter { if iter.any(|k| k.to_lowercase().as_str() == "uid") { - return Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Update cannot modify the post UID".to_owned() - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Update cannot modify the post UID" + )); } } Ok(()) @@ -456,13 +455,12 @@ async fn post_action( db: D, user: User, ) -> Result<(), MicropubError> { - let uri = if let Ok(uri) = action.url.parse::() { - uri - } else { - return Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Your URL doesn't parse properly.".to_owned(), - }); + let uri = match action.url.parse::() { + Ok(uri) => uri, + Err(err) => return Err(MicropubError::new( + ErrorKind::InvalidRequest, + format!("url parsing error: {}", err) + )) }; if uri.authority().unwrap() @@ -474,38 +472,38 @@ async fn post_action( .authority() .unwrap() { - return Err(MicropubError { - error: ErrorKind::Forbidden, - error_description: "Don't tamper with others' posts!".to_owned(), - }); + return Err(MicropubError::from_static( + ErrorKind::Forbidden, + "Don't tamper with others' posts!" + )); } match action.action { ActionType::Delete => { if !user.check_scope(&Scope::Delete) { - return Err(MicropubError { - error: ErrorKind::InvalidScope, - error_description: "You need a \"delete\" scope for this.".to_owned(), - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidScope, + "You need a \"delete\" scope for this." + )); } db.delete_post(&action.url).await? } ActionType::Update => { if !user.check_scope(&Scope::Update) { - return Err(MicropubError { - error: ErrorKind::InvalidScope, - error_description: "You need an \"update\" scope for this.".to_owned(), - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidScope, + "You need an \"update\" scope for this." + )); } let update = if let Some(update) = action.update { update } else { - return Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Update request is not set.".to_owned(), - }) + return Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Update request is not set." + )); }; update.check_validity()?; @@ -546,18 +544,18 @@ async fn dispatch_body( } else if let Ok(body) = serde_json::from_slice::(&body) { // quick sanity check if !body.is_object() || !body["type"].is_array() { - return Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Invalid MF2-JSON detected: `.` should be an object, `.type` should be an array of MF2 types".to_owned() - }); + return Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Invalid MF2-JSON detected: `.` should be an object, `.type` should be an array of MF2 types" + )); } Ok(PostBody::MF2(body)) } else { - Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Invalid JSON object passed.".to_owned(), - }) + Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Invalid JSON object passed." + )) } } else if content_type == ContentType::form_url_encoded() { if let Ok(body) = serde_urlencoded::from_bytes::(&body) { @@ -565,14 +563,13 @@ async fn dispatch_body( } else if let Ok(body) = serde_urlencoded::from_bytes::>(&body) { Ok(PostBody::MF2(form_to_mf2_json(body))) } else { - Err(MicropubError { - error: ErrorKind::InvalidRequest, - error_description: "Invalid form-encoded data. Try h=entry&content=Hello!" - .to_owned(), - }) + Err(MicropubError::from_static( + ErrorKind::InvalidRequest, + "Invalid form-encoded data. Try h=entry&content=Hello!" + )) } } else { - Err(MicropubError::new( + Err(MicropubError::from_static( ErrorKind::UnsupportedMediaType, "This Content-Type is not recognized. Try application/json instead?", )) @@ -616,7 +613,7 @@ pub(crate) async fn query( let query = if let Some(Query(query)) = query { query } else { - return MicropubError::new( + return MicropubError::from_static( ErrorKind::InvalidRequest, "Invalid query provided. Try ?q=config to see what you can do." ).into_response(); @@ -628,7 +625,7 @@ pub(crate) async fn query( .unwrap() != &host { - return MicropubError::new( + return MicropubError::from_static( ErrorKind::NotAuthorized, "This website doesn't belong to you.", ) @@ -644,9 +641,9 @@ pub(crate) async fn query( Err(err) => { return MicropubError::new( ErrorKind::InternalServerError, - &format!("Error fetching channels: {}", err), + format!("Error fetching channels: {}", err), ) - .into_response() + .into_response() } }; @@ -668,36 +665,36 @@ pub(crate) async fn query( map } }) - .into_response() + .into_response() } QueryType::Source => { match query.url { Some(url) => { match db.get_post(&url).await { Ok(some) => match some { - Some(post) => axum::response::Json(&post).into_response(), - None => MicropubError::new( + Some(post) => { + let mut response = axum::response::Json(&post).into_response(); + + response + }, + None => MicropubError::from_static( ErrorKind::NotFound, "The specified MF2 object was not found in database.", ) - .into_response(), + .into_response(), }, - Err(err) => MicropubError::new( - ErrorKind::InternalServerError, - &format!("Backend error: {}", err), - ) - .into_response(), + Err(err) => MicropubError::from(err).into_response(), } } None => { // Here, one should probably attempt to query at least the main feed and collect posts // Using a pre-made query function can't be done because it does unneeded filtering // Don't implement for now, this is optional - MicropubError::new( + MicropubError::from_static( ErrorKind::InvalidRequest, "Querying for post list is not implemented yet.", ) - .into_response() + .into_response() } } } @@ -705,9 +702,9 @@ pub(crate) async fn query( Ok(chans) => axum::response::Json(json!({ "channels": chans })).into_response(), Err(err) => MicropubError::new( ErrorKind::InternalServerError, - &format!("Error fetching channels: {}", err), + format!("error fetching channels: backend error: {}", err), ) - .into_response(), + .into_response(), }, QueryType::SyndicateTo => { axum::response::Json(json!({ "syndicate-to": [] })).into_response() @@ -718,16 +715,16 @@ pub(crate) async fn query( Err(err) => { return MicropubError::new( ErrorKind::InternalServerError, - &format!("Error fetching categories: {}", err) + format!("error fetching categories: backend error: {}", err) ).into_response() } }; axum::response::Json(json!({ "categories": categories })).into_response() }, - QueryType::Unknown(q) => return MicropubError { - error: ErrorKind::InvalidRequest, - error_description: format!("Invalid query: {}", q) - }.into_response(), + QueryType::Unknown(q) => return MicropubError::new( + ErrorKind::InvalidRequest, + format!("Invalid query: {}", q) + ).into_response(), } } diff --git a/templates-neo/Cargo.toml b/templates-neo/Cargo.toml index 3f58b42..22865f0 100644 --- a/templates-neo/Cargo.toml +++ b/templates-neo/Cargo.toml @@ -32,7 +32,7 @@ features = ["formatting"] version = "^0.4.19" features = ["serde"] [dependencies.kittybox-util] -version = "0.2.0" +version = "0.3.0" path = "../util" [dependencies.kittybox-indieauth] version = "0.2.0" diff --git a/templates/Cargo.toml b/templates/Cargo.toml index 5281418..607f29e 100644 --- a/templates/Cargo.toml +++ b/templates/Cargo.toml @@ -29,7 +29,7 @@ axum = "^0.7.5" version = "^0.4.19" features = ["serde"] [dependencies.kittybox-util] -version = "0.2.0" +version = "0.3.0" path = "../util" [dependencies.kittybox-indieauth] version = "0.2.0" diff --git a/util/Cargo.toml b/util/Cargo.toml index 5169cbe..8eb2b4f 100644 --- a/util/Cargo.toml +++ b/util/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kittybox-util" -version = "0.2.0" +version = "0.3.0" edition = "2021" rust-version = "1.75.0" diff --git a/util/src/micropub.rs b/util/src/micropub.rs index 6127079..9d2c525 100644 --- a/util/src/micropub.rs +++ b/util/src/micropub.rs @@ -98,8 +98,8 @@ pub struct Error { /// General kind of an error that occured. pub error: ErrorKind, /// A human-readable error description intended for application developers. - // TODO use Cow<'static, str> to save on heap allocations - pub error_description: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub error_description: Option>, } impl std::error::Error for Error {} @@ -113,9 +113,13 @@ impl std::fmt::Display for Error { }; f.write_str(&s)?; - f.write_str(" (")?; - f.write_str(&self.error_description)?; - f.write_str(")") + if let Some(desc) = self.error_description.as_deref() { + f.write_str(" (")?; + f.write_str(desc)?; + f.write_str(")")?; + }; + + Ok(()) } } @@ -124,21 +128,34 @@ impl From for Error { use ErrorKind::*; Self { error: InvalidRequest, - error_description: err.to_string(), + error_description: Some(err.to_string().into()), } } } impl Error { /// Create a new Micropub error. - pub fn new(error: ErrorKind, error_description: &str) -> Self { + pub fn new(error: ErrorKind, error_description: String) -> Self { + Self { + error, + error_description: Some(error_description.into()), + } + } + /// Create a new Micropub error from a static string. + pub const fn from_static(error: ErrorKind, error_description: &'static str) -> Self { Self { error, - error_description: error_description.to_owned(), + error_description: Some(std::borrow::Cow::Borrowed(error_description)) } } } +impl From for Error { + fn from(error: ErrorKind) -> Self { + Self { error, error_description: None } + } +} + #[cfg(feature = "http")] impl From<&Error> for http::StatusCode { fn from(err: &Error) -> Self { -- cgit 1.4.1