Make composite checksum check S3 specific

Author: 0marperez

codegen/protocol-tests/model/error-correction-tests.smithy

@@ -37,7 +37,11 @@ operation SayHello { output: TestOutputDocument, errors: [Error] }
 @http(method: "POST", uri: "/")
 operation SayHelloXml { output: TestOutput, errors: [Error] }
 
-structure TestOutputDocument with [TestStruct] { innerField: Nested, @required document: Document }
+structure TestOutputDocument with [TestStruct] {
+    innerField: Nested,
+    // @required
+    document: Document
+}
 structure TestOutput with [TestStruct] { innerField: Nested }
 
 @mixin
@@ -60,7 +64,7 @@ structure TestStruct {
     @required
     nestedListValue: NestedList
 
-    @required
+    // @required
     nested: Nested
 
     @required
@@ -91,7 +95,7 @@ union MyUnion {
 }
 
 structure Nested {
-    @required
+    // @required
     a: String
 }
 

runtime/protocol/http-client/api/http-client.api

@@ -340,7 +340,7 @@ public final class aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksums
 
 public final class aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsResponseInterceptor : aws/smithy/kotlin/runtime/client/Interceptor {
 	public static final field Companion Laws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsResponseInterceptor$Companion;
-	public fun <init> (ZLaws/smithy/kotlin/runtime/client/config/HttpChecksumConfigOption;)V
+	public fun <init> (ZLaws/smithy/kotlin/runtime/client/config/HttpChecksumConfigOption;Z)V
 	public fun modifyBeforeAttemptCompletion-gIAlu-s (Laws/smithy/kotlin/runtime/client/ResponseInterceptorContext;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun modifyBeforeCompletion-gIAlu-s (Laws/smithy/kotlin/runtime/client/ResponseInterceptorContext;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun modifyBeforeDeserialization (Laws/smithy/kotlin/runtime/client/ProtocolResponseInterceptorContext;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;

runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsRequestInterceptor.kt

@@ -26,7 +26,7 @@ import kotlinx.coroutines.job
 import kotlin.coroutines.coroutineContext
 
 /**
- * Calculates a request's checksum.
+ * Handles request checksums.
  *
  * If a user supplies a checksum via an HTTP header no calculation will be done. The exception is MD5, if a user
  * supplies an MD5 checksum header it will be ignored.
@@ -36,65 +36,62 @@ import kotlin.coroutines.coroutineContext
  * - If no checksum is configured for the request then use the default checksum algorithm to calculate a checksum.
  *
  * If the request will be streamed:
- * - The checksum calculation is done asynchronously using a hashing & completing body.
+ * - The checksum calculation is done during transmission using a hashing & completing body.
  * - The checksum will be sent in a trailing header, once the request is consumed.
  *
  * If the request will not be streamed:
- * - The checksum calculation is done synchronously
+ * - The checksum calculation is done before transmission
  * - The checksum will be sent in a header
  *
  * Business metrics MUST be emitted for the checksum algorithm used.
  *
  * @param requestChecksumRequired Model sourced flag indicating if checksum calculation is mandatory.
  * @param requestChecksumCalculation Configuration option that determines when checksum calculation should be done.
- * @param userSelectedChecksumAlgorithm The checksum algorithm that the user selected for the request, may be null.
+ * @param requestChecksumAlgorithm The checksum algorithm that the user selected for the request, may be null.
  */
 @InternalApi
 public class FlexibleChecksumsRequestInterceptor(
     requestChecksumRequired: Boolean,
     requestChecksumCalculation: HttpChecksumConfigOption?,
-    userSelectedChecksumAlgorithm: String?,
+    requestChecksumAlgorithm: String?,
 ) : AbstractChecksumInterceptor() {
-    private val forcedToCalculateChecksum = requestChecksumRequired || requestChecksumCalculation == HttpChecksumConfigOption.WHEN_SUPPORTED
-    private val checksumHeader = StringBuilder("x-amz-checksum-")
-    private val defaultChecksumAlgorithm = lazy { Crc32() }
-    private val defaultChecksumAlgorithmHeaderPostfix = "crc32"
-
-    private val checksumAlgorithm = userSelectedChecksumAlgorithm?.let {
-        val hashFunction = userSelectedChecksumAlgorithm.toHashFunction()
+    private val checksumHeader = buildString {
+        append("x-amz-checksum-")
+        append(requestChecksumAlgorithm?.lowercase() ?: "crc32")
+    }
+    private val checksumAlgorithm = requestChecksumAlgorithm?.let {
+        val hashFunction = requestChecksumAlgorithm.toHashFunction()
         if (hashFunction == null || !hashFunction.isSupported) {
-            throw ClientException("Checksum algorithm '$userSelectedChecksumAlgorithm' is not supported for flexible checksums")
+            throw ClientException("Checksum algorithm '$requestChecksumAlgorithm' is not supported for flexible checksums")
         }
-        checksumHeader.append(userSelectedChecksumAlgorithm.lowercase())
         hashFunction
-    } ?: if (forcedToCalculateChecksum) {
-        checksumHeader.append(defaultChecksumAlgorithmHeaderPostfix)
-        defaultChecksumAlgorithm.value
+    } ?: if (requestChecksumRequired || requestChecksumCalculation == HttpChecksumConfigOption.WHEN_SUPPORTED) {
+        Crc32()
     } else {
         null
     }
 
     override suspend fun modifyBeforeSigning(context: ProtocolRequestInterceptorContext<Any, HttpRequest>): HttpRequest {
         val logger = coroutineContext.logger<FlexibleChecksumsRequestInterceptor>()
 
-        userProviderChecksumHeader(context.protocolRequest, logger)?.let {
-            logger.debug { "User supplied a checksum via header, skipping checksum calculation" }
+        context.protocolRequest.userProvidedChecksumHeader(logger)?.let {
+            logger.debug { "Checksum was supplied via header, skipping checksum calculation" }
 
             val request = context.protocolRequest.toBuilder()
             request.headers.removeAllChecksumHeadersExcept(it)
             return request.build()
         }
 
         if (checksumAlgorithm == null) {
-            logger.debug { "User didn't select a checksum algorithm and checksum calculation isn't required, skipping checksum calculation" }
+            logger.debug { "A checksum algorithm isn't selected and checksum calculation isn't required, skipping checksum calculation" }
             return context.protocolRequest
         }
 
-        logger.debug { "Calculating checksum using '$checksumAlgorithm'" }
-
         val request = context.protocolRequest.toBuilder()
 
         if (request.body.isEligibleForAwsChunkedStreaming) {
+            logger.debug { "Calculating checksum during transmission using '$checksumAlgorithm'" }
+
             val deferredChecksum = CompletableDeferred<String>(context.executionContext.coroutineContext.job)
 
             request.body = request.body
@@ -106,26 +103,28 @@ public class FlexibleChecksumsRequestInterceptor(
                     deferredChecksum,
                 )
 
-            request.headers.append("x-amz-trailer", checksumHeader.toString())
-            request.trailingHeaders.append(checksumHeader.toString(), deferredChecksum)
+            request.headers.append("x-amz-trailer", checksumHeader)
+            request.trailingHeaders.append(checksumHeader, deferredChecksum)
         } else {
+            logger.debug { "Calculating checksum before transmission using '$checksumAlgorithm'" }
+
             checksumAlgorithm.update(
                 request.body.readAll() ?: byteArrayOf(),
             )
-            request.headers[checksumHeader.toString()] = checksumAlgorithm.digest().encodeBase64String()
+            request.headers[checksumHeader] = checksumAlgorithm.digest().encodeBase64String()
         }
 
         context.executionContext.emitBusinessMetric(checksumAlgorithm.toBusinessMetric())
-        request.headers.removeAllChecksumHeadersExcept(checksumHeader.toString())
+        request.headers.removeAllChecksumHeadersExcept(checksumHeader)
 
         return request.build()
     }
 
     override suspend fun calculateChecksum(context: ProtocolRequestInterceptorContext<Any, HttpRequest>): String? {
-        val req = context.protocolRequest.toBuilder()
-
         if (checksumAlgorithm == null) return null
 
+        val req = context.protocolRequest.toBuilder()
+
         return when {
             req.body.contentLength == null && !req.body.isOneShot -> {
                 val channel = req.body.toSdkByteReadChannel()!!
@@ -145,8 +144,8 @@ public class FlexibleChecksumsRequestInterceptor(
     ): HttpRequest {
         val req = context.protocolRequest.toBuilder()
 
-        if (!req.headers.contains(checksumHeader.toString())) {
-            req.header(checksumHeader.toString(), checksum)
+        if (!req.headers.contains(checksumHeader)) {
+            req.header(checksumHeader, checksum)
         }
 
         return req.build()
@@ -234,15 +233,15 @@ public class FlexibleChecksumsRequestInterceptor(
     /**
      * Checks if a user provided a checksum for a request via an HTTP header.
      * The header must start with "x-amz-checksum-" followed by the checksum algorithm's name.
-     * MD5 is not considered a valid checksum algorithm.
+     * MD5 is not considered a supported checksum algorithm.
      */
-    private fun userProviderChecksumHeader(request: HttpRequest, logger: Logger): String? {
-        request.headers.entries().forEach { header ->
+    private fun HttpRequest.userProvidedChecksumHeader(logger: Logger): String? {
+        this.headers.entries().forEach { header ->
             val headerName = header.key.lowercase()
             if (headerName.startsWith("x-amz-checksum-")) {
-                if (headerName.endsWith("md5")) {
+                if (headerName == "x-amz-checksum-md5") {
                     logger.debug {
-                        "User provided md5 request checksum via headers, md5 is not a valid algorithm, ignoring header"
+                        "MD5 checksum was supplied via header, MD5 is not a supported algorithm, ignoring header"
                     }
                 } else {
                     return headerName

runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsResponseInterceptor.kt

@@ -32,21 +32,22 @@ internal val CHECKSUM_HEADER_VALIDATION_PRIORITY_LIST: List<String> = listOf(
 )
 
 /**
- * Validate a response's checksum.
+ * Handles response checksums.
  *
  * If it's a streaming response, it wraps the response in a hashing body, calculating the checksum as the response is
  * streamed to the user. The checksum is validated after the user has consumed the entire body using a checksum validating body.
  * Otherwise, the checksum if calculated all at once.
  *
  * Users can check which checksum was validated by referencing the `ResponseChecksumValidated` execution context variable.
  *
- * @param responseValidationRequired Flag indicating if the checksum validation is mandatory.
+ * @param responseValidationRequired Model sourced flag indicating if the checksum validation is mandatory.
  * @param responseChecksumValidation Configuration option that determines when checksum validation should be done.
  */
 @InternalApi
 public class FlexibleChecksumsResponseInterceptor(
     private val responseValidationRequired: Boolean,
     private val responseChecksumValidation: HttpChecksumConfigOption?,
+    private val serviceUsesCompositeChecksums: Boolean,
 ) : HttpInterceptor {
     @InternalApi
     public companion object {
@@ -62,24 +63,25 @@ public class FlexibleChecksumsResponseInterceptor(
 
         val checksumHeader = CHECKSUM_HEADER_VALIDATION_PRIORITY_LIST
             .firstOrNull { context.protocolResponse.headers.contains(it) } ?: run {
-            logger.warn { "User requested checksum validation, but the response headers did not contain any valid checksums" }
+            logger.warn { "Checksum validation was requested but the response headers didn't contain a valid checksum." }
             return context.protocolResponse
         }
         val serviceChecksumValue = context.protocolResponse.headers[checksumHeader]!!
 
-        if (serviceChecksumValue.isCompositeChecksum()) {
+        if (serviceUsesCompositeChecksums && serviceChecksumValue.isCompositeChecksum()) {
             logger.debug { "Service returned composite checksum. Skipping validation." }
             return context.protocolResponse
         }
 
-        logger.debug { "Validating checksum from $checksumHeader" }
         context.executionContext[ChecksumHeaderValidated] = checksumHeader
 
         val checksumAlgorithm = checksumHeader
             .removePrefix("x-amz-checksum-")
             .toHashFunction() ?: throw ClientException("Could not parse checksum algorithm from header $checksumHeader")
 
         if (context.protocolResponse.body is HttpBody.Bytes) {
+            logger.debug { "Validating checksum before deserialization from $checksumHeader" }
+
             // Calculate checksum
             checksumAlgorithm.update(
                 context.protocolResponse.body.readAll() ?: byteArrayOf(),
@@ -93,6 +95,8 @@ public class FlexibleChecksumsResponseInterceptor(
 
             return context.protocolResponse
         } else {
+            logger.debug { "Validating checksum during deserialization from $checksumHeader" }
+
             // Wrap the response body in a hashing body
             return context.protocolResponse.copy(
                 body = context.protocolResponse.body
@@ -157,7 +161,7 @@ private fun validateAndThrow(expected: String, actual: String) {
  * Composite checksums are used for multipart uploads.
  */
 private fun String.isCompositeChecksum(): Boolean {
-    // Ends with "-#" where "#" is a number between 1-1000
-    val regex = Regex("-([1-9][0-9]{0,2}|1000)$")
+    // Ends with "-#" where "#" is a number
+    val regex = Regex("-(\\d)+$")
     return regex.containsMatchIn(this)
 }

runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsRequestInterceptorTest.kt

@@ -43,7 +43,7 @@ class FlexibleChecksumsRequestInterceptorTest {
 
             op.interceptors.add(
                 FlexibleChecksumsRequestInterceptor(
-                    userSelectedChecksumAlgorithm = checksumAlgorithmName,
+                    requestChecksumAlgorithm = checksumAlgorithmName,
                     requestChecksumRequired = true,
                     requestChecksumCalculation = HttpChecksumConfigOption.WHEN_SUPPORTED,
                 ),
@@ -70,7 +70,7 @@ class FlexibleChecksumsRequestInterceptorTest {
 
         op.interceptors.add(
             FlexibleChecksumsRequestInterceptor(
-                userSelectedChecksumAlgorithm = checksumAlgorithmName,
+                requestChecksumAlgorithm = checksumAlgorithmName,
                 requestChecksumRequired = true,
                 requestChecksumCalculation = HttpChecksumConfigOption.WHEN_SUPPORTED,
             ),
@@ -95,7 +95,7 @@ class FlexibleChecksumsRequestInterceptorTest {
         assertFailsWith<ClientException> {
             op.interceptors.add(
                 FlexibleChecksumsRequestInterceptor(
-                    userSelectedChecksumAlgorithm = unsupportedChecksumAlgorithmName,
+                    requestChecksumAlgorithm = unsupportedChecksumAlgorithmName,
                     requestChecksumRequired = true,
                     requestChecksumCalculation = HttpChecksumConfigOption.WHEN_SUPPORTED,
                 ),
@@ -121,7 +121,7 @@ class FlexibleChecksumsRequestInterceptorTest {
 
         op.interceptors.add(
             FlexibleChecksumsRequestInterceptor(
-                userSelectedChecksumAlgorithm = checksumAlgorithmName,
+                requestChecksumAlgorithm = checksumAlgorithmName,
                 requestChecksumRequired = true,
                 requestChecksumCalculation = HttpChecksumConfigOption.WHEN_SUPPORTED,
             ),
@@ -189,7 +189,7 @@ class FlexibleChecksumsRequestInterceptorTest {
 
         op.interceptors.add(
             FlexibleChecksumsRequestInterceptor(
-                userSelectedChecksumAlgorithm = checksumAlgorithmName,
+                requestChecksumAlgorithm = checksumAlgorithmName,
                 requestChecksumRequired = true,
                 requestChecksumCalculation = HttpChecksumConfigOption.WHEN_SUPPORTED,
             ),

runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/interceptors/FlexibleChecksumsResponseInterceptorTest.kt

@@ -77,6 +77,7 @@ class FlexibleChecksumsResponseInterceptorTest {
                 FlexibleChecksumsResponseInterceptor(
                     responseValidationRequired = true,
                     responseChecksumValidation = HttpChecksumConfigOption.WHEN_SUPPORTED,
+                    serviceUsesCompositeChecksums = false,
                 ),
             )
 
@@ -104,6 +105,7 @@ class FlexibleChecksumsResponseInterceptorTest {
                 FlexibleChecksumsResponseInterceptor(
                     responseValidationRequired = true,
                     responseChecksumValidation = HttpChecksumConfigOption.WHEN_SUPPORTED,
+                    serviceUsesCompositeChecksums = false,
                 ),
             )
 
@@ -132,6 +134,7 @@ class FlexibleChecksumsResponseInterceptorTest {
             FlexibleChecksumsResponseInterceptor(
                 responseValidationRequired = true,
                 responseChecksumValidation = HttpChecksumConfigOption.WHEN_SUPPORTED,
+                serviceUsesCompositeChecksums = false,
             ),
         )
 
@@ -157,6 +160,7 @@ class FlexibleChecksumsResponseInterceptorTest {
             FlexibleChecksumsResponseInterceptor(
                 responseValidationRequired = true,
                 responseChecksumValidation = HttpChecksumConfigOption.WHEN_SUPPORTED,
+                serviceUsesCompositeChecksums = false,
             ),
         )
 
@@ -168,4 +172,31 @@ class FlexibleChecksumsResponseInterceptorTest {
 
         op.roundTrip(client, TestInput("input"))
     }
+
+    @Test
+    fun testSkipsValidationWhenDisabled() = runTest {
+        val req = HttpRequestBuilder()
+        val op = newTestOperation<TestInput>(req)
+
+        op.interceptors.add(
+            FlexibleChecksumsResponseInterceptor (
+                responseValidationRequired = false,
+                responseChecksumValidation = HttpChecksumConfigOption.WHEN_REQUIRED,
+                serviceUsesCompositeChecksums = false,
+            )
+        )
+
+        val responseChecksumHeaderName = "x-amz-checksum-crc32"
+
+        val responseHeaders = Headers {
+            append(responseChecksumHeaderName, "incorrect-checksum-would-throw-if-validated")
+        }
+
+        val client = getMockClient(response, responseHeaders)
+
+        val output = op.roundTrip(client, TestInput("input"))
+        output.body.readAll()
+
+        assertNull(op.context.getOrNull(ChecksumHeaderValidated))
+    }
 }

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/businessmetrics/BusinessMetricsUtils.kt

@@ -98,4 +98,7 @@ public enum class SmithyBusinessMetric(public override val identifier: String) :
     FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED("a"),
     FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED("b"),
     FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED("c"),
+    ;
+
+    override fun toString(): String = identifier
 }