From 3f22859524a0b830da43c204d62759b0863185e1 Mon Sep 17 00:00:00 2001 From: Ian Botsford <83236726+ianbotsf@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:51:26 -0700 Subject: [PATCH] fix: handle bad time formats in clock skew interceptor (#1082) --- .../e3e20544-5633-4722-8923-106117678325.json | 8 ++ .../awsprotocol/ClockSkewInterceptor.kt | 8 +- .../common/test/ClockSkewInterceptorTest.kt | 89 ++++++++++--------- 3 files changed, 62 insertions(+), 43 deletions(-) create mode 100644 .changes/e3e20544-5633-4722-8923-106117678325.json diff --git a/.changes/e3e20544-5633-4722-8923-106117678325.json b/.changes/e3e20544-5633-4722-8923-106117678325.json new file mode 100644 index 000000000..88dfe2c10 --- /dev/null +++ b/.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" + ] +} \ No newline at end of file diff --git a/runtime/protocol/aws-protocol-core/common/src/aws/smithy/kotlin/runtime/awsprotocol/ClockSkewInterceptor.kt b/runtime/protocol/aws-protocol-core/common/src/aws/smithy/kotlin/runtime/awsprotocol/ClockSkewInterceptor.kt index 2847e7550..f365881ab 100644 --- a/runtime/protocol/aws-protocol-core/common/src/aws/smithy/kotlin/runtime/awsprotocol/ClockSkewInterceptor.kt +++ b/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() 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 diff --git a/runtime/protocol/aws-protocol-core/common/test/ClockSkewInterceptorTest.kt b/runtime/protocol/aws-protocol-core/common/test/ClockSkewInterceptorTest.kt index d01d3c238..dfa091a2a 100644 --- a/runtime/protocol/aws-protocol-core/common/test/ClockSkewInterceptorTest.kt +++ b/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 { + 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 = "bar".encodeToByteArray().toHttpBody() - } - req.headers.append("x-amz-date", clientTimeString) - - val op = newTestOperation(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 { - 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, + ) } /**