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 3 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
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("").await.unwrap();
Velfi marked this conversation as resolved.
Show resolved Hide resolved
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("").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