fix: allow Content-Length=0 headers in the CRT HTTP engine (#819)

Author: lauzadis

.changes/85c91e25-fbc9-4416-a8bc-54991580a802.json

@@ -0,0 +1,8 @@
+{
+    "id": "85c91e25-fbc9-4416-a8bc-54991580a802",
+    "type": "bugfix",
+    "description": "Allow requests with zero content length in the CRT HTTP engine",
+    "issues": [
+        "https://github.com/awslabs/smithy-kotlin/issues/818"
+    ]
+}

runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/RequestUtil.kt

@@ -60,11 +60,7 @@ internal fun HttpRequest.toCrtRequest(callContext: CoroutineContext): aws.sdk.ko
         headers.forEach { key, values -> appendAll(key, values) }
     }
 
-    val bodyLen = body.contentLength
-    val contentLength = when {
-        bodyLen != null -> if (bodyLen > 0) bodyLen.toString() else null
-        else -> headers[CONTENT_LENGTH_HEADER]
-    }
+    val contentLength = body.contentLength?.toString() ?: headers[CONTENT_LENGTH_HEADER]
     contentLength?.let { crtHeaders.append(CONTENT_LENGTH_HEADER, it) }
 
     return aws.sdk.kotlin.crt.http.HttpRequest(method.name, url.encodedPath, crtHeaders.build(), bodyStream)

runtime/protocol/http-client-engines/http-client-engine-crt/common/test/aws/smithy/kotlin/runtime/http/engine/crt/RequestConversionTest.kt

@@ -92,7 +92,7 @@ class RequestConversionTest {
     }
 
     @Test
-    fun testEngineDoesNotAddContentLengthHeaderForEmptyBody() {
+    fun testEngineAddsContentLengthHeaderForEmptyBody() {
         val request = HttpRequest(
             HttpMethod.POST,
             Url.parse("https://test.aws.com?foo=bar"),
@@ -102,7 +102,7 @@ class RequestConversionTest {
 
         val testContext = EmptyCoroutineContext + Job()
         val crtRequest = request.toCrtRequest(testContext)
-        assertFalse(crtRequest.headers.contains("Content-Length"))
+        assertEquals("0", crtRequest.headers["Content-Length"])
     }
 
     @Test

runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpUtils.kt

@@ -57,7 +57,18 @@ internal fun HttpRequest.toOkHttpRequest(
         when (val body = body) {
             is HttpBody.Empty -> ByteArray(0).toRequestBody(null, 0, 0)
             is HttpBody.Bytes -> body.bytes().let { it.toRequestBody(null, 0, it.size) }
-            is HttpBody.SourceContent, is HttpBody.ChannelContent -> StreamingRequestBody(body, callContext)
+            is HttpBody.SourceContent, is HttpBody.ChannelContent -> {
+                val updatedBody: HttpBody = headers["Content-Length"]?.let {
+                    if (body.contentLength == null || body.contentLength == -1L) {
+                        when (body) {
+                            is HttpBody.SourceContent -> body.readFrom().toHttpBody(it.toLong())
+                            is HttpBody.ChannelContent -> body.readFrom().toHttpBody(it.toLong())
+                            else -> null
+                        }
+                    } else { null }
+                } ?: body
+                StreamingRequestBody(updatedBody, callContext)
+            }
         }
     } else {
         // assert we don't silently ignore a body even though one is unexpected here

runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/test/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpRequestTest.kt

@@ -135,4 +135,57 @@ class OkHttpRequestTest {
         actualBody.writeTo(buffer)
         assertEquals(content, buffer.readUtf8())
     }
+
+    @Test
+    fun itUsesContentLengthHeaderWhenContentLengthIsUnknown() {
+        val url = Url.parse("https://aws.amazon.com")
+        val content = "Hello OkHttp from HttpBody.Streaming".repeat(1024)
+        val contentBytes = content.encodeToByteArray()
+        val chan = SdkByteReadChannel(contentBytes)
+        val body = object : HttpBody.ChannelContent() {
+            override val contentLength: Long = -1
+            override fun readFrom(): SdkByteReadChannel = chan
+        }
+        val expectedContentLength = "5"
+        val headers = HeadersBuilder().apply {
+            append("Content-Length", expectedContentLength)
+        }.build()
+
+        val request = HttpRequest(HttpMethod.POST, url, headers, body)
+        val execContext = ExecutionContext()
+        val actual = request.toOkHttpRequest(execContext, EmptyCoroutineContext)
+
+        val actualBody = assertNotNull(actual.body)
+        assertEquals(expectedContentLength.toLong(), actualBody.contentLength())
+
+        val buffer = Buffer()
+        actualBody.writeTo(buffer)
+        assertEquals(content, buffer.readUtf8())
+    }
+
+    @Test
+    fun itDoesNotUseContentLengthHeaderWhenContentLengthIsDefined() {
+        val url = Url.parse("https://aws.amazon.com")
+        val content = "Hello OkHttp from HttpBody.Streaming".repeat(1024)
+        val contentBytes = content.encodeToByteArray()
+        val chan = SdkByteReadChannel(contentBytes)
+        val body = object : HttpBody.ChannelContent() {
+            override val contentLength: Long = contentBytes.size.toLong()
+            override fun readFrom(): SdkByteReadChannel = chan
+        }
+        val headers = HeadersBuilder().apply {
+            append("Content-Length", "9")
+        }.build()
+
+        val request = HttpRequest(HttpMethod.POST, url, headers, body)
+        val execContext = ExecutionContext()
+        val actual = request.toOkHttpRequest(execContext, EmptyCoroutineContext)
+
+        val actualBody = assertNotNull(actual.body)
+        assertEquals(request.body.contentLength, actualBody.contentLength())
+
+        val buffer = Buffer()
+        actualBody.writeTo(buffer)
+        assertEquals(content, buffer.readUtf8())
+    }
 }