Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Only enforce content length for GET requests #3657

Merged
merged 5 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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