Skip to content

Commit

Permalink
Fix presigned put bug with flex checksums (#3971)
Browse files Browse the repository at this point in the history
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
Fixing bug reported in
awslabs/aws-sdk-rust#1240 (comment)

## Description
<!--- Describe your changes in detail -->

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Updated presigning integ tests to account for behavior specified in SEP,
added new test for the user provided checksum case

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] For changes to the AWS SDK, generated SDK code, or SDK runtime
crates, I have created a changelog entry Markdown file in the
`.changelog` directory, specifying "aws-sdk-rust" in the `applies_to`
key.

----

_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
landonxjames authored Jan 17, 2025
1 parent 41ff31b commit 733eab7
Show file tree
Hide file tree
Showing 5 changed files with 36,312 additions and 34,111 deletions.
10 changes: 10 additions & 0 deletions .changelog/presigning-bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
applies_to: ["aws-sdk-rust"]
authors: ["landonxjames"]
references: ["aws-sdk-rust#1240"]
breaking: false
new_feature: false
bug_fix: true
---

Fix bug with presigned requests introduced by new flexibile checksums functionality
24 changes: 11 additions & 13 deletions aws/rust-runtime/aws-inlineable/src/http_request_checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,24 +193,22 @@ where
.load::<RequestChecksumCalculation>()
.unwrap_or(&RequestChecksumCalculation::WhenSupported);

// Determine if we actually calculate the checksum. If the user setting is WhenSupported (the default)
// we always calculate it (because this interceptor isn't added if it isn't supported). If it is
// WhenRequired we only calculate it if the checksum is marked required on the trait.
let calculate_checksum = match request_checksum_calculation {
RequestChecksumCalculation::WhenRequired => request_checksum_required,
RequestChecksumCalculation::WhenSupported => true,
// Need to know if this is a presigned req because we do not calculate checksums for those.
let is_presigned_req = cfg.load::<PresigningMarker>().is_some();

// Determine if we actually calculate the checksum. If this is a presigned request we do not
// If the user setting is WhenSupported (the default) we always calculate it (because this interceptor
// isn't added if it isn't supported). If it is WhenRequired we only calculate it if the checksum
// is marked required on the trait.
let calculate_checksum = match (request_checksum_calculation, is_presigned_req) {
(_, true) => false,
(RequestChecksumCalculation::WhenRequired, false) => request_checksum_required,
(RequestChecksumCalculation::WhenSupported, false) => true,
_ => true,
};

// Calculate the checksum if necessary
if calculate_checksum {
let is_presigned_req = cfg.load::<PresigningMarker>().is_some();

// If this is a presigned request and the user has not set a checksum we short circuit
if is_presigned_req && checksum_algorithm.is_none() {
return Ok(());
}

// If a checksum override is set in the ConfigBag we use that instead (currently only used by S3Express)
// If we have made it this far without a checksum being set we set the default (currently Crc32)
let checksum_algorithm =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ private fun HttpChecksumTrait.checksumAlgorithmToStr(
// Checksums are required but a user can't set one, so we set crc32 for them
rust("""let checksum_algorithm = Some("crc32");""")
} else {
// Use checksum algo set by user or crc32 if one has not been set
rust("""let checksum_algorithm = checksum_algorithm.map(|algorithm| algorithm.as_str()).or(Some("crc32"));""")
// Use checksum algo set by user
rust("""let checksum_algorithm = checksum_algorithm.map(|algorithm| algorithm.as_str());""")
}

// If a request checksum is not required and there's no way to set one, do nothing
Expand Down Expand Up @@ -205,17 +205,22 @@ class HttpRequestChecksumCustomization(
// From the httpChecksum trait
let http_checksum_required = $requestChecksumRequired;
let is_presigned_req = cfg.load::<#{PresigningMarker}>().is_some();
// If the request is presigned we do not set a default.
// If the RequestChecksumCalculation is WhenSupported and the user has not set a checksum value or algo
// we default to Crc32. If it is WhenRequired and a checksum is required by the trait and the user has not
// set a checksum value or algo we also set the default. In all other cases we do nothing.
match (
request_checksum_calculation,
http_checksum_required,
user_set_checksum_value,
user_set_checksum_algo
user_set_checksum_algo,
is_presigned_req,
) {
(#{RequestChecksumCalculation}::WhenSupported, _, false, false)
| (#{RequestChecksumCalculation}::WhenRequired, true, false, false) => {
(_, _, _, _, true) => {}
(#{RequestChecksumCalculation}::WhenSupported, _, false, false, _)
| (#{RequestChecksumCalculation}::WhenRequired, true, false, false, _) => {
request.headers_mut().insert(${requestAlgoHeader.dq()}, "CRC32");
}
_ => {},
Expand Down Expand Up @@ -254,6 +259,7 @@ class HttpRequestChecksumCustomization(
codegenContext.model,
),
),
"PresigningMarker" to AwsRuntimeType.presigning().resolve("PresigningMarker"),
)
}
}
Expand Down
Loading

0 comments on commit 733eab7

Please sign in to comment.