From 1d8cb1f0fa6bbe851701fb85c6ee73ff9c412cbf Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Wed, 24 Aug 2022 22:50:15 +0000 Subject: [PATCH 1/9] Add protocol specific routers --- .../aws-smithy-http-server/src/protocols.rs | 12 + .../aws-smithy-http-server/src/response.rs | 4 + .../src/routing/future.rs | 22 +- .../aws-smithy-http-server/src/routing/mod.rs | 213 +++++------------- .../src/routing/routers/aws_json.rs | 128 +++++++++++ .../src/routing/routers/mod.rs | 144 ++++++++++++ .../src/routing/routers/rest.rs | 129 +++++++++++ .../src/runtime_error.rs | 4 - 8 files changed, 478 insertions(+), 178 deletions(-) create mode 100644 rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs create mode 100644 rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs create mode 100644 rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs diff --git a/rust-runtime/aws-smithy-http-server/src/protocols.rs b/rust-runtime/aws-smithy-http-server/src/protocols.rs index 5f099f8eb4..8da8d05e7f 100644 --- a/rust-runtime/aws-smithy-http-server/src/protocols.rs +++ b/rust-runtime/aws-smithy-http-server/src/protocols.rs @@ -7,6 +7,18 @@ use crate::rejection::MissingContentTypeReason; use crate::request::RequestParts; +/// Rest JSON 1.0 Protocol. +pub struct RestJson1; + +/// Rest XML Protocol. +pub struct RestXml1; + +/// AWS JSON 1.0 Protocol. +pub struct AwsJson10; + +/// AWS JSON 1.1 Protocol. +pub struct AwsJson11; + /// Supported protocols. #[derive(Debug, Clone, Copy)] pub enum Protocol { diff --git a/rust-runtime/aws-smithy-http-server/src/response.rs b/rust-runtime/aws-smithy-http-server/src/response.rs index fab5f5e0f2..a85b72395c 100644 --- a/rust-runtime/aws-smithy-http-server/src/response.rs +++ b/rust-runtime/aws-smithy-http-server/src/response.rs @@ -36,3 +36,7 @@ use crate::body::BoxBody; #[doc(hidden)] pub type Response = http::Response; + +pub trait IntoResponse { + fn into_response(self) -> http::Response; +} diff --git a/rust-runtime/aws-smithy-http-server/src/routing/future.rs b/rust-runtime/aws-smithy-http-server/src/routing/future.rs index a0e6345042..95d4cbcf5f 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/future.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/future.rs @@ -33,29 +33,11 @@ */ //! Future types. -use crate::body::BoxBody; -use futures_util::future::Either; -use http::{Request, Response}; -use std::{convert::Infallible, future::ready}; -use tower::util::Oneshot; +use super::Route; pub use super::{into_make_service::IntoMakeService, route::RouteFuture}; -type OneshotRoute = Oneshot, Request>; -type ReadyResponse = std::future::Ready, Infallible>>; - opaque_future! { /// Response future for [`Router`](super::Router). - pub type RouterFuture = - futures_util::future::Either, ReadyResponse>; -} - -impl RouterFuture { - pub(super) fn from_oneshot(future: Oneshot, Request>) -> Self { - Self::new(Either::Left(future)) - } - - pub(super) fn from_response(response: Response) -> Self { - Self::new(Either::Right(ready(Ok(response)))) - } + pub type RouterFuture = super::routers::RoutingFuture, B>; } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs index 31c2d94f66..5ac611edde 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs @@ -8,18 +8,17 @@ //! [Smithy specification]: https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html use self::request_spec::RequestSpec; -use self::tiny_map::TinyMap; +use self::routers::{aws_json::AwsJsonRouter, rest::RestRouter, RoutingService}; use crate::body::{boxed, Body, BoxBody, HttpBody}; use crate::error::BoxError; -use crate::protocols::Protocol; -use crate::runtime_error::{RuntimeError, RuntimeErrorKind}; -use http::{Request, Response, StatusCode}; +use crate::protocols::{AwsJson10, AwsJson11, RestJson1, RestXml1}; + +use http::{Request, Response}; use std::{ convert::Infallible, task::{Context, Poll}, }; use tower::layer::Layer; -use tower::util::ServiceExt; use tower::{Service, ServiceBuilder}; use tower_http::map_response_body::MapResponseBodyLayer; @@ -31,6 +30,7 @@ mod lambda_handler; pub mod request_spec; mod route; +mod routers; mod tiny_map; pub use self::lambda_handler::LambdaHandler; @@ -61,11 +61,6 @@ pub struct Router { routes: Routes, } -// This constant determines when the `TinyMap` implementation switches from being a `Vec` to a -// `HashMap`. This is chosen to be 15 as a result of the discussion around -// https://github.com/awslabs/smithy-rs/pull/1429#issuecomment-1147516546 -const ROUTE_CUTOFF: usize = 15; - /// Protocol-aware routes types. /// /// RestJson1 and RestXml routes are stored in a `Vec` because there can be multiple matches on the @@ -75,10 +70,10 @@ const ROUTE_CUTOFF: usize = 15; /// directly found in the `X-Amz-Target` HTTP header. #[derive(Debug)] enum Routes { - RestXml(Vec<(Route, RequestSpec)>), - RestJson1(Vec<(Route, RequestSpec)>), - AwsJson10(TinyMap, ROUTE_CUTOFF>), - AwsJson11(TinyMap, ROUTE_CUTOFF>), + RestXml(RoutingService>, RestXml1>), + RestJson1(RoutingService>, RestJson1>), + AwsJson10(RoutingService>, AwsJson10>), + AwsJson11(RoutingService>, AwsJson11>), } impl Clone for Router { @@ -104,29 +99,6 @@ impl Router where B: Send + 'static, { - /// Return the correct, protocol-specific "Not Found" response for an unknown operation. - fn unknown_operation(&self) -> RouterFuture { - let protocol = match &self.routes { - Routes::RestJson1(_) => Protocol::RestJson1, - Routes::RestXml(_) => Protocol::RestXml, - Routes::AwsJson10(_) => Protocol::AwsJson10, - Routes::AwsJson11(_) => Protocol::AwsJson11, - }; - let error = RuntimeError { - protocol, - kind: RuntimeErrorKind::UnknownOperation, - }; - RouterFuture::from_response(error.into_response()) - } - - /// Return the HTTP error response for non allowed method. - fn method_not_allowed(&self) -> RouterFuture { - RouterFuture::from_response({ - let mut res = Response::new(crate::body::empty()); - *res.status_mut() = StatusCode::METHOD_NOT_ALLOWED; - res - }) - } /// Convert this router into a [`MakeService`], that is a [`Service`] whose /// response is another service. /// @@ -151,6 +123,7 @@ where L::Service: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, >>::Future: Send + 'static, + NewReqBody: Send + 'static, NewResBody: HttpBody + Send + 'static, NewResBody::Error: Into, { @@ -159,42 +132,18 @@ where .layer(MapResponseBodyLayer::new(boxed)) .layer(layer); match self.routes { - Routes::RestJson1(routes) => { - let routes = routes - .into_iter() - .map(|(route, request_spec)| (Layer::layer(&layer, route), request_spec)) - .collect(); - Router { - routes: Routes::RestJson1(routes), - } - } - Routes::RestXml(routes) => { - let routes = routes - .into_iter() - .map(|(route, request_spec)| (Layer::layer(&layer, route), request_spec)) - .collect(); - Router { - routes: Routes::RestXml(routes), - } - } - Routes::AwsJson10(routes) => { - let routes = routes - .into_iter() - .map(|(operation, route)| (operation, Layer::layer(&layer, route))) - .collect(); - Router { - routes: Routes::AwsJson10(routes), - } - } - Routes::AwsJson11(routes) => { - let routes = routes - .into_iter() - .map(|(operation, route)| (operation, Layer::layer(&layer, route))) - .collect(); - Router { - routes: Routes::AwsJson11(routes), - } - } + Routes::RestJson1(routes) => Router { + routes: Routes::RestJson1(routes.map(|router| router.layer(layer).boxed())), + }, + Routes::RestXml(routes) => Router { + routes: Routes::RestXml(routes.map(|router| router.layer(layer).boxed())), + }, + Routes::AwsJson10(routes) => Router { + routes: Routes::AwsJson10(routes.map(|router| router.layer(layer).boxed())), + }, + Routes::AwsJson11(routes) => Router { + routes: Routes::AwsJson11(routes.map(|router| router.layer(layer).boxed())), + }, } } @@ -211,18 +160,14 @@ where ), >, { - let mut routes: Vec<(Route, RequestSpec)> = routes - .into_iter() - .map(|(svc, request_spec)| (Route::from_box_clone_service(svc), request_spec)) - .collect(); - - // Sort them once by specifity, with the more specific routes sorted before the less - // specific ones, so that when routing a request we can simply iterate through the routes - // and pick the first one that matches. - routes.sort_by_key(|(_route, request_spec)| std::cmp::Reverse(request_spec.rank())); - + let svc = RoutingService::new( + routes + .into_iter() + .map(|(svc, request_spec)| (Route::from_box_clone_service(svc), request_spec)) + .collect(), + ); Self { - routes: Routes::RestJson1(routes), + routes: Routes::RestJson1(svc), } } @@ -239,18 +184,14 @@ where ), >, { - let mut routes: Vec<(Route, RequestSpec)> = routes - .into_iter() - .map(|(svc, request_spec)| (Route::from_box_clone_service(svc), request_spec)) - .collect(); - - // Sort them once by specifity, with the more specific routes sorted before the less - // specific ones, so that when routing a request we can simply iterate through the routes - // and pick the first one that matches. - routes.sort_by_key(|(_route, request_spec)| std::cmp::Reverse(request_spec.rank())); - + let svc = RoutingService::new( + routes + .into_iter() + .map(|(svc, request_spec)| (Route::from_box_clone_service(svc), request_spec)) + .collect(), + ); Self { - routes: Routes::RestXml(routes), + routes: Routes::RestXml(svc), } } @@ -267,13 +208,15 @@ where ), >, { - let routes = routes - .into_iter() - .map(|(svc, operation)| (operation, Route::from_box_clone_service(svc))) - .collect(); + let svc = RoutingService::new( + routes + .into_iter() + .map(|(svc, operation)| (operation, Route::from_box_clone_service(svc))) + .collect(), + ); Self { - routes: Routes::AwsJson10(routes), + routes: Routes::AwsJson10(svc), } } @@ -290,13 +233,15 @@ where ), >, { - let routes = routes - .into_iter() - .map(|(svc, operation)| (operation, Route::from_box_clone_service(svc))) - .collect(); + let svc = RoutingService::new( + routes + .into_iter() + .map(|(svc, operation)| (operation, Route::from_box_clone_service(svc))) + .collect(), + ); Self { - routes: Routes::AwsJson11(routes), + routes: Routes::AwsJson11(svc), } } } @@ -316,55 +261,15 @@ where #[inline] fn call(&mut self, req: Request) -> Self::Future { - match &self.routes { + let fut = match &mut self.routes { // REST routes. - Routes::RestJson1(routes) | Routes::RestXml(routes) => { - let mut method_not_allowed = false; - - // Loop through all the routes and validate if any of them matches. Routes are already ranked. - for (route, request_spec) in routes { - match request_spec.matches(&req) { - request_spec::Match::Yes => { - return RouterFuture::from_oneshot(route.clone().oneshot(req)); - } - request_spec::Match::MethodNotAllowed => method_not_allowed = true, - // Continue looping to see if another route matches. - request_spec::Match::No => continue, - } - } - - if method_not_allowed { - // The HTTP method is not correct. - self.method_not_allowed() - } else { - // In any other case return the `RuntimeError::UnknownOperation`. - self.unknown_operation() - } - } + Routes::RestJson1(routes) => routes.call(req), + Routes::RestXml(routes) => routes.call(req), // AwsJson routes. - Routes::AwsJson10(routes) | Routes::AwsJson11(routes) => { - if req.uri() == "/" { - // Check the request method for POST. - if req.method() == http::Method::POST { - // Find the `x-amz-target` header. - if let Some(target) = req.headers().get("x-amz-target") { - if let Ok(target) = target.to_str() { - // Lookup in the `TinyMap` for a route for the target. - let route = routes.get(target); - if let Some(route) = route { - return RouterFuture::from_oneshot(route.clone().oneshot(req)); - } - } - } - } else { - // The HTTP method is not POST. - return self.method_not_allowed(); - } - } - // In any other case return the `RuntimeError::UnknownOperation`. - self.unknown_operation() - } - } + Routes::AwsJson10(routes) => routes.call(req), + Routes::AwsJson11(routes) => routes.call(req), + }; + RouterFuture::new(fut) } } @@ -376,7 +281,7 @@ mod rest_tests { routing::request_spec::*, }; use futures_util::Future; - use http::{HeaderMap, Method}; + use http::{HeaderMap, Method, StatusCode}; use std::pin::Pin; /// Helper function to build a `Request`. Used in other test modules. @@ -601,7 +506,7 @@ mod awsjson_tests { use super::*; use crate::body::boxed; use futures_util::Future; - use http::{HeaderMap, HeaderValue, Method}; + use http::{HeaderMap, HeaderValue, Method, StatusCode}; use pretty_assertions::assert_eq; use std::pin::Pin; diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs new file mode 100644 index 0000000000..282a4ca87a --- /dev/null +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -0,0 +1,128 @@ +use std::convert::Infallible; + +use http::header::ToStrError; +use tower::{Layer, Service}; + +use crate::{ + body::BoxBody, + extension::RuntimeErrorExtension, + protocols::{AwsJson10, AwsJson11}, + response::IntoResponse, + routing::{tiny_map::TinyMap, Route}, +}; + +use super::Router; + +pub enum Error { + NotRootUrl, + MethodDisallowed, + MissingHeader, + InvalidHeader(ToStrError), + NotFound, +} + +impl IntoResponse for Error { + fn into_response(self) -> http::Response { + match self { + Error::MethodDisallowed => super::method_disallowed(), + _ => http::Response::builder() + .status(http::StatusCode::NOT_FOUND) + .header("Content-Type", "application/x-amz-json-1.0") + .extension(RuntimeErrorExtension::new( + super::UNKNOWN_OPERATION_EXCEPTION.to_string(), + )) + .body(crate::body::to_boxed("")) + .expect("invalid HTTP response"), + } + } +} + +impl IntoResponse for Error { + fn into_response(self) -> http::Response { + match self { + Error::MethodDisallowed => super::method_disallowed(), + _ => http::Response::builder() + .status(http::StatusCode::NOT_FOUND) + .header("Content-Type", "application/x-amz-json-1.1") + .extension(RuntimeErrorExtension::new( + super::UNKNOWN_OPERATION_EXCEPTION.to_string(), + )) + .body(crate::body::to_boxed("")) + .expect("invalid HTTP response"), + } + } +} + +// This constant determines when the `TinyMap` implementation switches from being a `Vec` to a +// `HashMap`. This is chosen to be 15 as a result of the discussion around +// https://github.com/awslabs/smithy-rs/pull/1429#issuecomment-1147516546 +const ROUTE_CUTOFF: usize = 15; + +#[derive(Debug, Clone)] +pub struct AwsJsonRouter { + routes: TinyMap, +} + +impl AwsJsonRouter { + pub fn layer(self, layer: L) -> AwsJsonRouter + where + L: Layer, + { + AwsJsonRouter { + routes: self + .routes + .into_iter() + .map(|(key, route)| (key, layer.layer(route))) + .collect(), + } + } + + pub fn boxed(self) -> AwsJsonRouter> + where + S: Service, Response = http::Response, Error = Infallible>, + S: Send + Clone + 'static, + S::Future: Send + 'static, + { + AwsJsonRouter { + routes: self.routes.into_iter().map(|(key, s)| (key, Route::new(s))).collect(), + } + } +} + +impl Router for AwsJsonRouter +where + S: Clone, +{ + type Service = S; + type Error = Error; + + fn match_route(&self, request: &http::Request) -> Result { + if request.uri() != "/" { + return Err(Error::NotRootUrl); + } + + if request.method() != http::Method::POST { + return Err(Error::MethodDisallowed); + } + + // Find the `x-amz-target` header. + let target = request.headers().get("x-amz-target").ok_or(Error::MissingHeader)?; + let target = target.to_str().map_err(Error::InvalidHeader)?; + + // Lookup in the `TinyMap` for a route for the target. + let route = self.routes.get(target).ok_or(Error::NotFound)?; + Ok(route.clone()) + } +} + +impl FromIterator<(String, S)> for AwsJsonRouter { + #[inline] + fn from_iter>(iter: T) -> Self { + Self { + routes: iter + .into_iter() + .map(|(svc, request_spec)| (svc, request_spec)) + .collect(), + } + } +} diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs new file mode 100644 index 0000000000..f7809c371f --- /dev/null +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs @@ -0,0 +1,144 @@ +use std::{ + fmt::Debug, + future::{ready, Future, Ready}, + marker::PhantomData, + pin::Pin, + task::{Context, Poll}, +}; + +use futures_util::future::Either; +use http::Response; +use tower::{util::Oneshot, Service, ServiceExt}; + +pub mod aws_json; +// pub mod merged; +pub mod rest; + +use crate::{ + body::{empty, BoxBody}, + response::IntoResponse, +}; + +const UNKNOWN_OPERATION_EXCEPTION: &str = "UnknownOperationException"; + +fn method_disallowed() -> http::Response { + http::Response::builder() + .status(http::StatusCode::METHOD_NOT_ALLOWED) + .body(empty()) + .expect("valid HTTP response") +} + +pub trait Router { + type Service; + type Error; + + fn match_route(&self, request: &http::Request) -> Result; +} + +pub struct RoutingService { + router: R, + _protocol: PhantomData

