diff --git a/src/api.rs b/src/api.rs index b64fc22a..df7b0d8b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -593,8 +593,16 @@ impl Api { } else { // If there is no healthcheck handler registered, just return [HealthStatus::Available] // by default; after all, if this handler is getting hit at all, the service must be up. - let mut accept = Accept::from_headers(req.headers()).ok().flatten(); - route::health_check_response(&mut accept, HealthStatus::Available) + route::health_check_response( + &req.accept().unwrap_or_else(|_| { + // The healthcheck endpoint is not allowed to fail, so just use the default content + // type if we can't parse the Accept header. + let mut accept = Accept::new(); + accept.set_wildcard(true); + accept + }), + HealthStatus::Available, + ) } } @@ -653,8 +661,11 @@ where req: RequestParams, state: &State, ) -> Result> { - let accept = accept_header(&req)?; - response_from_result(accept, state.read(|state| (self.handler)(req, state)).await) + let accept = req.accept()?; + response_from_result( + &accept, + state.read(|state| (self.handler)(req, state)).await, + ) } } @@ -679,9 +690,9 @@ where req: RequestParams, state: &State, ) -> Result> { - let accept = accept_header(&req)?; + let accept = req.accept()?; response_from_result( - accept, + &accept, state.write(|state| (self.handler)(req, state)).await, ) } diff --git a/src/app.rs b/src/app.rs index eac2728c..a77bdc56 100644 --- a/src/app.rs +++ b/src/app.rs @@ -27,8 +27,7 @@ use std::collections::hash_map::{Entry, HashMap}; use std::convert::Infallible; use std::io; use tide::{ - http::headers::HeaderValue, - http::{content::Accept, mime}, + http::{headers::HeaderValue, mime}, security::{CorsMiddleware, Origin}, StatusCode, }; @@ -235,9 +234,10 @@ impl App(err).into_tide_error(), - ) + let accept = RequestParams::accept_from_headers(&req)?; + respond_with(&accept, api.version()).map_err(|err| { + Error::from_route_error::(err).into_tide_error() + }) } }); } @@ -250,14 +250,15 @@ impl App>| async move { - respond_with(&mut Accept::from_headers(&req)?, req.state().version()) + let accept = RequestParams::accept_from_headers(&req)?; + respond_with(&accept, req.state().version()) .map_err(|err| Error::from_route_error::(err).into_tide_error()) }); @@ -344,7 +345,7 @@ fn add_error_body( next: tide::Next, ) -> BoxFuture { Box::pin(async { - let mut accept = Accept::from_headers(&req)?; + let accept = RequestParams::accept_from_headers(&req)?; let mut res = next.run(req).await; if let Some(error) = res.take_error() { let error = E::from_server_error(error); @@ -352,7 +353,7 @@ fn add_error_body( // Try to add the error to the response body using a format accepted by the client. If // we cannot do that (for example, if the client requested a format that is incompatible // with a serialized error) just add the error as a string using plaintext. - let (body, content_type) = route::response_body::<_, E>(&mut accept, &error) + let (body, content_type) = route::response_body::<_, E>(&accept, &error) .unwrap_or_else(|_| (error.to_string().into(), mime::PLAIN)); res.set_body(body); res.set_content_type(content_type); diff --git a/src/request.rs b/src/request.rs index 0fbc6e2b..92227c7b 100644 --- a/src/request.rs +++ b/src/request.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::fmt::Display; use strum_macros::EnumString; use tagged_base64::TaggedBase64; -use tide::http::Headers; +use tide::http::{content::Accept, Headers}; #[derive(Clone, Debug, Snafu)] pub enum RequestError { @@ -37,6 +37,9 @@ pub enum RequestError { #[snafu(display("Body type not specified or type not supported"))] UnsupportedBody, + + #[snafu(display("HTTP protocol error: {}", reason))] + Http { reason: String }, } /// Parameters passed to a route handler. @@ -73,6 +76,37 @@ impl RequestParams { &self.headers } + /// The [Accept] header of this request. + /// + /// The media type proposals in the resulting header are sorted in order of decreasing weight. + /// + /// If no [Accept] header was explicitly set, defaults to the wildcard `Accept: *`. + /// + /// # Error + /// + /// Returns [RequestError::Http] if the [Accept] header is malformed. + pub fn accept(&self) -> Result { + Self::accept_from_headers(&self.headers) + } + + pub(crate) fn accept_from_headers( + headers: impl AsRef, + ) -> Result { + match Accept::from_headers(headers).map_err(|err| RequestError::Http { + reason: err.to_string(), + })? { + Some(mut accept) => { + accept.sort(); + Ok(accept) + } + None => { + let mut accept = Accept::new(); + accept.set_wildcard(true); + Ok(accept) + } + } + } + /// Get the value of a named parameter. /// /// The name of the parameter can be given by any type that implements [Display]. Of course, the diff --git a/src/route.rs b/src/route.rs index 499b8ff5..5166190d 100644 --- a/src/route.rs +++ b/src/route.rs @@ -1,6 +1,6 @@ use crate::{ healthcheck::HealthCheck, - request::{RequestParam, RequestParamType, RequestParams}, + request::{RequestError, RequestParam, RequestParamType, RequestParams}, }; use async_trait::async_trait; use derive_more::From; @@ -29,6 +29,7 @@ use tide::{ /// result of the user-installed handler into an HTTP response. pub enum RouteError { AppSpecific(E), + Request(RequestError), UnsupportedContentType, Bincode(bincode::Error), Json(serde_json::Error), @@ -39,6 +40,7 @@ impl Display for RouteError { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { Self::AppSpecific(err) => write!(f, "{}", err), + Self::Request(err) => write!(f, "{}", err), Self::UnsupportedContentType => write!(f, "requested content type is not supported"), Self::Bincode(err) => write!(f, "error creating byte stream: {}", err), Self::Json(err) => write!(f, "error creating JSON response: {}", err), @@ -50,7 +52,7 @@ impl Display for RouteError { impl RouteError { pub fn status(&self) -> StatusCode { match self { - Self::UnsupportedContentType => StatusCode::BadRequest, + Self::Request(_) | Self::UnsupportedContentType => StatusCode::BadRequest, _ => StatusCode::InternalServerError, } } @@ -58,6 +60,7 @@ impl RouteError { pub fn map_app_specific(self, f: impl Fn(E) -> E2) -> RouteError { match self { RouteError::AppSpecific(e) => RouteError::AppSpecific(f(e)), + RouteError::Request(e) => RouteError::Request(e), RouteError::UnsupportedContentType => RouteError::UnsupportedContentType, RouteError::Bincode(err) => RouteError::Bincode(err), RouteError::Json(err) => RouteError::Json(err), @@ -66,6 +69,12 @@ impl RouteError { } } +impl From for RouteError { + fn from(err: RequestError) -> Self { + Self::Request(err) + } +} + /// A route handler. /// /// The [Handler] trait defines the interface required of route handlers -- they must be able to @@ -115,23 +124,17 @@ where req: RequestParams, state: &State, ) -> Result> { - let accept = accept_header(&req)?; - response_from_result(accept, (self.0)(req, state).await) + let accept = req.accept()?; + response_from_result(&accept, (self.0)(req, state).await) } } -pub(crate) fn accept_header( - req: &RequestParams, -) -> Result, RouteError> { - Accept::from_headers(req.headers()).map_err(RouteError::Tide) -} - pub(crate) fn response_from_result( - mut accept: Option, + accept: &Accept, res: Result, ) -> Result> { res.map_err(RouteError::AppSpecific) - .and_then(|res| respond_with(&mut accept, &res)) + .and_then(|res| respond_with(accept, &res)) } #[async_trait] @@ -399,10 +402,7 @@ where pub(crate) type HealthCheckHandler = Box BoxFuture<'_, tide::Response>>; -pub(crate) fn health_check_response( - accept: &mut Option, - health: H, -) -> tide::Response { +pub(crate) fn health_check_response(accept: &Accept, health: H) -> tide::Response { let status = health.status(); let (body, content_type) = response_body::(accept, health).unwrap_or_else(|err| { @@ -431,67 +431,59 @@ where F: 'static + Send + Future, { Box::new(move |req, state| { - let mut accept = Accept::from_headers(req.headers()).ok().flatten(); + let accept = req.accept().unwrap_or_else(|_| { + // The healthcheck endpoint is not allowed to fail, so just use the default content + // type if we can't parse the Accept header. + let mut accept = Accept::new(); + accept.set_wildcard(true); + accept + }); let future = handler(state); async move { let health = future.await; - health_check_response(&mut accept, health) + health_check_response(&accept, health) } .boxed() }) } -fn best_response_type( - accept: &mut Option, - available: &[Mime], -) -> Result> { - match accept { - Some(accept) => { - // The Accept type has a `negotiate` method, but it doesn't properly handle - // wildcards. It handles * but not */* and basetype/*, because for content type - // proposals like */* and basetype/*, it looks for a literal match in `available`, - // it does not perform pattern matching. So, we implement negotiation ourselves. - // - // First sort by the weight parameter, which the Accept type does do correctly. - accept.sort(); - // Go through each proposed content type, in the order specified by the client, and - // match them against our available types, respecting wildcards. - for proposed in accept.iter() { - if proposed.basetype() == "*" { - // The only acceptable Accept value with a basetype of * is */*, therefore - // this will match any available type. - return Ok(available[0].clone()); - } else if proposed.subtype() == "*" { - // If the subtype is * but the basetype is not, look for a proposed type - // with a matching basetype and any subtype. - for mime in available { - if mime.basetype() == proposed.basetype() { - return Ok(mime.clone()); - } - } - } else if available.contains(proposed) { - // If neither part of the proposal is a wildcard, look for a literal match. - return Ok((**proposed).clone()); +fn best_response_type(accept: &Accept, available: &[Mime]) -> Result> { + // The Accept type has a `negotiate` method, but it doesn't properly handle wildcards. It + // handles * but not */* and basetype/*, because for content type proposals like */* and + // basetype/*, it looks for a literal match in `available`, it does not perform pattern + // matching. So, we implement negotiation ourselves. Go through each proposed content type, in + // the order specified by the client, and match them against our available types, respecting + // wildcards. + for proposed in accept.iter() { + if proposed.basetype() == "*" { + // The only acceptable Accept value with a basetype of * is */*, therefore this will + // match any available type. + return Ok(available[0].clone()); + } else if proposed.subtype() == "*" { + // If the subtype is * but the basetype is not, look for a proposed type with a matching + // basetype and any subtype. + for mime in available { + if mime.basetype() == proposed.basetype() { + return Ok(mime.clone()); } } - - if accept.wildcard() { - // If no proposals are available but a wildcard flag * was given, return any - // available content type. - Ok(available[0].clone()) - } else { - Err(RouteError::UnsupportedContentType) - } - } - None => { - // If no content type is explicitly requested, default to the first available type. - Ok(available[0].clone()) + } else if available.contains(proposed) { + // If neither part of the proposal is a wildcard, look for a literal match. + return Ok((**proposed).clone()); } } + + if accept.wildcard() { + // If no proposals are available but a wildcard flag * was given, return any available + // content type. + Ok(available[0].clone()) + } else { + Err(RouteError::UnsupportedContentType) + } } pub(crate) fn response_body( - accept: &mut Option, + accept: &Accept, body: T, ) -> Result<(Body, Mime), RouteError> { let ty = best_response_type(accept, &[mime::JSON, mime::BYTE_STREAM])?; @@ -507,7 +499,7 @@ pub(crate) fn response_body( } pub(crate) fn respond_with( - accept: &mut Option, + accept: &Accept, body: T, ) -> Result> { let (body, content_type) = response_body(accept, body)?;