diff --git a/.changelog/1722544572.md b/.changelog/1722544572.md new file mode 100644 index 0000000000..a3d00cb34f --- /dev/null +++ b/.changelog/1722544572.md @@ -0,0 +1,12 @@ +--- +applies_to: +- client +authors: +- Velfi +references: +- smithy-rs#3779 +breaking: false +new_feature: false +bug_fix: false +--- +Improve error messaging when HTTP headers aren't valid UTF-8 diff --git a/rust-runtime/aws-smithy-runtime-api/Cargo.toml b/rust-runtime/aws-smithy-runtime-api/Cargo.toml index 2b2761d800..45f8df686d 100644 --- a/rust-runtime/aws-smithy-runtime-api/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime-api/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "aws-smithy-runtime-api" -version = "1.7.1" +version = "1.7.2" authors = ["AWS Rust SDK Team ", "Zelda Hessler "] description = "Smithy runtime types." edition = "2021" diff --git a/rust-runtime/aws-smithy-runtime-api/src/http/error.rs b/rust-runtime/aws-smithy-runtime-api/src/http/error.rs index 97c946d7d6..68b2c0cf6c 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/http/error.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/http/error.rs @@ -16,48 +16,140 @@ use std::str::Utf8Error; /// An error occurred constructing an Http Request. /// /// This is normally due to configuration issues, internal SDK bugs, or other user error. -pub struct HttpError(BoxError); +pub struct HttpError { + kind: Kind, + source: Option, +} -impl HttpError { - // TODO(httpRefactor): Add better error internals - pub(super) fn new>>(err: E) -> Self { - HttpError(err.into()) +#[derive(Debug)] +enum Kind { + InvalidExtensions, + InvalidHeaderName, + InvalidHeaderValue, + InvalidStatusCode, + InvalidUri, + InvalidUriParts, + MissingAuthority, + MissingScheme, + NonUtf8Header(NonUtf8Header), +} + +#[derive(Debug)] +pub(super) struct NonUtf8Header { + error: Utf8Error, + value: Vec, + name: Option, +} + +impl NonUtf8Header { + #[cfg(any(feature = "http-1x", feature = "http-02x"))] + pub(super) fn new(name: String, value: Vec, error: Utf8Error) -> Self { + Self { + error, + value, + name: Some(name), + } + } + + pub(super) fn new_missing_name(value: Vec, error: Utf8Error) -> Self { + Self { + error, + value, + name: None, + } } +} - #[allow(dead_code)] +impl HttpError { pub(super) fn invalid_extensions() -> Self { - Self("Extensions were provided during initialization. This prevents the request format from being converted.".into()) + Self { + kind: Kind::InvalidExtensions, + source: None, + } } - pub(super) fn invalid_header_value(err: InvalidHeaderValue) -> Self { - Self(err.into()) + pub(super) fn invalid_header_name(err: InvalidHeaderName) -> Self { + Self { + kind: Kind::InvalidHeaderName, + source: Some(Box::new(err)), + } } - pub(super) fn header_was_not_a_string(err: Utf8Error) -> Self { - Self(err.into()) + pub(super) fn invalid_header_value(err: InvalidHeaderValue) -> Self { + Self { + kind: Kind::InvalidHeaderValue, + source: Some(Box::new(err)), + } } - pub(super) fn invalid_header_name(err: InvalidHeaderName) -> Self { - Self(err.into()) + pub(super) fn invalid_status_code() -> Self { + Self { + kind: Kind::InvalidStatusCode, + source: None, + } } pub(super) fn invalid_uri(err: InvalidUri) -> Self { - Self(err.into()) + Self { + kind: Kind::InvalidUri, + source: Some(Box::new(err)), + } } - pub(super) fn invalid_status_code() -> Self { - Self("invalid HTTP status code".into()) + pub(super) fn invalid_uri_parts(err: http_02x::Error) -> Self { + Self { + kind: Kind::InvalidUriParts, + source: Some(Box::new(err)), + } + } + + pub(super) fn missing_authority() -> Self { + Self { + kind: Kind::MissingAuthority, + source: None, + } + } + + pub(super) fn missing_scheme() -> Self { + Self { + kind: Kind::MissingScheme, + source: None, + } + } + + pub(super) fn non_utf8_header(non_utf8_header: NonUtf8Header) -> Self { + Self { + kind: Kind::NonUtf8Header(non_utf8_header), + source: None, + } } } impl Display for HttpError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "an error occurred creating an HTTP Request") + use Kind::*; + match &self.kind { + InvalidExtensions => write!(f, "Extensions were provided during initialization. This prevents the request format from being converted."), + InvalidHeaderName => write!(f, "invalid header name"), + InvalidHeaderValue => write!(f, "invalid header value"), + InvalidStatusCode => write!(f, "invalid HTTP status code"), + InvalidUri => write!(f, "endpoint is not a valid URI"), + InvalidUriParts => write!(f, "endpoint parts are not valid"), + MissingAuthority => write!(f, "endpoint must contain authority"), + MissingScheme => write!(f, "endpoint must contain scheme"), + NonUtf8Header(hv) => { + // In some cases, we won't know the key so we default to "". + let key = hv.name.as_deref().unwrap_or(""); + let value = String::from_utf8_lossy(&hv.value); + let index = hv.error.valid_up_to(); + write!(f, "header `{key}={value}` contains non-UTF8 octet at index {index}") + }, + } } } impl Error for HttpError { fn source(&self) -> Option<&(dyn Error + 'static)> { - Some(self.0.as_ref()) + self.source.as_ref().map(|err| err.as_ref() as _) } } diff --git a/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs b/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs index 79ab2668b1..98e571b8c2 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs @@ -5,7 +5,7 @@ //! Types for HTTP headers -use crate::http::error::HttpError; +use crate::http::error::{HttpError, NonUtf8Header}; use std::borrow::Cow; use std::fmt::Debug; use std::str::FromStr; @@ -181,12 +181,12 @@ impl TryFrom for Headers { type Error = HttpError; fn try_from(value: http_02x::HeaderMap) -> Result { - if let Some(e) = value - .values() - .filter_map(|value| std::str::from_utf8(value.as_bytes()).err()) - .next() - { - Err(HttpError::header_was_not_a_string(e)) + if let Some(utf8_error) = value.iter().find_map(|(k, v)| { + std::str::from_utf8(v.as_bytes()) + .err() + .map(|err| NonUtf8Header::new(k.as_str().to_owned(), v.as_bytes().to_vec(), err)) + }) { + Err(HttpError::non_utf8_header(utf8_error)) } else { let mut string_safe_headers: http_02x::HeaderMap = Default::default(); string_safe_headers.extend( @@ -206,12 +206,12 @@ impl TryFrom for Headers { type Error = HttpError; fn try_from(value: http_1x::HeaderMap) -> Result { - if let Some(e) = value - .values() - .filter_map(|value| std::str::from_utf8(value.as_bytes()).err()) - .next() - { - Err(HttpError::header_was_not_a_string(e)) + if let Some(utf8_error) = value.iter().find_map(|(k, v)| { + std::str::from_utf8(v.as_bytes()) + .err() + .map(|err| NonUtf8Header::new(k.as_str().to_owned(), v.as_bytes().to_vec(), err)) + }) { + Err(HttpError::non_utf8_header(utf8_error)) } else { let mut string_safe_headers: http_02x::HeaderMap = Default::default(); string_safe_headers.extend(value.into_iter().map(|(k, v)| { @@ -285,13 +285,23 @@ mod sealed { fn into_maybe_static(self) -> Result { Ok(Cow::Owned( std::str::from_utf8(self.as_bytes()) - .map_err(HttpError::header_was_not_a_string)? + .map_err(|err| { + HttpError::non_utf8_header(NonUtf8Header::new_missing_name( + self.as_bytes().to_vec(), + err, + )) + })? .to_string(), )) } fn as_str(&self) -> Result<&str, HttpError> { - std::str::from_utf8(self.as_bytes()).map_err(HttpError::header_was_not_a_string) + std::str::from_utf8(self.as_bytes()).map_err(|err| { + HttpError::non_utf8_header(NonUtf8Header::new_missing_name( + self.as_bytes().to_vec(), + err, + )) + }) } } @@ -315,7 +325,6 @@ mod sealed { mod header_value { use super::*; - use std::str::Utf8Error; /// HeaderValue type /// @@ -334,16 +343,26 @@ mod header_value { impl HeaderValue { #[allow(dead_code)] - pub(crate) fn from_http02x(value: http_02x::HeaderValue) -> Result { - let _ = std::str::from_utf8(value.as_bytes())?; + pub(crate) fn from_http02x(value: http_02x::HeaderValue) -> Result { + let _ = std::str::from_utf8(value.as_bytes()).map_err(|err| { + HttpError::non_utf8_header(NonUtf8Header::new_missing_name( + value.as_bytes().to_vec(), + err, + )) + })?; Ok(Self { _private: Inner::H0(value), }) } #[allow(dead_code)] - pub(crate) fn from_http1x(value: http_1x::HeaderValue) -> Result { - let _ = std::str::from_utf8(value.as_bytes())?; + pub(crate) fn from_http1x(value: http_1x::HeaderValue) -> Result { + let _ = std::str::from_utf8(value.as_bytes()).map_err(|err| { + HttpError::non_utf8_header(NonUtf8Header::new_missing_name( + value.as_bytes().to_vec(), + err, + )) + })?; Ok(Self { _private: Inner::H1(value), }) @@ -426,7 +445,7 @@ fn header_name( Cow::Borrowed(s) if panic_safe => { http_02x::HeaderName::try_from(s).map_err(HttpError::invalid_header_name) } - Cow::Borrowed(staticc) => Ok(http_02x::HeaderName::from_static(staticc)), + Cow::Borrowed(static_s) => Ok(http_02x::HeaderName::from_static(static_s)), Cow::Owned(s) => { http_02x::HeaderName::try_from(s).map_err(HttpError::invalid_header_name) } @@ -445,7 +464,7 @@ fn header_value(value: MaybeStatic, panic_safe: bool) -> Result