diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e33c74f..48afa6bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ - Fix prompt in TUI always rendering as sensitive ([#108](https://github.com/LucasPickering/slumber/issues/108)) - Fix content type identification for extended JSON MIME types ([#103](https://github.com/LucasPickering/slumber/issues/103)) +- Use named records in binary blobs in the local DB + - This required wiping out existing binary blobs, meaning **all request history and UI state will be lost on upgrade** ## [0.13.1] - 2024-03-07 diff --git a/Cargo.lock b/Cargo.lock index ad0367e0..bc7c629a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,6 +195,9 @@ name = "bytes" version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2bd12c1caf447e69cd4528f47f94d203fd2582878ecb9e9465484c4148a8223" +dependencies = [ + "serde", +] [[package]] name = "cassowary" @@ -1802,6 +1805,7 @@ version = "0.13.1" dependencies = [ "anyhow", "async-trait", + "bytes", "chrono", "clap", "cli-clipboard", @@ -2219,6 +2223,7 @@ dependencies = [ "form_urlencoded", "idna", "percent-encoding", + "serde", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 946db62e..cc9ec2de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ rust-version = "1.74.0" [dependencies] anyhow = {version = "^1.0.75", features = ["backtrace"]} async-trait = "^0.1.73" +bytes = { version = "1.5.0", features = ["serde"] } chrono = {version = "^0.4.31", default-features = false, features = ["clock", "serde", "std"]} clap = {version = "^4.4.2", features = ["derive"]} cli-clipboard = "0.4.0" @@ -43,7 +44,7 @@ thiserror = "^1.0.48" tokio = {version = "^1.32.0", default-features = false, features = ["full"]} tracing = "^0.1.37" tracing-subscriber = {version = "^0.3.17", default-features = false, features = ["env-filter", "fmt", "registry"]} -url = "^2.5.0" +url = { version = "^2.5.0", features = ["serde"] } uuid = {version = "^1.4.1", default-features = false, features = ["serde", "v4"]} [dev-dependencies] diff --git a/src/cli/request.rs b/src/cli/request.rs index 586413e5..7aae5a09 100644 --- a/src/cli/request.rs +++ b/src/cli/request.rs @@ -5,7 +5,7 @@ use crate::{ db::Database, http::{HttpEngine, RecipeOptions, RequestBuilder}, template::{Prompt, Prompter, TemplateContext}, - util::ResultExt, + util::{MaybeStr, ResultExt}, GlobalArgs, }; use anyhow::{anyhow, Context}; @@ -137,7 +137,7 @@ impl Subcommand for RequestCommand { println!("{}", HeaderDisplay(&record.response.headers)); } if !self.no_body { - print!("{}", record.response.body.text()); + print!("{}", MaybeStr(record.response.body.bytes())); } if self.exit_status && status.as_u16() >= 400 { @@ -200,7 +200,7 @@ impl<'a> Display for HeaderDisplay<'a> { f, "{}: {}", key_style.apply_to(key), - value.to_str().unwrap_or("") + MaybeStr(value.as_bytes()), )?; } Ok(()) diff --git a/src/db.rs b/src/db.rs index da07df4f..12571d01 100644 --- a/src/db.rs +++ b/src/db.rs @@ -120,6 +120,11 @@ impl Database { )", ) .down("DROP TABLE ui_state"), + // This is a sledgehammer migration. Added when we switch from + // rmp_serde::to_vec to rmp_serde::to_vec_named. This affected the + // serialization of all binary blobs, so there's no easy way to + // migrate it all. It's easiest just to wipe it all out. + M::up("DELETE FROM requests; DELETE FROM ui_state;").down(""), ]); migrations.to_latest(connection)?; Ok(()) @@ -519,7 +524,7 @@ struct Bytes(T); impl ToSql for Bytes { fn to_sql(&self) -> rusqlite::Result> { - let bytes = rmp_serde::to_vec(&self.0).map_err(|err| { + let bytes = rmp_serde::to_vec_named(&self.0).map_err(|err| { rusqlite::Error::ToSqlConversionFailure(Box::new(err)) })?; Ok(ToSqlOutput::Owned(bytes.into())) diff --git a/src/factory.rs b/src/factory.rs index 3a0b0c84..e60bbb85 100644 --- a/src/factory.rs +++ b/src/factory.rs @@ -6,7 +6,6 @@ use crate::{ }; use chrono::Utc; use factori::{create, factori}; -use indexmap::IndexMap; use reqwest::{header::HeaderMap, Method, StatusCode}; factori!(Profile, { @@ -23,9 +22,8 @@ factori!(Request, { profile_id = None, recipe_id = "recipe1".into(), method = Method::GET, - url = "/url".into(), + url = "http://localhost/url".parse().unwrap(), headers = HeaderMap::new(), - query = IndexMap::new(), body = None, } }); diff --git a/src/http.rs b/src/http.rs index b45b8e44..7e0219f7 100644 --- a/src/http.rs +++ b/src/http.rs @@ -46,6 +46,7 @@ use crate::{ template::TemplateContext, util::ResultExt, }; use anyhow::Context; +use bytes::Bytes; use chrono::Utc; use futures::future; use indexmap::IndexMap; @@ -56,6 +57,7 @@ use reqwest::{ use std::collections::HashSet; use tokio::try_join; use tracing::{debug, info, info_span}; +use url::Url; const USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")); @@ -201,8 +203,7 @@ impl HttpEngine { // Convert to reqwest's request format let mut request_builder = self .client - .request(request.method.clone(), &request.url) - .query(&request.query) + .request(request.method.clone(), request.url.clone()) .headers(request.headers.clone()); // Add body @@ -226,7 +227,7 @@ impl HttpEngine { let headers = response.headers().clone(); // Pre-resolve the content, so we get all the async work done - let body = response.text().await?.into(); + let body = response.bytes().await?.into(); Ok(Response { status, @@ -308,13 +309,18 @@ impl RequestBuilder { let method = self.recipe.method.parse()?; // Render everything in parallel - let (url, headers, query, body) = try_join!( + let (mut url, headers, query, body) = try_join!( self.render_url(), self.render_headers(), self.render_query(), self.render_body(), )?; + // Join query into URL. if check prevents bare ? for empty query + if !query.is_empty() { + url.query_pairs_mut().extend_pairs(&query); + } + info!( recipe_id = %self.recipe.id, "Built request from recipe", @@ -329,18 +335,19 @@ impl RequestBuilder { recipe_id: self.recipe.id, method, url, - query, body, headers, }) } - async fn render_url(&self) -> anyhow::Result { - self.recipe - .url - .render(&self.template_context) - .await - .context("Error rendering URL") + async fn render_url(&self) -> anyhow::Result { + // Shitty try block + async { + let url = self.recipe.url.render(&self.template_context).await?; + url.parse().map_err(anyhow::Error::from) + } + .await + .context("Error rendering URL") } async fn render_headers(&self) -> anyhow::Result { @@ -404,13 +411,16 @@ impl RequestBuilder { .collect::>()) } - async fn render_body(&self) -> anyhow::Result> { + async fn render_body(&self) -> anyhow::Result> { match &self.recipe.body { - Some(body) => Ok(Some( - body.render(&self.template_context) + Some(body) => { + let body = body + .render(&self.template_context) .await - .context("Error rendering body")?, - )), + .context("Error rendering body")? + .into(); + Ok(Some(body)) + } None => Ok(None), } } diff --git a/src/http/parse.rs b/src/http/parse.rs index 196eef2d..997511af 100644 --- a/src/http/parse.rs +++ b/src/http/parse.rs @@ -36,7 +36,7 @@ pub trait ResponseContent: Debug + Display { fn content_type(&self) -> ContentType; /// Parse the response body as this type - fn parse(body: &str) -> anyhow::Result + fn parse(body: &[u8]) -> anyhow::Result where Self: Sized; @@ -62,8 +62,8 @@ impl ResponseContent for Json { ContentType::Json } - fn parse(body: &str) -> anyhow::Result { - Ok(Self(serde_json::from_str(body)?)) + fn parse(body: &[u8]) -> anyhow::Result { + Ok(Self(serde_json::from_slice(body)?)) } fn prettify(&self) -> String { @@ -86,7 +86,7 @@ impl ContentType { /// object. pub fn parse_content( self, - content: &str, + content: &[u8], ) -> anyhow::Result> { match self { Self::Json => Ok(Box::new(Json::parse(content)?)), @@ -111,7 +111,8 @@ impl ContentType { pub(super) fn parse_response( response: &Response, ) -> anyhow::Result> { - Self::from_response(response)?.parse_content(response.body.text()) + let content_type = Self::from_response(response)?; + content_type.parse_content(response.body.bytes()) } /// Parse the content type from a file's extension @@ -128,10 +129,10 @@ impl ContentType { /// Parse the content type from a response's `Content-Type` header pub fn from_response(response: &Response) -> anyhow::Result { // If the header value isn't utf-8, we're hosed - let header_value = - std::str::from_utf8(response.content_type().ok_or_else(|| { - anyhow!("Response has no content-type header") - })?) + let header_value = response + .content_type() + .ok_or_else(|| anyhow!("Response has no content-type header"))?; + let header_value = std::str::from_utf8(header_value) .context("content-type header is not valid utf-8")?; Self::from_header(header_value) } diff --git a/src/http/record.rs b/src/http/record.rs index dcc74134..1f07c466 100644 --- a/src/http/record.rs +++ b/src/http/record.rs @@ -6,6 +6,7 @@ use crate::{ util::ResultExt, }; use anyhow::Context; +use bytes::Bytes; use chrono::{DateTime, Duration, Utc}; use derive_more::{Display, From}; use indexmap::IndexMap; @@ -16,6 +17,7 @@ use reqwest::{ use serde::{Deserialize, Serialize}; use std::fmt::Debug; use thiserror::Error; +use url::Url; use uuid::Uuid; /// An error that can occur while *building* a request @@ -100,13 +102,11 @@ pub struct Request { #[serde(with = "serde_method")] pub method: Method, - pub url: String, + pub url: Url, #[serde(with = "serde_header_map")] pub headers: HeaderMap, - pub query: IndexMap, - /// Text body content. At some point we'll support other formats (binary, - /// streaming from file, etc.) - pub body: Option, + /// Body content as bytes. This should be decoded as needed + pub body: Option, } /// A resolved HTTP response, with all content loaded and ready to be displayed @@ -145,22 +145,38 @@ impl Response { } } -/// HTTP response body. Right now we store as text only, but at some point -/// should add support for binary responses +/// HTTP response body. Content is stored as bytes to support non-text content. +/// Should be converted to text only as needed #[derive(Default, From, Serialize, Deserialize)] -pub struct Body(String); +pub struct Body(Bytes); impl Body { - pub fn new(text: String) -> Self { - Self(text) + pub fn new(bytes: Bytes) -> Self { + Self(bytes) } - pub fn text(&self) -> &str { + /// Raw content bytes + pub fn bytes(&self) -> &[u8] { &self.0 } - pub fn into_text(self) -> String { - self.0 + /// Owned raw content bytes + pub fn into_bytes(self) -> Vec { + self.0.into() + } +} + +#[cfg(test)] +impl From for Body { + fn from(value: String) -> Self { + Self(value.into()) + } +} + +#[cfg(test)] +impl From<&str> for Body { + fn from(value: &str) -> Self { + value.to_owned().into() } } @@ -173,12 +189,6 @@ impl Debug for Body { } } -impl From<&str> for Body { - fn from(value: &str) -> Self { - Body::new(value.into()) - } -} - /// Serialization/deserialization for [reqwest::Method] mod serde_method { use super::*; diff --git a/src/template/error.rs b/src/template/error.rs index e44277e9..f0718e1b 100644 --- a/src/template/error.rs +++ b/src/template/error.rs @@ -75,6 +75,12 @@ pub enum ChainError { /// generated by our code so we don't need any extra context. #[error(transparent)] Database(anyhow::Error), + /// Chain source produced non-UTF-8 bytes + #[error("Error decoding content as UTF-8")] + InvalidUtf8 { + #[source] + error: FromUtf8Error, + }, /// The chain ID is valid, but the corresponding recipe has no successful /// response #[error("No response available")] @@ -104,12 +110,6 @@ pub enum ChainError { #[source] error: io::Error, }, - #[error("Error decoding output for {command:?}")] - CommandInvalidUtf8 { - command: Vec, - #[source] - error: FromUtf8Error, - }, #[error("Error reading from file `{path}`")] File { path: PathBuf, diff --git a/src/template/render.rs b/src/template/render.rs index cb24109d..b9c6a8eb 100644 --- a/src/template/render.rs +++ b/src/template/render.rs @@ -241,7 +241,7 @@ impl<'a> TemplateSource<'a> for ChainTemplateSource<'a> { // Guess content type based on HTTP header let content_type = ContentType::from_response(&response).ok(); - (response.body.into_text(), content_type) + (response.body.into_bytes(), content_type) } ChainSource::File(path) => { // Guess content type based on file extension @@ -258,7 +258,8 @@ impl<'a> TemplateSource<'a> for ChainTemplateSource<'a> { label.as_deref(), chain.sensitive, ) - .await?, + .await? + .into_bytes(), // No way to guess content type on this None, ), @@ -277,7 +278,9 @@ impl<'a> TemplateSource<'a> for ChainTemplateSource<'a> { .map_err(|err| ChainError::ParseResponse { error: err })?; selector.query_to_string(&*value)? } else { - value + // We just want raw text - decode as UTF-8 + String::from_utf8(value) + .map_err(|error| ChainError::InvalidUtf8 { error })? }; Ok(RenderedChunk { @@ -315,20 +318,18 @@ impl<'a> ChainTemplateSource<'a> { } /// Render a chained value from a file - async fn render_file(&self, path: &'a Path) -> Result { - fs::read_to_string(path) - .await - .map_err(|error| ChainError::File { - path: path.to_owned(), - error, - }) + async fn render_file(&self, path: &'a Path) -> Result, ChainError> { + fs::read(path).await.map_err(|error| ChainError::File { + path: path.to_owned(), + error, + }) } /// Render a chained value from an external command async fn render_command( &self, command: &[String], - ) -> Result { + ) -> Result, ChainError> { match command { [] => Err(ChainError::CommandMissing), [program, args @ ..] => { @@ -343,14 +344,9 @@ impl<'a> ChainTemplateSource<'a> { ?command, stdout = %String::from_utf8_lossy(&output.stdout), stderr = %String::from_utf8_lossy(&output.stderr), - "Executing subcommand" + "Executed subcommand" ); - String::from_utf8(output.stdout).map_err(|error| { - ChainError::CommandInvalidUtf8 { - command: command.to_owned(), - error, - } - }) + Ok(output.stdout) } } } diff --git a/src/tui/view/common.rs b/src/tui/view/common.rs index 3a70c7f9..8ae4fd82 100644 --- a/src/tui/view/common.rs +++ b/src/tui/view/common.rs @@ -17,6 +17,7 @@ use crate::{ context::TuiContext, view::{draw::Generate, state::Notification}, }, + util::MaybeStr, }; use chrono::{DateTime, Duration, Local, Utc}; use itertools::Itertools; @@ -185,10 +186,7 @@ impl Generate for &HeaderValue { where Self: 'this, { - match self.to_str() { - Ok(s) => s.into(), - Err(_) => "".into(), - } + MaybeStr(self.as_bytes()).to_str().into() } } diff --git a/src/tui/view/component/response/body.rs b/src/tui/view/component/response/body.rs index 4481ef7f..17d520e8 100644 --- a/src/tui/view/component/response/body.rs +++ b/src/tui/view/component/response/body.rs @@ -13,7 +13,7 @@ use crate::{ Component, }, }, - util::ResultExt, + util::{MaybeStr, ResultExt}, }; use anyhow::Context; use derive_more::Debug; @@ -167,7 +167,8 @@ fn init_text_window( .unwrap_or_else(|| parsed_body.prettify()) }) // Content couldn't be parsed, fall back to the raw text - .unwrap_or_else(|| record.response.body.text().to_owned()); + // If the text isn't UTF-8, we'll show a placeholder instead + .unwrap_or_else(|| MaybeStr(record.response.body.bytes()).to_string()); TextWindow::new(body).into() } diff --git a/src/util.rs b/src/util.rs index c6ad4629..b987898f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -3,7 +3,7 @@ use anyhow::Context; use derive_more::{DerefMut, Display}; use serde::de::DeserializeOwned; use std::{ - fs, + fmt, fs, ops::Deref, path::{Path, PathBuf}, }; @@ -139,6 +139,22 @@ impl ResultExt for Result { } } +/// Helper to printing bytes. If the bytes aren't valid UTF-8, a message about +/// them being invalid will be printed instead. +pub struct MaybeStr<'a>(pub &'a [u8]); + +impl<'a> MaybeStr<'a> { + pub fn to_str(&self) -> &'a str { + std::str::from_utf8(self.0).unwrap_or("") + } +} + +impl<'a> Display for MaybeStr<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.to_str()) + } +} + /// A wrapper around two iterable enums, chaining them into a single iterator #[derive(Copy, Clone, Debug, Display, PartialEq)] pub enum EnumChain {