fix: handle bad time formats in clock skew interceptor (#1082)

.changes/e3e20544-5633-4722-8923-106117678325.json

@@ -0,0 +1,8 @@
+{
+    "id": "e3e20544-5633-4722-8923-106117678325",
+    "type": "bugfix",
+    "description": "Gracefully degrade in clock skew interceptor when receiving a `Date` header value with a malformed date",
+    "issues": [
+        "awslabs/aws-sdk-kotlin#1293"
+    ]
+}

runtime/protocol/aws-protocol-core/common/src/aws/smithy/kotlin/runtime/awsprotocol/ClockSkewInterceptor.kt

@@ -17,6 +17,7 @@ import aws.smithy.kotlin.runtime.http.response.HttpResponse
 import aws.smithy.kotlin.runtime.http.response.header
 import aws.smithy.kotlin.runtime.telemetry.logging.logger
 import aws.smithy.kotlin.runtime.time.Instant
+import aws.smithy.kotlin.runtime.time.ParseException
 import aws.smithy.kotlin.runtime.time.until
 import kotlinx.atomicfu.*
 import kotlin.coroutines.coroutineContext
@@ -85,7 +86,12 @@ public class ClockSkewInterceptor : HttpInterceptor {
         val logger = coroutineContext.logger<ClockSkewInterceptor>()
 
         val serverTime = context.protocolResponse?.header("Date")?.let {
-            Instant.fromRfc5322(it)
+            try {
+                Instant.fromRfc5322(it)
+            } catch (e: ParseException) {
+                logger.warn(e) { "Service returned malformed \"Date\" header value \"$it\", skipping skew calculation" }
+                return context.response
+            }
         } ?: run {
             logger.debug { "service did not return \"Date\" header, skipping skew calculation" }
             return context.response

runtime/protocol/aws-protocol-core/common/test/ClockSkewInterceptorTest.kt

@@ -78,18 +78,19 @@ class ClockSkewInterceptorTest {
         assertFalse(clientTime.isSkewed(serverTime, POSSIBLE_SKEWED_RESPONSE_CODE_DESCRIPTION))
     }
 
-    @Test
-    fun testClockSkewApplied() = runTest {
-        val serverTimeString = "Wed, 14 Sep 2023 16:20:50 -0400"
-        val serverTime = Instant.fromRfc5322(serverTimeString)
-
-        val clientTimeString = "20231006T131604Z"
+    private suspend fun testRoundTrip(
+        serverTimeString: String,
+        clientTimeString: String,
+        httpStatusCode: HttpStatusCode,
+        expectException: Boolean,
+    ) {
+        val serverTime = runCatching { Instant.fromRfc5322(serverTimeString) }.getOrNull()
         val clientTime = Instant.fromIso8601(clientTimeString)
 
         val client = getMockClient(
             "bla".encodeToByteArray(),
             Headers { append("Date", serverTimeString) },
-            HttpStatusCode(403, "Forbidden"),
+            httpStatusCode,
         )
 
         val req = HttpRequestBuilder().apply {
@@ -106,48 +107,52 @@ class ClockSkewInterceptorTest {
 
         op.interceptors.add(ClockSkewInterceptor())
 
-        op.roundTrip(client, Unit)
+        if (expectException) {
+            assertFailsWith<SdkBaseException> {
+                op.roundTrip(client, Unit)
+            }
 
-        // Validate the skew got stored in execution context
-        val expectedSkew = clientTime.until(serverTime)
-        assertEquals(expectedSkew, op.context.getOrNull(HttpOperationContext.ClockSkew))
+            // Validate no skew was detected
+            assertNull(op.context.getOrNull(HttpOperationContext.ClockSkew))
+        } else {
+            op.roundTrip(client, Unit)
+
+            serverTime?.let {
+                // Validate the skew got stored in execution context
+                val expectedSkew = clientTime.until(it)
+                assertEquals(expectedSkew, op.context.getOrNull(HttpOperationContext.ClockSkew))
+            }
+        }
     }
 
     @Test
-    fun testClockSkewNotApplied() = runTest {
-        val serverTimeString = "Wed, 06 Oct 2023 13:16:04 -0000"
-        val clientTimeString = "20231006T131604Z"
-        assertEquals(Instant.fromRfc5322(serverTimeString), Instant.fromIso8601(clientTimeString))
-
-        val client = getMockClient(
-            "bla".encodeToByteArray(),
-            Headers {
-                append("Date", serverTimeString)
-            },
-            HttpStatusCode(403, POSSIBLE_SKEWED_RESPONSE_CODE_DESCRIPTION),
+    fun testClockSkewApplied() = runTest {
+        testRoundTrip(
+            serverTimeString = "Wed, 14 Sep 2023 16:20:50 -0400", // Big skew
+            clientTimeString = "20231006T131604Z",
+            httpStatusCode = HttpStatusCode.Forbidden,
+            expectException = false,
         )
+    }
 
-        val req = HttpRequestBuilder().apply {
-            body = "<Foo>bar</Foo>".encodeToByteArray().toHttpBody()
-        }
-        req.headers.append("x-amz-date", clientTimeString)
-
-        val op = newTestOperation<Unit, Unit>(req, Unit)
-
-        val clockSkewException = SdkBaseException()
-        clockSkewException.sdkErrorMetadata.attributes[ServiceErrorMetadata.ErrorCode] =
-            POSSIBLE_SKEWED_RESPONSE_CODE_DESCRIPTION
-        op.interceptors.add(FailedResultInterceptor(clockSkewException))
-
-        op.interceptors.add(ClockSkewInterceptor())
-
-        // The request should fail because it's a non-retryable error, but there should be no skew detected.
-        assertFailsWith<SdkBaseException> {
-            op.roundTrip(client, Unit)
-        }
+    @Test
+    fun testClockSkewNotApplied_NoSkew() = runTest {
+        testRoundTrip(
+            serverTimeString = "Wed, 06 Oct 2023 13:16:04 -0000", // No skew
+            clientTimeString = "20231006T131604Z",
+            httpStatusCode = HttpStatusCode(403, POSSIBLE_SKEWED_RESPONSE_CODE_DESCRIPTION),
+            expectException = true,
+        )
+    }
 
-        // Validate no skew was detected
-        assertNull(op.context.getOrNull(HttpOperationContext.ClockSkew))
+    @Test
+    fun testClockSkewNotApplied_BadDate() = runTest {
+        testRoundTrip(
+            serverTimeString = "Wed, 06 Oct 23 13:16:04 -0000", // Two digit year == ☠️
+            clientTimeString = "20231006T131604Z",
+            httpStatusCode = HttpStatusCode(403, POSSIBLE_SKEWED_RESPONSE_CODE_DESCRIPTION),
+            expectException = true,
+        )
     }
 
     /**