, +} + +impl Debug for RoutingService +where + R: Debug, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RoutingService") + .field("router", &self.router) + .field("_protocol", &self._protocol) + .finish() + } +} + +impl Clone for RoutingService +where + R: Clone, +{ + fn clone(&self) -> Self { + Self { + router: self.router.clone(), + _protocol: PhantomData, + } + } +} + +impl RoutingService { + pub fn new(router: R) -> Self { + Self { + router, + _protocol: PhantomData, + } + } + + pub fn map RNew>(self, f: F) -> RoutingService { + RoutingService { + router: f(self.router), + _protocol: PhantomData, + } + } +} + +type EitherOneshotReady = Either< + Oneshot>, + Ready>>::Response, >>::Error>>, +>; + +pin_project_lite::pin_project! { + pub struct RoutingFuture where S: Service> { + #[pin] + inner: EitherOneshotReady + } +} + +impl RoutingFuture +where + S: Service>, +{ + pub(super) fn from_oneshot(future: Oneshot>) -> Self { + Self { + inner: Either::Left(future), + } + } + + pub(super) fn from_response(response: S::Response) -> Self { + Self { + inner: Either::Right(ready(Ok(response))), + } + } +} + +impl Future for RoutingFuture +where + S: Service>, +{ + type Output = ::Output; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + self.project().inner.poll(cx) + } +} + +impl Service> for RoutingService +where + R: Router, + R::Service: Service, Response = http::Response> + Clone, + R::Error: IntoResponse

