Skip to content

Commit

Permalink
Fix: Only enforce content length for GET requests (#3657)
Browse files Browse the repository at this point in the history
_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: Zelda Hessler <zhessler@amazon.com>
Co-authored-by: ysaito1001 <awsaito@amazon.com>
  • Loading branch information
3 people authored May 22, 2024
1 parent ccec237 commit db89652
Show file tree
Hide file tree
Showing 9 changed files with 555 additions and 6 deletions.
14 changes: 13 additions & 1 deletion CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,16 @@
# message = "Fix typos in module documentation for generated crates"
# references = ["smithy-rs#920"]
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
# author = "rcoh"
# author = "rcoh"

[[aws-sdk-rust]]
message = "Fix the Content-Length enforcement so it is only applied to GET requests."
references = ["smithy-rs#3656", "smithy-rs#3657"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
authors = ["rcoh", "Velfi"]

[[smithy-rs]]
message = "Fix the Content-Length enforcement so it is only applied to GET requests."
references = ["smithy-rs#3656", "smithy-rs#3657"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }
authors = ["rcoh", "Velfi"]
2 changes: 1 addition & 1 deletion aws/sdk/integration-tests/s3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ aws-sdk-s3 = { path = "../../build/aws-sdk/sdk/s3", features = ["test-util", "be
aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async", features = ["test-util", "rt-tokio"] }
aws-smithy-http = { path = "../../build/aws-sdk/sdk/aws-smithy-http" }
aws-smithy-protocol-test = { path = "../../build/aws-sdk/sdk/aws-smithy-protocol-test" }
aws-smithy-runtime = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime", features = ["test-util", "wire-mock"] }
aws-smithy-runtime = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime", features = ["test-util", "wire-mock", "tls-rustls"] }
aws-smithy-runtime-api = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime-api", features = ["test-util"] }
aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" }
aws-smithy-experimental = { path = "../../build/aws-sdk/sdk/aws-smithy-experimental", features = ["crypto-ring"] }
Expand Down
99 changes: 99 additions & 0 deletions aws/sdk/integration-tests/s3/tests/content-length-enforcement.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

use aws_sdk_s3::{config::Region, error::DisplayErrorContext, Client, Config};
use aws_smithy_runtime::client::http::test_util::dvr::ReplayingClient;

#[tokio::test]
async fn test_content_length_enforcement_is_not_applied_to_head_request() {
let http_client =
ReplayingClient::from_file("tests/data/content-length-enforcement/head-object.json")
.expect("recorded HTTP communication exists");
let config = Config::builder()
.with_test_defaults()
.http_client(http_client.clone())
.region(Region::new("us-east-1"))
.build();
let client = Client::from_conf(config);
let _resp = client
.head_object()
.key("dontcare.json")
.bucket("dontcare")
.send()
.await
.expect("content length enforcement must not apply to HEAD requests");

// The body returned will be empty, so we pass an empty string to full_validate.
// That way, it'll do a string equality check on the empty strings.
http_client.full_validate("").await.unwrap();
}

#[tokio::test]
async fn test_content_length_enforcement_get_request_short() {
let http_client =
ReplayingClient::from_file("tests/data/content-length-enforcement/get-object-short.json")
.expect("recorded HTTP communication exists");
let config = Config::builder()
.with_test_defaults()
.http_client(http_client.clone())
.region(Region::new("us-east-1"))
.build();
let client = Client::from_conf(config);
// The file we're fetching is exactly 10,000 bytes long, but we've set the
// response's content-length to 9,999 bytes. This should trigger the
// content-length enforcement.

// This will succeed.
let output = client
.get_object()
.key("1000-lines.txt")
.bucket("dontcare")
.send()
.await
.unwrap();

// This will fail with a content-length mismatch error.
let content_length_err = output.body.collect().await.unwrap_err();

http_client.full_validate("application/text").await.unwrap();
assert_eq!(
DisplayErrorContext(content_length_err).to_string(),
"streaming error: Invalid Content-Length: Expected 9999 bytes but 10000 bytes were received (Error { kind: StreamingError(ContentLengthError { expected: 9999, received: 10000 }) })"
);
}

#[tokio::test]
async fn test_content_length_enforcement_get_request_long() {
let http_client =
ReplayingClient::from_file("tests/data/content-length-enforcement/get-object-long.json")
.expect("recorded HTTP communication exists");
let config = Config::builder()
.with_test_defaults()
.http_client(http_client.clone())
.region(Region::new("us-east-1"))
.build();
let client = Client::from_conf(config);
// The file we're fetching is exactly 10,000 bytes long, but we've set the
// response's content-length to 9,999 bytes. This should trigger the
// content-length enforcement.

// This will succeed.
let output = client
.get_object()
.key("1000-lines.txt")
.bucket("dontcare")
.send()
.await
.unwrap();

// This will fail with a content-length mismatch error.
let content_length_err = output.body.collect().await.unwrap_err();

http_client.full_validate("application/text").await.unwrap();
assert_eq!(
DisplayErrorContext(content_length_err).to_string(),
"streaming error: Invalid Content-Length: Expected 10001 bytes but 10000 bytes were received (Error { kind: StreamingError(ContentLengthError { expected: 10001, received: 10000 }) })"
);
}
Loading

0 comments on commit db89652

Please sign in to comment.