-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat: Add support for flexible checksums on streaming payloads #1329
Merged
Merged
Changes from all commits
Commits
Show all changes
72 commits
Select commit
Hold shift + click to select a range
50a2c46
add support for Streaming flexible checksums
dayaffe 6e08f16
fix rebase
dayaffe 020603f
update crt version and S3 client codegen
dayaffe f8e116d
swiftlint
dayaffe e0c5fe8
fix merge conflict in test
dayaffe 235d6ba
remove empty file
dayaffe 6b2813f
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 3210f33
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 93068be
bump to crt 0.27.0
dayaffe 0cae37b
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 9439af7
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 156046d
add integration tests for FC
dayaffe 959fe54
Merge branch 'main' into day/aws-chunked-encoding
dayaffe eef3020
bump to crt 0.28.0
dayaffe f770b76
switch integration test from file to buffered stream
dayaffe 37b9062
fix swiftlint
dayaffe 0d83778
Merge branch 'main' into day/aws-chunked-encoding
dayaffe db21c76
Use xcodebuild to run integration tests
jbelkins 92cc708
Put xcodebuild in right place
jbelkins bb0298a
Fix scheme name
jbelkins 33186c9
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 811d4c6
go back to swift toolchain
dayaffe 7c0910a
Test using xcode 14.3.1
dayaffe 9698cac
test all different kinds of xcode versions with mac 14
dayaffe ebe51f1
revert xcode versions
dayaffe 71ffb89
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 734734d
test xcode 14.3.1
dayaffe 830601c
remove exclusions from test runner for xcode15.2
dayaffe 396642d
checksumAlgorithm not needed to set chunked headers
dayaffe b8c849a
refactor for identity and auth
dayaffe 5b9f7b1
merge/refactor for identity & auth
dayaffe eb9c76e
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 3807125
remove unused variable
dayaffe d26ec5e
Merge branch 'main' into day/aws-chunked-encoding
dayaffe 5e32579
rename HashFunction to ChecksumAlgorithm for SRA
dayaffe 434cbb6
fix name
dayaffe 87c439e
Merge branch 'main' into day/aws-chunked-encoding
dayaffe d8a6c39
Merge branch 'main' into day/aws-chunked-encoding
jbelkins b544a13
Merge branch 'main' into day/aws-chunked-encoding
jbelkins bc0ae0c
Enable Intel Mac integration tests
jbelkins 6b54549
Merge branch 'main' into day/aws-chunked-encoding
jbelkins c1d3b64
Get OS version in logs
jbelkins f70290d
Merge branch 'main' into day/aws-chunked-encoding
jbelkins ed0f60e
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 4f7ecc7
Merge branch 'main' into day/aws-chunked-encoding
jbelkins fc51f47
Refactor flexible checksum tests
7dd0ff9
More flexible checksum test refactors
3c3eef0
Refactor FC tests even more
jbelkins c9a3159
Merge branch 'main' into day/aws-chunked-encoding
jbelkins ae502f9
Exclude the Transfer-Encoding header from AWS request signature
jbelkins e5b32cf
Refactor FC integration tests
jbelkins 9550710
Fix lint
jbelkins ae9dd19
Rename tests to match class under test
jbelkins 9d17776
Run integration tests on Intel CI only
jbelkins 1a246b7
Don't run integration tests in an environment
jbelkins e78129e
Don't run Linux integration tests, run 4 Intel Macs
jbelkins 9f6f25e
Many runners
jbelkins 3e4450a
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 7aba130
Re-enable Linux integration tests
jbelkins a42e403
Merge branch 'main' into day/aws-chunked-encoding
jbelkins e60ac8e
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 2a42286
Revert integration test CI workflow
jbelkins 55a5d52
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 61a0a9c
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 9a47471
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 12aa861
Merge branch 'main' into day/aws-chunked-encoding
jbelkins 87ffefa
Merge remote-tracking branch 'origin/main' into day/aws-chunked-encoding
jbelkins a462a2f
Run many integration tests
jbelkins 8add54f
Quotes around Swift versions in CI workflow file
jbelkins e177ef1
Run integration tests on AL2
jbelkins 4105b7a
Use AWS GH action v3
jbelkins 4a694cf
Revert integration test CI workflow
jbelkins File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
134 changes: 134 additions & 0 deletions
134
IntegrationTests/Services/AWSS3IntegrationTests/S3FlexibleChecksumsTests.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import XCTest | ||
import AWSS3 | ||
@testable import ClientRuntime | ||
|
||
final class S3FlexibleChecksumsTests: S3XCTestCase { | ||
var originalData: Data! | ||
|
||
override func setUp() { | ||
super.setUp() | ||
// Fill one MB with random data. Data is refreshed for each flexible checksums tests below. | ||
originalData = Data((0..<(1024 * 1024)).map { _ in UInt8.random(in: UInt8.min...UInt8.max) }) | ||
} | ||
|
||
// MARK: - Data uploads | ||
|
||
func test_putGetObject_data_crc32() async throws { | ||
try await _testPutGetObject(withChecksumAlgorithm: .crc32, objectNameSuffix: "crc32-data", upload: .data(originalData)) | ||
} | ||
|
||
func test_putGetObject_data_crc32c() async throws { | ||
try await _testPutGetObject(withChecksumAlgorithm: .crc32c, objectNameSuffix: "crc32c-data", upload: .data(originalData)) | ||
} | ||
|
||
func test_putGetObject_data_sha1() async throws { | ||
try await _testPutGetObject(withChecksumAlgorithm: .sha1, objectNameSuffix: "sha1-data", upload: .data(originalData)) | ||
} | ||
|
||
func test_putGetObject_data_sha256() async throws { | ||
try await _testPutGetObject(withChecksumAlgorithm: .sha256, objectNameSuffix: "sha256-data", upload: .data(originalData)) | ||
} | ||
|
||
// MARK: - Streaming uploads | ||
|
||
func test_putGetObject_streaming_crc32() async throws { | ||
let bufferedStream = BufferedStream(data: originalData, isClosed: true) | ||
try await _testPutGetObject(withChecksumAlgorithm: .crc32, objectNameSuffix: "crc32", upload: .stream(bufferedStream)) | ||
} | ||
|
||
func test_putGetObject_streaming_crc32c() async throws { | ||
let bufferedStream = BufferedStream(data: originalData, isClosed: true) | ||
try await _testPutGetObject(withChecksumAlgorithm: .crc32c, objectNameSuffix: "crc32c", upload: .stream(bufferedStream)) | ||
} | ||
|
||
func test_putGetObject_streaming_sha1() async throws { | ||
let bufferedStream = BufferedStream(data: originalData, isClosed: true) | ||
try await _testPutGetObject(withChecksumAlgorithm: .sha1, objectNameSuffix: "sha1", upload: .stream(bufferedStream)) | ||
} | ||
|
||
func test_putGetObject_streaming_sha256() async throws { | ||
let bufferedStream = BufferedStream(data: originalData, isClosed: true) | ||
try await _testPutGetObject(withChecksumAlgorithm: .sha256, objectNameSuffix: "sha256", upload: .stream(bufferedStream)) | ||
} | ||
|
||
// Streaming without checksum (chunked encoding) | ||
func test_putGetObject_streaming_chunked() async throws { | ||
let bufferedStream = BufferedStream(data: originalData, isClosed: true) | ||
let objectName = "flexible-checksums-s3-test-chunked" | ||
|
||
let putObjectInput = PutObjectInput( | ||
body: .stream(bufferedStream), | ||
bucket: bucketName, | ||
key: objectName | ||
) | ||
|
||
_ = try await client.putObject(input: putObjectInput) | ||
|
||
let getObjectInput = GetObjectInput(bucket: bucketName, key: objectName) | ||
let getObjectOutput = try await client.getObject(input: getObjectInput) | ||
let body = try XCTUnwrap(getObjectOutput.body) | ||
let data = try await body.readData() | ||
XCTAssertEqual(data, originalData) | ||
} | ||
|
||
// MARK: - Private methods | ||
|
||
private func _testPutGetObject( | ||
withChecksumAlgorithm algorithm: S3ClientTypes.ChecksumAlgorithm, | ||
objectNameSuffix: String, upload: ByteStream, file: StaticString = #filePath, line: UInt = #line | ||
) async throws { | ||
let objectName = "flexible-checksums-s3-test-\(objectNameSuffix)" | ||
|
||
let input = PutObjectInput( | ||
body: upload, | ||
bucket: bucketName, | ||
checksumAlgorithm: algorithm, | ||
key: objectName | ||
) | ||
|
||
let output = try await client.putObject(input: input) | ||
|
||
// Verify the checksum response based on the algorithm used. | ||
let checksumResponse = try XCTUnwrap(getChecksumResponse(from: output, with: algorithm), file: file, line: line) | ||
XCTAssertNotNil(checksumResponse, file: file, line: line) | ||
|
||
let getInput = GetObjectInput(bucket: bucketName, checksumMode: S3ClientTypes.ChecksumMode.enabled, key: objectName) | ||
let getOutput = try await client.getObject(input: getInput) // will error for normal payloads if checksum mismatch | ||
XCTAssertNotNil(getOutput.body, file: file, line: line) // Ensure there's a body in the response. | ||
|
||
// Additional step for stream: Validate stream and read data. | ||
if case .stream = upload { | ||
let streamingBody = try XCTUnwrap(getOutput.body, file: file, line: line) | ||
if case .stream(let stream) = streamingBody { | ||
XCTAssert(stream is ValidatingBufferedStream, "Expected ValidatingBufferedStream for streaming upload", file: file, line: line) | ||
let data = try await streamingBody.readData() // will error if checksum mismatch | ||
XCTAssertEqual(data, originalData, file: file, line: line) | ||
} else { | ||
XCTFail("Did not receive a stream when expected for checksum validation!", file: file, line: line) | ||
} | ||
} | ||
} | ||
|
||
private func getChecksumResponse(from response: PutObjectOutput, with algorithm: S3ClientTypes.ChecksumAlgorithm) throws -> String? { | ||
switch algorithm { | ||
case .crc32: | ||
return response.checksumCRC32 | ||
case .crc32c: | ||
return response.checksumCRC32C | ||
case .sha1: | ||
return response.checksumSHA1 | ||
case .sha256: | ||
return response.checksumSHA256 | ||
default: | ||
XCTFail("Unsupported checksum algorithm") | ||
return nil | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason for checking en_US_POSIX locale?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't specify the locale for the date & string operations that are locale-dependent, then you get the locale that the user sets as default on their device. And that may not result in lowercasing like you expected.
(I have had this happen with datetime stamps in the past, not with lowercasing, but the same risk is present)