, +{ + type Response = Response; + type Error = >>::Error; + type Future = RoutingFuture; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: http::Request) -> Self::Future { + let result = self.router.match_route(&req); + match result { + Ok(ok) => RoutingFuture::from_oneshot(ok.oneshot(req)), + Err(err) => RoutingFuture::from_response(err.into_response()), + } + } +} diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs new file mode 100644 index 0000000000..7776a5bf03 --- /dev/null +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -0,0 +1,129 @@ +use std::convert::Infallible; + +use tower::{Layer, Service}; + +use crate::{ + body::BoxBody, + extension::RuntimeErrorExtension, + protocols::{RestJson1, RestXml1}, + response::IntoResponse, + routing::{ + request_spec::{Match, RequestSpec}, + Route, + }, +}; + +use super::Router; + +pub enum Error { + NotFound, + MethodDisallowed, +} + +impl IntoResponse for Error { + fn into_response(self) -> http::Response { + match self { + Error::NotFound => http::Response::builder() + .status(http::StatusCode::NOT_FOUND) + .header("Content-Type", "application/json") + .header("X-Amzn-Errortype", super::UNKNOWN_OPERATION_EXCEPTION) + .extension(RuntimeErrorExtension::new( + super::UNKNOWN_OPERATION_EXCEPTION.to_string(), + )) + .body(crate::body::to_boxed("{}")) + .expect("invalid HTTP response"), + Error::MethodDisallowed => super::method_disallowed(), + } + } +} + +impl IntoResponse for Error { + fn into_response(self) -> http::Response { + match self { + Error::NotFound => http::Response::builder() + .status(http::StatusCode::NOT_FOUND) + .header("Content-Type", "application/xml") + .extension(RuntimeErrorExtension::new( + super::UNKNOWN_OPERATION_EXCEPTION.to_string(), + )) + .body(crate::body::to_boxed("")) + .expect("invalid HTTP response"), + Error::MethodDisallowed => super::method_disallowed(), + } + } +} + +#[derive(Debug, Clone)] +pub struct RestRouter { + routes: Vec<(S, RequestSpec)>, +} + +impl RestRouter { + pub fn layer(self, layer: L) -> RestRouter + where + L: Layer, + { + RestRouter { + routes: self + .routes + .into_iter() + .map(|(route, request_spec)| (layer.layer(route), request_spec)) + .collect(), + } + } + + pub fn boxed(self) -> RestRouter> + where + S: Service, Response = http::Response, Error = Infallible>, + S: Send + Clone + 'static, + S::Future: Send + 'static, + { + RestRouter { + routes: self.routes.into_iter().map(|(s, spec)| (Route::new(s), spec)).collect(), + } + } +} + +impl Router for RestRouter +where + S: Clone, +{ + type Service = S; + type Error = Error; + + fn match_route(&self, request: &http::Request) -> Result { + let mut method_allowed = true; + + for (route, request_spec) in &self.routes { + match request_spec.matches(request) { + Match::Yes => return Ok(route.clone()), + Match::MethodNotAllowed => method_allowed = false, + // Continue looping to see if another route matches. + Match::No => continue, + } + } + + if method_allowed { + Err(Error::NotFound) + } else { + Err(Error::MethodDisallowed) + } + } +} + +impl FromIterator<(S, RequestSpec)> for RestRouter { + #[inline] + fn from_iter>(iter: T) -> Self { + let mut routes: Vec<(S, RequestSpec)> = iter + .into_iter() + .map(|(svc, request_spec)| (svc, request_spec)) + .collect(); + + // Sort them once by specificity, with the more specific routes sorted before the less + // specific ones, so that when routing a request we can simply iterate through the routes + // and pick the first one that matches. + routes.sort_by_key(|(_route, request_spec)| std::cmp::Reverse(request_spec.rank())); + + Self { routes } + } +} diff --git a/rust-runtime/aws-smithy-http-server/src/runtime_error.rs b/rust-runtime/aws-smithy-http-server/src/runtime_error.rs index d1bdfec230..203d0e1d56 100644 --- a/rust-runtime/aws-smithy-http-server/src/runtime_error.rs +++ b/rust-runtime/aws-smithy-http-server/src/runtime_error.rs @@ -25,8 +25,6 @@ use crate::{protocols::Protocol, response::Response}; #[derive(Debug)] pub enum RuntimeErrorKind { - /// The requested operation does not exist. - UnknownOperation, /// Request failed to deserialize or response failed to serialize. Serialization(crate::Error), /// As of writing, this variant can only occur upon failure to extract an @@ -44,7 +42,6 @@ impl RuntimeErrorKind { match self { RuntimeErrorKind::Serialization(_) => "SerializationException", RuntimeErrorKind::InternalFailure(_) => "InternalFailureException", - RuntimeErrorKind::UnknownOperation => "UnknownOperationException", RuntimeErrorKind::NotAcceptable => "NotAcceptableException", } } @@ -61,7 +58,6 @@ impl RuntimeError { let status_code = match self.kind { RuntimeErrorKind::Serialization(_) => http::StatusCode::BAD_REQUEST, RuntimeErrorKind::InternalFailure(_) => http::StatusCode::INTERNAL_SERVER_ERROR, - RuntimeErrorKind::UnknownOperation => http::StatusCode::NOT_FOUND, RuntimeErrorKind::NotAcceptable => http::StatusCode::NOT_ACCEPTABLE, }; From 17ad96af17ae4407c0d171019bc39727a7bd8fec Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 07:54:55 +0000 Subject: [PATCH 2/9] Add copyright --- .../aws-smithy-http-server/src/routing/routers/aws_json.rs | 5 +++++ .../aws-smithy-http-server/src/routing/routers/mod.rs | 5 +++++ .../aws-smithy-http-server/src/routing/routers/rest.rs | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs index 282a4ca87a..53cda0257d 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + use std::convert::Infallible; use http::header::ToStrError; diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs index f7809c371f..967e5d9e92 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + use std::{ fmt::Debug, future::{ready, Future, Ready}, diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 7776a5bf03..83eb432826 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + use std::convert::Infallible; use tower::{Layer, Service}; From 1f50577a32c48de4505fe792e4ea9f8b9d0ea4cf Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 12:33:23 +0000 Subject: [PATCH 3/9] Add documentation --- .../aws-smithy-http-server/src/protocols.rs | 12 +++--- .../aws-smithy-http-server/src/response.rs | 2 + .../aws-smithy-http-server/src/routing/mod.rs | 6 +-- .../src/routing/routers/aws_json.rs | 14 +++++++ .../src/routing/routers/mod.rs | 38 ++++++++++++------- .../src/routing/routers/rest.rs | 17 +++++++-- 6 files changed, 64 insertions(+), 25 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/protocols.rs b/rust-runtime/aws-smithy-http-server/src/protocols.rs index 8da8d05e7f..d9bae8258d 100644 --- a/rust-runtime/aws-smithy-http-server/src/protocols.rs +++ b/rust-runtime/aws-smithy-http-server/src/protocols.rs @@ -7,16 +7,16 @@ use crate::rejection::MissingContentTypeReason; use crate::request::RequestParts; -/// Rest JSON 1.0 Protocol. -pub struct RestJson1; +/// [AWS REST JSON 1.0 Protocol](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-restjson1-protocol.html). +pub struct AwsRestJson1; -/// Rest XML Protocol. -pub struct RestXml1; +/// [AWS REST XML Protocol](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-restxml-protocol.html). +pub struct AwsRestXml; -/// AWS JSON 1.0 Protocol. +/// [AWS JSON 1.0 Protocol](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-json-1_0-protocol.html). pub struct AwsJson10; -/// AWS JSON 1.1 Protocol. +/// [AWS JSON 1.1 Protocol](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-json-1_1-protocol.html). pub struct AwsJson11; /// Supported protocols. diff --git a/rust-runtime/aws-smithy-http-server/src/response.rs b/rust-runtime/aws-smithy-http-server/src/response.rs index a85b72395c..4c7bab848d 100644 --- a/rust-runtime/aws-smithy-http-server/src/response.rs +++ b/rust-runtime/aws-smithy-http-server/src/response.rs @@ -37,6 +37,8 @@ use crate::body::BoxBody; #[doc(hidden)] pub type Response = http::Response; +/// A protocol aware function taking `self` to [`http::Response`]. pub trait IntoResponse { + /// Performs a conversion into a [`http::Response`]. fn into_response(self) -> http::Response; } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs index 5ac611edde..cf746f9498 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs @@ -11,7 +11,7 @@ use self::request_spec::RequestSpec; use self::routers::{aws_json::AwsJsonRouter, rest::RestRouter, RoutingService}; use crate::body::{boxed, Body, BoxBody, HttpBody}; use crate::error::BoxError; -use crate::protocols::{AwsJson10, AwsJson11, RestJson1, RestXml1}; +use crate::protocols::{AwsJson10, AwsJson11, AwsRestJson1, AwsRestXml}; use http::{Request, Response}; use std::{ @@ -70,8 +70,8 @@ pub struct Router { /// directly found in the `X-Amz-Target` HTTP header. #[derive(Debug)] enum Routes { - RestXml(RoutingService>, RestXml1>), - RestJson1(RoutingService>, RestJson1>), + RestXml(RoutingService>, AwsRestXml>), + RestJson1(RoutingService>, AwsRestJson1>), AwsJson10(RoutingService>, AwsJson10>), AwsJson11(RoutingService>, AwsJson11>), } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs index 53cda0257d..0357ae9eef 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -18,11 +18,17 @@ use crate::{ use super::Router; +/// An AWS JSON routing error. pub enum Error { + /// Relative URI was not "/". NotRootUrl, + /// Method was not `POST`. MethodDisallowed, + /// Missing the `x-amz-target` header. MissingHeader, + /// Unable to parse header into UTF-8. InvalidHeader(ToStrError), + /// Operation not found. NotFound, } @@ -63,12 +69,17 @@ impl IntoResponse for Error { // https://github.com/awslabs/smithy-rs/pull/1429#issuecomment-1147516546 const ROUTE_CUTOFF: usize = 15; +/// A [`Router`] supporting [`AWS JSON 1.0`] and [`AWS JSON 1.1`] protocols. +/// +/// [AWS JSON 1.0]: https://awslabs.github.io/smithy/2.0/aws/protocols/aws-json-1_0-protocol.html +/// [AWS JSON 1.1]: https://awslabs.github.io/smithy/2.0/aws/protocols/aws-json-1_1-protocol.html #[derive(Debug, Clone)] pub struct AwsJsonRouter { routes: TinyMap, } impl AwsJsonRouter { + /// Applies a [`Layer`] uniformly to all routes. pub fn layer(self, layer: L) -> AwsJsonRouter where L: Layer, @@ -82,6 +93,7 @@ impl AwsJsonRouter { } } + /// Applies type erasure to the inner route using [`Route::new`]. pub fn boxed(self) -> AwsJsonRouter> where S: Service, Response = http::Response, Error = Infallible>, @@ -102,10 +114,12 @@ where type Error = Error; fn match_route(&self, request: &http::Request) -> Result { + // The URI must be root, if request.uri() != "/" { return Err(Error::NotRootUrl); } + // Only `Method::POST` is allowed. if request.method() != http::Method::POST { return Err(Error::MethodDisallowed); } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs index 967e5d9e92..1fd3dc6487 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs @@ -4,7 +4,7 @@ */ use std::{ - fmt::Debug, + fmt, future::{ready, Future, Ready}, marker::PhantomData, pin::Pin, @@ -15,15 +15,14 @@ use futures_util::future::Either; use http::Response; use tower::{util::Oneshot, Service, ServiceExt}; -pub mod aws_json; -// pub mod merged; -pub mod rest; - use crate::{ body::{empty, BoxBody}, response::IntoResponse, }; +pub mod aws_json; +pub mod rest; + const UNKNOWN_OPERATION_EXCEPTION: &str = "UnknownOperationException"; fn method_disallowed() -> http::Response { @@ -33,23 +32,28 @@ fn method_disallowed() -> http::Response { .expect("valid HTTP response") } +/// An interface for retrieving an inner [`Service`] given a [`http::Request`]. pub trait Router { type Service; type Error; + /// Matches a [`http::Request`] to a target [`Service`]. fn match_route(&self, request: &http::Request) -> Result; } -pub struct RoutingService { +/// A [`Service`] using the a [`Router`] `R` to redirect messages to specific routes. +/// +/// The `Protocol` parameter is used to determine +pub struct RoutingService { router: R, - _protocol: PhantomData

