KTOR-8051 Fix EOFException on retry body check

ktorio · Jan 22, 2025 · 4938ac7 · 4938ac7
1 parent 32b6578
commit 4938ac7
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 7 deletions.
diff --git a/ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpRequestRetry.kt b/ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/HttpRequestRetry.kt
@@ -235,7 +235,6 @@ public class HttpRequestRetryConfig {
  * }
  * ```
  */
-@OptIn(InternalAPI::class)
 @Suppress("NAME_SHADOWING")
 public val HttpRequestRetry: ClientPlugin<HttpRequestRetryConfig> = createClientPlugin(
     "RetryFeature",
@@ -317,9 +316,7 @@ public val HttpRequestRetry: ClientPlugin<HttpRequestRetryConfig> = createClient
                 call = proceed(subRequest)
                 if (!shouldRetry(retryCount, maxRetries, shouldRetry, call)) {
                     // throws exception if body is corrupt
-                    if (call.response.isSaved && !call.response.rawContent.isClosedForRead) {
-                        call.response.readBytes(0)
-                    }
+                    call.response.throwOnInvalidResponseBody()
                     break
                 }
                 HttpRetryEventData(subRequest, ++retryCount, call.response, null)
@@ -431,3 +428,10 @@ private fun Throwable.isTimeoutException(): Boolean {
         exception is ConnectTimeoutException ||
         exception is SocketTimeoutException
 }
+
+@OptIn(InternalAPI::class)
+private suspend fun HttpResponse.throwOnInvalidResponseBody(): Boolean {
+    // wait for saved content to pass through intermediate processing
+    // if the encoding is wrong, then this will throw an exception
+    return isSaved && rawContent.awaitContent()
+}
diff --git a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpRequestRetryTest.kt b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/HttpRequestRetryTest.kt
@@ -4,6 +4,7 @@
 
 package io.ktor.client.tests
 
+import io.ktor.client.call.body
 import io.ktor.client.engine.mock.*
 import io.ktor.client.plugins.*
 import io.ktor.client.request.*
@@ -456,4 +457,29 @@ class HttpRequestRetryTest {
             assertEquals(HttpStatusCode.OK, response.status)
         }
     }
+
+    @Test
+    fun testResponseBodyCheckIgnoresEmpty() = testWithEngine(MockEngine) {
+        config {
+            engine {
+                addHandler { respondError(HttpStatusCode.Unauthorized, "") }
+            }
+            install(HttpRequestRetry) {
+                retryOnExceptionIf(1) { _, e ->
+                    e.printStackTrace()
+                    true
+                }
+            }
+        }
+
+        test { client ->
+            try {
+                val response = client.get { }
+                assertEquals(HttpStatusCode.Unauthorized, response.status)
+                assertEquals("", response.body())
+            } catch (cause: Throwable) {
+                fail("No exception should be thrown", cause)
+            }
+        }
+    }
 }
diff --git a/ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt b/ktor-io/common/src/io/ktor/utils/io/ByteReadChannelOperations.kt
@@ -460,12 +460,9 @@ public val ByteReadChannel.availableForRead: Int
  */
 @OptIn(InternalAPI::class)
 public suspend fun ByteReadChannel.readFully(out: ByteArray, start: Int = 0, end: Int = out.size) {
-    if (isClosedForRead) throw EOFException("Channel is already closed")
-
     var offset = start
     while (offset < end) {
         if (readBuffer.exhausted()) awaitContent()
-        if (isClosedForRead) throw EOFException("Channel is already closed")
 
         val count = min(end - offset, readBuffer.remaining.toInt())
         readBuffer.readTo(out, offset, offset + count)