Skip to content

Commit

Permalink
Create the HTTP Response wrapper types (#3148)
Browse files Browse the repository at this point in the history
For the same reason that the request wrapper types were created in
#3059, this PR establishes the response wrapper types and refactors
existing code to use them.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
jdisanti authored Nov 11, 2023
1 parent 4a6b06a commit 7eb008c
Show file tree
Hide file tree
Showing 96 changed files with 1,039 additions and 560 deletions.
12 changes: 9 additions & 3 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,16 @@ references = ["smithy-rs#3164"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "utkarshgupta137"

[[aws-sdk-rust]]
message = "[Upgrade guidance for HTTP Request/Response changes](https://github.com/awslabs/aws-sdk-rust/discussions/950). HTTP request types moved, and a new HTTP response type was added."
references = ["smithy-rs#3138", "smithy-rs#3148"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
author = "jdisanti"

[[smithy-rs]]
message = "The HTTP `Request`, `Response`, `Headers`, and `HeaderValue` types have been moved from `aws_smithy_runtime_api::client::http::*` into `aws_smithy_runtime_api::http`"
references = ["smithy-rs#3138"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
message = "[Upgrade guidance for HTTP Request/Response changes](https://github.com/awslabs/smithy-rs/discussions/3154). HTTP request types moved, and a new HTTP response type was added."
references = ["smithy-rs#3138", "smithy-rs#3148"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all" }
author = "jdisanti"

[[smithy-rs]]
Expand Down
3 changes: 2 additions & 1 deletion aws/rust-runtime/aws-config/external-types.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ allowed_external_types = [
"aws_smithy_async::time::TimeSource",
"aws_smithy_http::endpoint",
"aws_smithy_http::endpoint::error::InvalidEndpointError",
"aws_smithy_runtime_api::client::result::SdkError",
"aws_smithy_runtime::client::identity::cache::IdentityCache",
"aws_smithy_runtime::client::identity::cache::lazy::LazyCacheBuilder",
"aws_smithy_runtime_api::client::dns::ResolveDns",
Expand All @@ -23,6 +22,8 @@ allowed_external_types = [
"aws_smithy_runtime_api::client::http::SharedHttpClient",
"aws_smithy_runtime_api::client::identity::ResolveCachedIdentity",
"aws_smithy_runtime_api::client::identity::ResolveIdentity",
"aws_smithy_runtime_api::client::orchestrator::HttpResponse",
"aws_smithy_runtime_api::client::result::SdkError",
"aws_smithy_types::body::SdkBody",
"aws_smithy_types::retry",
"aws_smithy_types::retry::*",
Expand Down
4 changes: 2 additions & 2 deletions aws/rust-runtime/aws-config/src/http_credential_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use aws_smithy_types::config_bag::Layer;
use aws_smithy_types::retry::RetryConfig;
use aws_smithy_types::timeout::TimeoutConfig;
use http::header::{ACCEPT, AUTHORIZATION};
use http::{HeaderValue, Response};
use http::HeaderValue;
use std::time::Duration;

const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(5);
Expand Down Expand Up @@ -149,7 +149,7 @@ impl Builder {

fn parse_response(
provider_name: &'static str,
response: &Response<SdkBody>,
response: &HttpResponse,
) -> Result<Credentials, OrchestratorError<CredentialsError>> {
if !response.status().is_success() {
return Err(OrchestratorError::operation(
Expand Down
24 changes: 15 additions & 9 deletions aws/rust-runtime/aws-config/src/imds/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,11 +636,14 @@ pub(crate) mod test {
}

pub(crate) fn token_response(ttl: u32, token: &'static str) -> HttpResponse {
http::Response::builder()
.status(200)
.header("X-aws-ec2-metadata-token-ttl-seconds", ttl)
.body(SdkBody::from(token))
.unwrap()
HttpResponse::try_from(
http::Response::builder()
.status(200)
.header("X-aws-ec2-metadata-token-ttl-seconds", ttl)
.body(SdkBody::from(token))
.unwrap(),
)
.unwrap()
}

pub(crate) fn imds_request(path: &'static str, token: &str) -> HttpRequest {
Expand All @@ -655,10 +658,13 @@ pub(crate) mod test {
}

pub(crate) fn imds_response(body: &'static str) -> HttpResponse {
http::Response::builder()
.status(200)
.body(SdkBody::from(body))
.unwrap()
HttpResponse::try_from(
http::Response::builder()
.status(200)
.body(SdkBody::from(body))
.unwrap(),
)
.unwrap()
}

pub(crate) fn make_imds_client(http_client: &StaticReplayClient) -> super::Client {
Expand Down
7 changes: 3 additions & 4 deletions aws/rust-runtime/aws-config/src/imds/client/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use aws_smithy_http::endpoint::error::InvalidEndpointError;
use aws_smithy_runtime_api::client::orchestrator::HttpResponse;
use aws_smithy_runtime_api::client::result::SdkError;
use aws_smithy_types::body::SdkBody;
use std::error::Error;
use std::fmt;

Expand All @@ -32,12 +31,12 @@ impl FailedToLoadToken {
/// Error context for [`ImdsError::ErrorResponse`]
#[derive(Debug)]
pub struct ErrorResponse {
raw: http::Response<SdkBody>,
raw: HttpResponse,
}

impl ErrorResponse {
/// Returns the raw response from IMDS
pub fn response(&self) -> &http::Response<SdkBody> {
pub fn response(&self) -> &HttpResponse {
&self.raw
}
}
Expand Down Expand Up @@ -81,7 +80,7 @@ impl ImdsError {
Self::FailedToLoadToken(FailedToLoadToken { source })
}

pub(super) fn error_response(raw: http::Response<SdkBody>) -> Self {
pub(super) fn error_response(raw: HttpResponse) -> Self {
Self::ErrorResponse(ErrorResponse { raw })
}

Expand Down
2 changes: 0 additions & 2 deletions aws/rust-runtime/aws-config/src/imds/client/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,6 @@ fn parse_token_response(response: &HttpResponse) -> Result<TtlToken, TokenError>
.headers()
.get(X_AWS_EC2_METADATA_TOKEN_TTL_SECONDS)
.ok_or(TokenErrorKind::NoTtl)?
.to_str()
.map_err(|_| TokenErrorKind::InvalidTtl)?
.parse()
.map_err(|_parse_error| TokenErrorKind::InvalidTtl)?;
Ok(TtlToken {
Expand Down
2 changes: 2 additions & 0 deletions aws/rust-runtime/aws-http/external-types.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
allowed_external_types = [
"aws_smithy_runtime_api::http::headers::Headers",
"aws_smithy_types::body::Error",
"aws_smithy_types::config_bag::storable::Storable",
"aws_smithy_types::config_bag::storable::StoreReplace",
"aws_smithy_types::error::metadata::Builder",
"aws_types::app_name::AppName",
"aws_types::os_shim_internal::Env",
"bytes::bytes::Bytes",
Expand Down
66 changes: 30 additions & 36 deletions aws/rust-runtime/aws-http/src/request_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
*/

use aws_smithy_http::http::HttpHeaders;
use aws_smithy_runtime_api::client::orchestrator::HttpResponse;
use aws_smithy_runtime_api::client::result::SdkError;
use aws_smithy_runtime_api::http::Headers;
use aws_smithy_types::error::metadata::{
Builder as ErrorMetadataBuilder, ErrorMetadata, ProvideErrorMetadata,
};
use aws_smithy_types::error::Unhandled;
use http::{HeaderMap, HeaderValue};

/// Constant for the [`ErrorMetadata`] extra field that contains the request ID
const AWS_REQUEST_ID: &str = "aws_request_id";
Expand All @@ -26,8 +27,8 @@ where
{
fn request_id(&self) -> Option<&str> {
match self {
Self::ResponseError(err) => extract_request_id(err.raw().http_headers()),
Self::ServiceError(err) => extract_request_id(err.raw().http_headers()),
Self::ResponseError(err) => err.raw().http_headers().request_id(),
Self::ServiceError(err) => err.raw().http_headers().request_id(),
_ => None,
}
}
Expand All @@ -45,15 +46,16 @@ impl RequestId for Unhandled {
}
}

impl<B> RequestId for http::Response<B> {
impl RequestId for HttpResponse {
fn request_id(&self) -> Option<&str> {
extract_request_id(self.headers())
self.headers().request_id()
}
}

impl RequestId for HeaderMap {
impl RequestId for Headers {
fn request_id(&self) -> Option<&str> {
extract_request_id(self)
self.get("x-amzn-requestid")
.or(self.get("x-amz-request-id"))
}
}

Expand All @@ -71,43 +73,35 @@ where
}

/// Applies a request ID to a generic error builder
#[doc(hidden)]
pub fn apply_request_id(
builder: ErrorMetadataBuilder,
headers: &HeaderMap<HeaderValue>,
) -> ErrorMetadataBuilder {
if let Some(request_id) = extract_request_id(headers) {
pub fn apply_request_id(builder: ErrorMetadataBuilder, headers: &Headers) -> ErrorMetadataBuilder {
if let Some(request_id) = headers.request_id() {
builder.custom(AWS_REQUEST_ID, request_id)
} else {
builder
}
}

/// Extracts a request ID from HTTP response headers
fn extract_request_id(headers: &HeaderMap<HeaderValue>) -> Option<&str> {
headers
.get("x-amzn-requestid")
.or_else(|| headers.get("x-amz-request-id"))
.and_then(|value| value.to_str().ok())
}

#[cfg(test)]
mod tests {
use super::*;
use aws_smithy_types::body::SdkBody;
use http::Response;
use http::{HeaderValue, Response};

#[test]
fn test_request_id_sdk_error() {
let without_request_id = || Response::builder().body(SdkBody::empty()).unwrap();
let without_request_id =
|| HttpResponse::try_from(Response::builder().body(SdkBody::empty()).unwrap()).unwrap();
let with_request_id = || {
Response::builder()
.header(
"x-amzn-requestid",
HeaderValue::from_static("some-request-id"),
)
.body(SdkBody::empty())
.unwrap()
HttpResponse::try_from(
Response::builder()
.header(
"x-amzn-requestid",
HeaderValue::from_static("some-request-id"),
)
.body(SdkBody::empty())
.unwrap(),
)
.unwrap()
};
assert_eq!(
None,
Expand All @@ -129,28 +123,28 @@ mod tests {

#[test]
fn test_extract_request_id() {
let mut headers = HeaderMap::new();
assert_eq!(None, extract_request_id(&headers));
let mut headers = Headers::new();
assert_eq!(None, headers.request_id());

headers.append(
"x-amzn-requestid",
HeaderValue::from_static("some-request-id"),
);
assert_eq!(Some("some-request-id"), extract_request_id(&headers));
assert_eq!(Some("some-request-id"), headers.request_id());

headers.append(
"x-amz-request-id",
HeaderValue::from_static("other-request-id"),
);
assert_eq!(Some("some-request-id"), extract_request_id(&headers));
assert_eq!(Some("some-request-id"), headers.request_id());

headers.remove("x-amzn-requestid");
assert_eq!(Some("other-request-id"), extract_request_id(&headers));
assert_eq!(Some("other-request-id"), headers.request_id());
}

#[test]
fn test_apply_request_id() {
let mut headers = HeaderMap::new();
let mut headers = Headers::new();
assert_eq!(
ErrorMetadata::builder().build(),
apply_request_id(ErrorMetadata::builder(), &headers).build(),
Expand Down
10 changes: 3 additions & 7 deletions aws/rust-runtime/aws-inlineable/src/http_response_checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ use aws_smithy_runtime_api::client::interceptors::context::{
};
use aws_smithy_runtime_api::client::interceptors::Intercept;
use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents;
use aws_smithy_runtime_api::http::Headers;
use aws_smithy_types::body::SdkBody;
use aws_smithy_types::config_bag::{ConfigBag, Layer, Storable, StoreReplace};
use http::HeaderValue;
use std::{fmt, mem};

#[derive(Debug)]
Expand Down Expand Up @@ -131,7 +131,7 @@ pub(crate) fn wrap_body_with_checksum_validator(
/// If no checksum header is set, return `None`. If multiple checksum headers are set, the one that
/// is fastest to compute will be chosen.
pub(crate) fn check_headers_for_precalculated_checksum(
headers: &http::HeaderMap<HeaderValue>,
headers: &Headers,
response_algorithms: &[&str],
) -> Option<(ChecksumAlgorithm, bytes::Bytes)> {
let checksum_algorithms_to_check =
Expand All @@ -154,13 +154,9 @@ pub(crate) fn check_headers_for_precalculated_checksum(
let checksum_algorithm: ChecksumAlgorithm = checksum_algorithm.parse().expect(
"CHECKSUM_ALGORITHMS_IN_PRIORITY_ORDER only contains valid checksum algorithm names",
);
if let Some(precalculated_checksum) =
if let Some(base64_encoded_precalculated_checksum) =
headers.get(checksum_algorithm.into_impl().header_name())
{
let base64_encoded_precalculated_checksum = precalculated_checksum
.to_str()
.expect("base64 uses ASCII characters");

// S3 needs special handling for checksums of objects uploaded with `MultiPartUpload`.
if is_part_level_checksum(base64_encoded_precalculated_checksum) {
tracing::warn!(
Expand Down
4 changes: 2 additions & 2 deletions aws/rust-runtime/aws-inlineable/src/presigning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl PresignedRequest {
let _ = http_request
.try_clone()
.expect("must be cloneable, body is empty")
.into_http02x()?;
.try_into_http02x()?;
Ok(Self { http_request })
}

Expand Down Expand Up @@ -232,7 +232,7 @@ impl PresignedRequest {
/// Converts this `PresignedRequest` directly into an `http` request.
pub fn into_http_02x_request<B>(self, body: B) -> http::Request<B> {
self.http_request
.into_http02x()
.try_into_http02x()
.expect("constructor validated convertibility")
.map(|_req| body)
}
Expand Down
Loading

0 comments on commit 7eb008c

Please sign in to comment.