, + _protocol: PhantomData, } -impl Debug for RoutingService +impl fmt::Debug for RoutingService where - R: Debug, + R: fmt::Debug, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("RoutingService") .field("router", &self.router) .field("_protocol", &self._protocol) @@ -70,6 +74,7 @@ where } impl RoutingService { + /// Creates a [`RoutingService`] from a [`Router`]. pub fn new(router: R) -> Self { Self { router, @@ -77,7 +82,11 @@ impl RoutingService { } } - pub fn map RNew>(self, f: F) -> RoutingService { + /// Maps a [`Router`] using a closure. + pub fn map(self, f: F) -> RoutingService + where + F: FnOnce(R) -> RNew, + { RoutingService { router: f(self.router), _protocol: PhantomData, @@ -101,12 +110,14 @@ impl RoutingFuture where S: Service>, { + /// Creates a [`RoutingFuture`] from [`ServiceExt::oneshot`]. pub(super) fn from_oneshot(future: Oneshot>) -> Self { Self { inner: Either::Left(future), } } + /// Creates a [`RoutingFuture`] from [`Service::Response`]. pub(super) fn from_response(response: S::Response) -> Self { Self { inner: Either::Right(ready(Ok(response))), @@ -140,9 +151,10 @@ where } fn call(&mut self, req: http::Request) -> Self::Future { - let result = self.router.match_route(&req); - match result { + match self.router.match_route(&req) { + // Successfully routed, use the routes `Service::call`. Ok(ok) => RoutingFuture::from_oneshot(ok.oneshot(req)), + // Failed to route, use the `R::Error`s `IntoResponse

`. Err(err) => RoutingFuture::from_response(err.into_response()), } } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 83eb432826..414dd794b6 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -10,7 +10,7 @@ use tower::{Layer, Service}; use crate::{ body::BoxBody, extension::RuntimeErrorExtension, - protocols::{RestJson1, RestXml1}, + protocols::{AwsRestJson1, AwsRestXml}, response::IntoResponse, routing::{ request_spec::{Match, RequestSpec}, @@ -20,12 +20,15 @@ use crate::{ use super::Router; +/// An AWS REST routing error. pub enum Error { + /// Operation not found. NotFound, + /// Method was not allowed. MethodDisallowed, } -impl IntoResponse for Error { +impl IntoResponse for Error { fn into_response(self) -> http::Response { match self { Error::NotFound => http::Response::builder() @@ -42,7 +45,7 @@ impl IntoResponse for Error { } } -impl IntoResponse for Error { +impl IntoResponse for Error { fn into_response(self) -> http::Response { match self { Error::NotFound => http::Response::builder() @@ -58,12 +61,17 @@ impl IntoResponse for Error { } } +/// A [`Router`] supporting [`AWS REST JSON 1.0`] and [`AWS REST XML`] protocols. +/// +/// [AWS REST JSON 1.0]: https://awslabs.github.io/smithy/2.0/aws/protocols/aws-restjson1-protocol.html +/// [AWS REST XML]: https://awslabs.github.io/smithy/2.0/aws/protocols/aws-restxml-protocol.html #[derive(Debug, Clone)] pub struct RestRouter { routes: Vec<(S, RequestSpec)>, } impl RestRouter { + /// Applies a [`Layer`] uniformly to all routes. pub fn layer(self, layer: L) -> RestRouter where L: Layer, @@ -77,6 +85,7 @@ impl RestRouter { } } + /// Applies type erasure to the inner route using [`Route::new`]. pub fn boxed(self) -> RestRouter> where S: Service, Response = http::Response, Error = Infallible>, @@ -101,7 +110,9 @@ where for (route, request_spec) in &self.routes { match request_spec.matches(request) { + // Match found. Match::Yes => return Ok(route.clone()), + // Match found, but method disallowed. Match::MethodNotAllowed => method_allowed = false, // Continue looping to see if another route matches. Match::No => continue, From f76bb0e494f8d84e419e61da1228014cb81f1f6d Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 12:46:19 +0000 Subject: [PATCH 4/9] Address PR feedback --- .../src/routing/routers/aws_json.rs | 8 ++++---- .../aws-smithy-http-server/src/routing/routers/mod.rs | 2 +- .../aws-smithy-http-server/src/routing/routers/rest.rs | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs index 0357ae9eef..acc4f91be5 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -23,7 +23,7 @@ pub enum Error { /// Relative URI was not "/". NotRootUrl, /// Method was not `POST`. - MethodDisallowed, + MethodNotAllowed, /// Missing the `x-amz-target` header. MissingHeader, /// Unable to parse header into UTF-8. @@ -35,7 +35,7 @@ pub enum Error { impl IntoResponse for Error { fn into_response(self) -> http::Response { match self { - Error::MethodDisallowed => super::method_disallowed(), + Error::MethodNotAllowed => super::method_disallowed(), _ => http::Response::builder() .status(http::StatusCode::NOT_FOUND) .header("Content-Type", "application/x-amz-json-1.0") @@ -51,7 +51,7 @@ impl IntoResponse for Error { impl IntoResponse for Error { fn into_response(self) -> http::Response { match self { - Error::MethodDisallowed => super::method_disallowed(), + Error::MethodNotAllowed => super::method_disallowed(), _ => http::Response::builder() .status(http::StatusCode::NOT_FOUND) .header("Content-Type", "application/x-amz-json-1.1") @@ -121,7 +121,7 @@ where // Only `Method::POST` is allowed. if request.method() != http::Method::POST { - return Err(Error::MethodDisallowed); + return Err(Error::MethodNotAllowed); } // Find the `x-amz-target` header. diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs index 1fd3dc6487..118e641c25 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs @@ -29,7 +29,7 @@ fn method_disallowed() -> http::Response { http::Response::builder() .status(http::StatusCode::METHOD_NOT_ALLOWED) .body(empty()) - .expect("valid HTTP response") + .expect("invalid HTTP response") } /// An interface for retrieving an inner [`Service`] given a [`http::Request`]. diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 414dd794b6..1d154b3786 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -25,7 +25,7 @@ pub enum Error { /// Operation not found. NotFound, /// Method was not allowed. - MethodDisallowed, + MethodNotAllowed, } impl IntoResponse for Error { @@ -40,7 +40,7 @@ impl IntoResponse for Error { )) .body(crate::body::to_boxed("{}")) .expect("invalid HTTP response"), - Error::MethodDisallowed => super::method_disallowed(), + Error::MethodNotAllowed => super::method_disallowed(), } } } @@ -56,7 +56,7 @@ impl IntoResponse for Error { )) .body(crate::body::to_boxed("")) .expect("invalid HTTP response"), - Error::MethodDisallowed => super::method_disallowed(), + Error::MethodNotAllowed => super::method_disallowed(), } } } @@ -122,7 +122,7 @@ where if method_allowed { Err(Error::NotFound) } else { - Err(Error::MethodDisallowed) + Err(Error::MethodNotAllowed) } } } From 5eddefaa27ffc3e29ad80fee04757a5e10f8d830 Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 13:55:07 +0000 Subject: [PATCH 5/9] Add logging --- .../src/routing/routers/aws_json.rs | 7 +++++++ .../aws-smithy-http-server/src/routing/routers/mod.rs | 9 +++++++-- .../aws-smithy-http-server/src/routing/routers/rest.rs | 4 ++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs index acc4f91be5..971c8f4c86 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -6,6 +6,7 @@ use std::convert::Infallible; use http::header::ToStrError; +use thiserror::Error; use tower::{Layer, Service}; use crate::{ @@ -19,16 +20,22 @@ use crate::{ use super::Router; /// An AWS JSON routing error. +#[derive(Debug, Error)] pub enum Error { /// Relative URI was not "/". + #[error("relative URI is not \"/\"")] NotRootUrl, /// Method was not `POST`. + #[error("method not POST")] MethodNotAllowed, /// Missing the `x-amz-target` header. + #[error("missing the \"x-amz-target\" header")] MissingHeader, /// Unable to parse header into UTF-8. + #[error("failed to parse header: {0}")] InvalidHeader(ToStrError), /// Operation not found. + #[error("operation not found")] NotFound, } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs index 118e641c25..3ec825f86e 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs @@ -4,6 +4,7 @@ */ use std::{ + error::Error, fmt, future::{ready, Future, Ready}, marker::PhantomData, @@ -14,6 +15,7 @@ use std::{ use futures_util::future::Either; use http::Response; use tower::{util::Oneshot, Service, ServiceExt}; +use tracing::debug; use crate::{ body::{empty, BoxBody}, @@ -140,7 +142,7 @@ impl Service> for RoutingService where R: Router, R::Service: Service, Response = http::Response> + Clone, - R::Error: IntoResponse

, + R::Error: IntoResponse

+ Error, { type Response = Response; type Error = >>::Error; @@ -155,7 +157,10 @@ where // Successfully routed, use the routes `Service::call`. Ok(ok) => RoutingFuture::from_oneshot(ok.oneshot(req)), // Failed to route, use the `R::Error`s `IntoResponse

`. - Err(err) => RoutingFuture::from_response(err.into_response()), + Err(error) => { + debug!(%error, "failed to route"); + RoutingFuture::from_response(error.into_response()) + } } } } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 1d154b3786..5db66fa4b8 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -5,6 +5,7 @@ use std::convert::Infallible; +use thiserror::Error; use tower::{Layer, Service}; use crate::{ @@ -21,10 +22,13 @@ use crate::{ use super::Router; /// An AWS REST routing error. +#[derive(Debug, Error)] pub enum Error { /// Operation not found. + #[error("operation not found")] NotFound, /// Method was not allowed. + #[error("method was not allowed")] MethodNotAllowed, } From 242f609b82c7ee172241c748853b3a541ab0ea2b Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 14:35:09 +0000 Subject: [PATCH 6/9] Tweak error Response builder --- .../src/routing/routers/aws_json.rs | 10 +++++----- .../src/routing/routers/mod.rs | 13 +++++-------- .../src/routing/routers/rest.rs | 8 ++++---- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs index 971c8f4c86..990d61ee0d 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -10,7 +10,7 @@ use thiserror::Error; use tower::{Layer, Service}; use crate::{ - body::BoxBody, + body::{empty, BoxBody}, extension::RuntimeErrorExtension, protocols::{AwsJson10, AwsJson11}, response::IntoResponse, @@ -49,8 +49,8 @@ impl IntoResponse for Error { .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), )) - .body(crate::body::to_boxed("")) - .expect("invalid HTTP response"), + .body(empty()) + .expect("invalid HTTP response for AWS JSON routing error; please file a bug report under https://github.com/awslabs/smithy-rs/issues"), } } } @@ -65,8 +65,8 @@ impl IntoResponse for Error { .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), )) - .body(crate::body::to_boxed("")) - .expect("invalid HTTP response"), + .body(empty()) + .expect("invalid HTTP response for AWS JSON routing error; please file a bug report under https://github.com/awslabs/smithy-rs/issues"), } } } diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs index 3ec825f86e..a6d0d2fc07 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/mod.rs @@ -17,21 +17,18 @@ use http::Response; use tower::{util::Oneshot, Service, ServiceExt}; use tracing::debug; -use crate::{ - body::{empty, BoxBody}, - response::IntoResponse, -}; +use crate::{body::BoxBody, response::IntoResponse}; pub mod aws_json; pub mod rest; const UNKNOWN_OPERATION_EXCEPTION: &str = "UnknownOperationException"; +/// Constructs common response to method disallowed. fn method_disallowed() -> http::Response { - http::Response::builder() - .status(http::StatusCode::METHOD_NOT_ALLOWED) - .body(empty()) - .expect("invalid HTTP response") + let mut responses = http::Response::default(); + *responses.status_mut() = http::StatusCode::METHOD_NOT_ALLOWED; + responses } /// An interface for retrieving an inner [`Service`] given a [`http::Request`]. diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 5db66fa4b8..97be2a264d 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -9,7 +9,7 @@ use thiserror::Error; use tower::{Layer, Service}; use crate::{ - body::BoxBody, + body::{empty, BoxBody}, extension::RuntimeErrorExtension, protocols::{AwsRestJson1, AwsRestXml}, response::IntoResponse, @@ -43,7 +43,7 @@ impl IntoResponse for Error { super::UNKNOWN_OPERATION_EXCEPTION.to_string(), )) .body(crate::body::to_boxed("{}")) - .expect("invalid HTTP response"), + .expect("invalid HTTP response for REST JSON routing error; please file a bug report under https://github.com/awslabs/smithy-rs/issues"), Error::MethodNotAllowed => super::method_disallowed(), } } @@ -58,8 +58,8 @@ impl IntoResponse for Error { .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), )) - .body(crate::body::to_boxed("")) - .expect("invalid HTTP response"), + .body(empty()) + .expect("invalid HTTP response for REST JSON routing error; please file a bug report under https://github.com/awslabs/smithy-rs/issues"), Error::MethodNotAllowed => super::method_disallowed(), } } From 1ec55780d4df414e9c8a28d6e81ae602293c7e33 Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 14:47:56 +0000 Subject: [PATCH 7/9] Remove excess bound --- rust-runtime/aws-smithy-http-server/src/routing/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs index cf746f9498..79ff8966a8 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs @@ -123,12 +123,10 @@ where L::Service: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, >>::Future: Send + 'static, - NewReqBody: Send + 'static, NewResBody: HttpBody + Send + 'static, NewResBody::Error: Into, { let layer = ServiceBuilder::new() - .layer_fn(Route::new) .layer(MapResponseBodyLayer::new(boxed)) .layer(layer); match self.routes { From 1f3aba84f6bda36de22427d9c37a7d746a543ae5 Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 15:53:56 +0000 Subject: [PATCH 8/9] Switch RequestSpec and S --- .../aws-smithy-http-server/src/routing/mod.rs | 4 ++-- .../src/routing/routers/rest.rs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs index 79ff8966a8..0c65e4aff9 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/mod.rs @@ -161,7 +161,7 @@ where let svc = RoutingService::new( routes .into_iter() - .map(|(svc, request_spec)| (Route::from_box_clone_service(svc), request_spec)) + .map(|(svc, request_spec)| (request_spec, Route::from_box_clone_service(svc))) .collect(), ); Self { @@ -185,7 +185,7 @@ where let svc = RoutingService::new( routes .into_iter() - .map(|(svc, request_spec)| (Route::from_box_clone_service(svc), request_spec)) + .map(|(svc, request_spec)| (request_spec, Route::from_box_clone_service(svc))) .collect(), ); Self { diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 97be2a264d..8f673fdf6d 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -71,7 +71,7 @@ impl IntoResponse for Error { /// [AWS REST XML]: https://awslabs.github.io/smithy/2.0/aws/protocols/aws-restxml-protocol.html #[derive(Debug, Clone)] pub struct RestRouter { - routes: Vec<(S, RequestSpec)>, + routes: Vec<(RequestSpec, S)>, } impl RestRouter { @@ -84,7 +84,7 @@ impl RestRouter { routes: self .routes .into_iter() - .map(|(route, request_spec)| (layer.layer(route), request_spec)) + .map(|(request_spec, route)| (request_spec, layer.layer(route))) .collect(), } } @@ -97,7 +97,7 @@ impl RestRouter { S::Future: Send + 'static, { RestRouter { - routes: self.routes.into_iter().map(|(s, spec)| (Route::new(s), spec)).collect(), + routes: self.routes.into_iter().map(|(spec, s)| (spec, Route::new(s))).collect(), } } } @@ -112,7 +112,7 @@ where fn match_route(&self, request: &http::Request) -> Result { let mut method_allowed = true; - for (route, request_spec) in &self.routes { + for (request_spec, route) in &self.routes { match request_spec.matches(request) { // Match found. Match::Yes => return Ok(route.clone()), @@ -131,18 +131,18 @@ where } } -impl FromIterator<(S, RequestSpec)> for RestRouter { +impl FromIterator<(RequestSpec, S)> for RestRouter { #[inline] - fn from_iter>(iter: T) -> Self { - let mut routes: Vec<(S, RequestSpec)> = iter + fn from_iter>(iter: T) -> Self { + let mut routes: Vec<(RequestSpec, S)> = iter .into_iter() - .map(|(svc, request_spec)| (svc, request_spec)) + .map(|(request_spec, svc)| (request_spec, svc)) .collect(); // Sort them once by specificity, with the more specific routes sorted before the less // specific ones, so that when routing a request we can simply iterate through the routes // and pick the first one that matches. - routes.sort_by_key(|(_route, request_spec)| std::cmp::Reverse(request_spec.rank())); + routes.sort_by_key(|(request_spec, _route)| std::cmp::Reverse(request_spec.rank())); Self { routes } } From 2f9a02f3719295fa924c28c274674f7535f6ea2a Mon Sep 17 00:00:00 2001 From: Harry Barber Date: Thu, 25 Aug 2022 18:34:24 +0000 Subject: [PATCH 9/9] Use const CONTENT_TYPE --- .../aws-smithy-http-server/src/routing/routers/aws_json.rs | 4 ++-- .../aws-smithy-http-server/src/routing/routers/rest.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs index 990d61ee0d..eb83d0c178 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs @@ -45,7 +45,7 @@ impl IntoResponse for Error { Error::MethodNotAllowed => super::method_disallowed(), _ => http::Response::builder() .status(http::StatusCode::NOT_FOUND) - .header("Content-Type", "application/x-amz-json-1.0") + .header(http::header::CONTENT_TYPE, "application/x-amz-json-1.0") .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), )) @@ -61,7 +61,7 @@ impl IntoResponse for Error { Error::MethodNotAllowed => super::method_disallowed(), _ => http::Response::builder() .status(http::StatusCode::NOT_FOUND) - .header("Content-Type", "application/x-amz-json-1.1") + .header(http::header::CONTENT_TYPE, "application/x-amz-json-1.1") .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), )) diff --git a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs index 8f673fdf6d..53d16de55f 100644 --- a/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs +++ b/rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs @@ -37,7 +37,7 @@ impl IntoResponse for Error { match self { Error::NotFound => http::Response::builder() .status(http::StatusCode::NOT_FOUND) - .header("Content-Type", "application/json") + .header(http::header::CONTENT_TYPE, "application/json") .header("X-Amzn-Errortype", super::UNKNOWN_OPERATION_EXCEPTION) .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), @@ -54,7 +54,7 @@ impl IntoResponse for Error { match self { Error::NotFound => http::Response::builder() .status(http::StatusCode::NOT_FOUND) - .header("Content-Type", "application/xml") + .header(http::header::CONTENT_TYPE, "application/xml") .extension(RuntimeErrorExtension::new( super::UNKNOWN_OPERATION_EXCEPTION.to_string(), ))