-
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: modularize checksums #1568
Merged
Merged
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
4cc468e
Modularize I&A.
852a178
Merge branch 'main' into feat/modularize-identity-and-auth
cfb3866
ktlint
8f81e77
Update codegen test.
17e1b65
Dependency fix
767a28c
Fix integ test dependencies
638b3b8
Fix test dependency
89288f0
Fix integ test dependency
f4fd0b6
Add SmithyIdentity module dependency for services
0f63cd5
Merge branch 'main' into feat/modularize-identity-and-auth
3f9378a
Add SPI imports for AWSSDKCommon & AWSSDKIdentity to service clients
198c333
Add missing SPI tag
7ad1d84
Bruh
de1280a
Add dependency on AWSSDKCommon to serviceTargetDependency
927817e
Change SPI name in case conflict is causing issue
c43b24f
Imports in tests
70ac32e
Import changes
04dc1fe
Fix helper function
71f300c
Organize base package.swift
337ce08
Merge checksum separation work from David
d8a6390
Remove unnecessary import statement
1aea0e1
create AWSSDKChecksums
dayaffe 01e636b
remove Package.swift changes
dayaffe 4b5620e
Merge branch 'main' into day/modular-checksums
dayaffe d355c98
Merge branch 'main' into day/modular-checksums
dayaffe f60e12d
trim dependencies of awssdkchecksums
dayaffe 6351e1a
try to get CI to pass
dayaffe 86f31fb
Merge branch 'main' into day/modular-checksums
dayaffe 18d71ad
more stuff
dayaffe 12bc3d0
Merge branch 'main' into day/modular-checksums
dayaffe 5474cd0
more stuff
dayaffe 28f448b
fix integration import
dayaffe a2890f7
should pass nowww
dayaffe 03866cb
Merge branch 'main' into day/modular-checksums
dayaffe 19a4ccf
another update
dayaffe dfc3775
add missing dependency
dayaffe 664715c
fix lint
dayaffe 1760a63
add missing imports to integration tests
dayaffe 778af3a
ktlint
dayaffe ad92966
ktlint
dayaffe 05f2dbc
Merge branch 'main' into day/modular-checksums
dayaffe 4dc3d73
Merge branch 'main' into day/modular-checksums
dayaffe 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
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
119 changes: 119 additions & 0 deletions
119
Sources/Core/AWSClientRuntime/Middlewares/FlexibleChecksumsRequestMiddleware.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,119 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import enum SmithyChecksumsAPI.ChecksumAlgorithm | ||
import enum SmithyChecksums.ChecksumMismatchException | ||
import enum Smithy.ClientError | ||
import class Smithy.Context | ||
import AwsCommonRuntimeKit | ||
import AWSSDKChecksums | ||
import ClientRuntime | ||
import SmithyHTTPAPI | ||
|
||
public struct FlexibleChecksumsRequestMiddleware<OperationStackInput, OperationStackOutput>: Middleware { | ||
|
||
public let id: String = "FlexibleChecksumsRequestMiddleware" | ||
|
||
let checksumAlgorithm: String? | ||
|
||
public init(checksumAlgorithm: String?) { | ||
self.checksumAlgorithm = checksumAlgorithm | ||
} | ||
|
||
public func handle<H>(context: Context, | ||
input: SerializeStepInput<OperationStackInput>, | ||
next: H) async throws -> OperationOutput<OperationStackOutput> | ||
where H: Handler, | ||
Self.MInput == H.Input, | ||
Self.MOutput == H.Output { | ||
try await addHeaders(builder: input.builder, attributes: context) | ||
return try await next.handle(context: context, input: input) | ||
} | ||
|
||
private func addHeaders(builder: SdkHttpRequestBuilder, attributes: Context) async throws { | ||
if case(.stream(let stream)) = builder.body { | ||
attributes.isChunkedEligibleStream = stream.isEligibleForChunkedStreaming | ||
if stream.isEligibleForChunkedStreaming { | ||
try builder.setAwsChunkedHeaders() // x-amz-decoded-content-length | ||
} | ||
} | ||
|
||
// Initialize logger | ||
guard let logger = attributes.getLogger() else { | ||
throw ClientError.unknownError("No logger found!") | ||
} | ||
|
||
guard let checksumString = checksumAlgorithm else { | ||
logger.info("No checksum provided! Skipping flexible checksums workflow...") | ||
return | ||
} | ||
|
||
guard let checksumHashFunction = ChecksumAlgorithm.from(string: checksumString) else { | ||
logger.info("Found no supported checksums! Skipping flexible checksums workflow...") | ||
return | ||
} | ||
|
||
// Determine the header name | ||
let headerName = "x-amz-checksum-\(checksumHashFunction)" | ||
logger.debug("Resolved checksum header name: \(headerName)") | ||
|
||
// Check if any checksum header is already provided by the user | ||
let checksumHeaderPrefix = "x-amz-checksum-" | ||
if builder.headers.headers.contains(where: { | ||
$0.name.lowercased().starts(with: checksumHeaderPrefix) && | ||
$0.name.lowercased() != "x-amz-checksum-algorithm" | ||
}) { | ||
logger.debug("Checksum header already provided by the user. Skipping calculation.") | ||
return | ||
} | ||
|
||
// Handle body vs handle stream | ||
switch builder.body { | ||
case .data(let data): | ||
guard let data else { | ||
throw ClientError.dataNotFound("Cannot calculate checksum of empty body!") | ||
} | ||
|
||
if builder.headers.value(for: headerName) == nil { | ||
logger.debug("Calculating checksum") | ||
} | ||
|
||
// Create checksum instance | ||
let checksum = checksumHashFunction.createChecksum() | ||
|
||
// Pass data to hash | ||
try checksum.update(chunk: data) | ||
|
||
// Retrieve the hash | ||
let hash = try checksum.digest().toBase64String() | ||
|
||
builder.updateHeader(name: headerName, value: [hash]) | ||
case .stream: | ||
// Will handle calculating checksum and setting header later | ||
attributes.checksum = checksumHashFunction | ||
builder.updateHeader(name: "x-amz-trailer", value: [headerName]) | ||
case .noStream: | ||
throw ClientError.dataNotFound("Cannot calculate the checksum of an empty body!") | ||
} | ||
} | ||
|
||
public typealias MInput = SerializeStepInput<OperationStackInput> | ||
public typealias MOutput = OperationOutput<OperationStackOutput> | ||
} | ||
|
||
extension FlexibleChecksumsRequestMiddleware: HttpInterceptor { | ||
public typealias InputType = OperationStackInput | ||
public typealias OutputType = OperationStackOutput | ||
|
||
public func modifyBeforeRetryLoop( | ||
context: some MutableRequest<InputType, RequestType, AttributesType> | ||
) async throws { | ||
let builder = context.getRequest().toBuilder() | ||
try await addHeaders(builder: builder, attributes: context.getAttributes()) | ||
context.updateRequest(updated: builder.build()) | ||
} | ||
} |
145 changes: 145 additions & 0 deletions
145
Sources/Core/AWSClientRuntime/Middlewares/FlexibleChecksumsResponseMiddleware.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,145 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0. | ||
|
||
import Smithy | ||
import SmithyHTTPAPI | ||
import enum SmithyChecksumsAPI.ChecksumAlgorithm | ||
import enum SmithyChecksums.ChecksumMismatchException | ||
import ClientRuntime | ||
|
||
public struct FlexibleChecksumsResponseMiddleware<OperationStackInput, OperationStackOutput>: Middleware { | ||
|
||
public let id: String = "FlexibleChecksumsResponseMiddleware" | ||
|
||
// The priority to validate response checksums, if multiple are present | ||
let CHECKSUM_HEADER_VALIDATION_PRIORITY_LIST: [String] = [ | ||
ChecksumAlgorithm.crc32c, | ||
.crc32, | ||
.sha1, | ||
.sha256 | ||
].sorted().map { $0.toString() } | ||
|
||
let validationMode: Bool | ||
let priorityList: [String] | ||
|
||
public init(validationMode: Bool, priorityList: [String] = []) { | ||
self.validationMode = validationMode | ||
self.priorityList = !priorityList.isEmpty | ||
? withPriority(checksums: priorityList) | ||
: CHECKSUM_HEADER_VALIDATION_PRIORITY_LIST | ||
} | ||
|
||
public func handle<H>(context: Context, | ||
input: SdkHttpRequest, | ||
next: H) async throws -> OperationOutput<OperationStackOutput> | ||
where H: Handler, | ||
Self.MInput == H.Input, | ||
Self.MOutput == H.Output { | ||
|
||
// The name of the checksum header which was validated. If `null`, validation was not performed. | ||
context.attributes.set(key: AttributeKey<String>(name: "ChecksumHeaderValidated"), value: nil) | ||
|
||
// Initialize logger | ||
guard let logger = context.getLogger() else { throw ClientError.unknownError("No logger found!") } | ||
|
||
// Get the response | ||
let output = try await next.handle(context: context, input: input) | ||
|
||
try await validateChecksum(response: output.httpResponse, logger: logger, attributes: context) | ||
|
||
return output | ||
} | ||
|
||
private func validateChecksum(response: HttpResponse, logger: any LogAgent, attributes: Context) async throws { | ||
// Exit if validation should not be performed | ||
if !validationMode { | ||
logger.info("Checksum validation should not be performed! Skipping workflow...") | ||
return | ||
} | ||
|
||
let checksumHeaderIsPresent = priorityList.first { | ||
response.headers.value(for: "x-amz-checksum-\($0)") != nil | ||
} | ||
|
||
guard let checksumHeader = checksumHeaderIsPresent else { | ||
let message = | ||
"User requested checksum validation, but the response headers did not contain any valid checksums" | ||
logger.warn(message) | ||
return | ||
} | ||
|
||
let fullChecksumHeader = "x-amz-checksum-" + checksumHeader | ||
|
||
logger.debug("Validating checksum from \(fullChecksumHeader)") | ||
attributes.set(key: AttributeKey<String>(name: "ChecksumHeaderValidated"), value: fullChecksumHeader) | ||
|
||
let checksumString = checksumHeader.removePrefix("x-amz-checksum-") | ||
guard let responseChecksum = ChecksumAlgorithm.from(string: checksumString) else { | ||
throw ClientError.dataNotFound("Checksum found in header is not supported!") | ||
} | ||
|
||
guard let expectedChecksum = response.headers.value(for: fullChecksumHeader) else { | ||
throw ClientError.dataNotFound("Could not determine the expected checksum!") | ||
} | ||
|
||
// Handle body vs handle stream | ||
switch response.body { | ||
case .data(let data): | ||
guard let data else { | ||
throw ClientError.dataNotFound("Cannot calculate checksum of empty body!") | ||
} | ||
|
||
let responseChecksumHasher = responseChecksum.createChecksum() | ||
try responseChecksumHasher.update(chunk: data) | ||
let actualChecksum = try responseChecksumHasher.digest().toBase64String() | ||
|
||
guard expectedChecksum == actualChecksum else { | ||
let message = "Checksum mismatch. Expected \(expectedChecksum) but was \(actualChecksum)" | ||
throw ChecksumMismatchException.message(message) | ||
} | ||
case .stream(let stream): | ||
let validatingStream = ByteStream.getChecksumValidatingBody( | ||
stream: stream, | ||
expectedChecksum: expectedChecksum, | ||
checksumAlgorithm: responseChecksum | ||
) | ||
|
||
// Set the response to a validating stream | ||
attributes.httpResponse = response | ||
attributes.httpResponse?.body = validatingStream | ||
case .noStream: | ||
throw ClientError.dataNotFound("Cannot calculate the checksum of an empty body!") | ||
} | ||
} | ||
|
||
public typealias MInput = SdkHttpRequest | ||
public typealias MOutput = OperationOutput<OperationStackOutput> | ||
} | ||
|
||
extension FlexibleChecksumsResponseMiddleware: HttpInterceptor { | ||
public typealias InputType = OperationStackInput | ||
public typealias OutputType = OperationStackOutput | ||
|
||
public func modifyBeforeRetryLoop( | ||
context: some MutableRequest<InputType, RequestType, AttributesType> | ||
) async throws { | ||
context.getAttributes().set(key: AttributeKey<String>(name: "ChecksumHeaderValidated"), value: nil) | ||
} | ||
|
||
public func modifyBeforeDeserialization( | ||
context: some MutableResponse<InputType, RequestType, ResponseType, AttributesType> | ||
) async throws { | ||
guard let logger = context.getAttributes().getLogger() else { | ||
throw ClientError.unknownError("No logger found!") | ||
} | ||
|
||
let response = context.getResponse() | ||
try await validateChecksum(response: response, logger: logger, attributes: context.getAttributes()) | ||
context.updateResponse(updated: response) | ||
} | ||
} | ||
|
||
private func withPriority(checksums: [String]) -> [String] { | ||
let checksumsMap = checksums.compactMap { ChecksumAlgorithm.from(string: $0) } | ||
return checksumsMap.sorted().map { $0.toString() } | ||
} |
Oops, something went wrong.
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.
Question: Is a separate module actually necessary for just a single extension with two methods?
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.
I wish more could be moved to AWSSDKChecksums but that's dependent on CRT HTTP Client being refactored in the future. I'd say even if its only two methods its good to have but I don't feel strongly.
aws-chunked
is pretty explicitly aws-only though and one of those methods adds it to the headersThere 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.
I see. We could keep it the way it is in this PR, or move it into AWSClientRuntime. For both cases we would revisit this down the line. I don't feel strongly about either option. Unless Josh has preference for one over the other I suppose we can keep
AWSSDKChecksums
.