Skip to content

Commit 02bec41

Browse files
committed
fix: buffer the error body in jsonHandlerOrThrow to prevent use-after-close
On a non-2xx response, jsonHandlerOrThrow threw an HttpException that carried the live, still-open response body. Handlers are typically run inside response.use { ... }, so as the exception unwound the block closed the response — and any caller that later read the exception body hit a use-after-close on a released stream. Route the error path through Response.throwOnError, which buffers a bounded (<= 1 MiB) copy of the body into the thrown exception. The body now stays readable after the response is closed. The 2xx path still streams and decodes as before; KDoc updated to state the error body is buffered.
1 parent 5d95eea commit 02bec41

2 files changed

Lines changed: 45 additions & 14 deletions

File tree

sdk-serde-jackson/src/main/kotlin/org/dexpace/sdk/serde/jackson/JsonResponseHandler.kt

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ package org.dexpace.sdk.serde.jackson
1010
import com.fasterxml.jackson.core.type.TypeReference
1111
import org.dexpace.sdk.core.http.response.Response
1212
import org.dexpace.sdk.core.http.response.ResponseHandler
13+
import org.dexpace.sdk.core.http.response.exception.HttpException
1314
import org.dexpace.sdk.core.http.response.exception.HttpExceptionFactory
15+
import org.dexpace.sdk.core.http.response.throwOnError
1416
import org.dexpace.sdk.core.serde.Serde
1517
import org.dexpace.sdk.core.serde.SerdeException
1618

@@ -67,13 +69,14 @@ public fun <T> jsonHandler(
6769
* Status-aware variant of [jsonHandler]: on a 2xx response it decodes the body into [type]; on any
6870
* non-2xx response it throws instead of attempting to deserialize an error payload as [type].
6971
*
70-
* A 4xx / 5xx response is mapped through [HttpExceptionFactory] to the matching
71-
* [org.dexpace.sdk.core.http.response.exception.HttpException] subclass, which carries the
72-
* still-unconsumed error [org.dexpace.sdk.core.http.response.ResponseBody] — the caught exception
73-
* owns closing it (e.g. via `bodySnapshot()` or typed error-body deserialization). Any other
74-
* non-success status (1xx / 3xx reaching a terminal handler) closes the response and fails with a
75-
* [SerdeException]. On the success path the body is consumed and the response closed exactly as the
76-
* plain [jsonHandler].
72+
* A 4xx / 5xx response is mapped through [Response.throwOnError] to the matching
73+
* [org.dexpace.sdk.core.http.response.exception.HttpException] subclass, whose error
74+
* [org.dexpace.sdk.core.http.response.ResponseBody] is a **buffered** in-memory copy (bounded to
75+
* 1 MiB) — so it stays readable from the caught exception (e.g. via `bodySnapshot()` or typed
76+
* error-body deserialization) even after this handler runs inside a `response.use { … }` block that
77+
* closes the live response. Any other non-success status (1xx / 3xx reaching a terminal handler)
78+
* closes the response and fails with a [SerdeException]. On the success path the body is consumed
79+
* and the response closed exactly as the plain [jsonHandler].
7780
*
7881
* @param serde The serde whose deserializer decodes the body.
7982
* @param type The non-parametric success target type.
@@ -107,18 +110,19 @@ public fun <T> jsonHandlerOrThrow(
107110
}
108111

109112
/**
110-
* Throws when [response] is not a 2xx: a 4xx / 5xx is mapped to its [HttpExceptionFactory] exception
111-
* (which retains the live error body for the caller to inspect and close); any other non-success
112-
* status closes the response and raises a [SerdeException]. Returns normally on a 2xx so the caller
113-
* proceeds to decode.
113+
* Throws when [response] is not a 2xx: a 4xx / 5xx is mapped to its [HttpException] via
114+
* [Response.throwOnError], which buffers a bounded (≤ 1 MiB) copy of the error body into the thrown
115+
* exception so it survives the live response being closed (e.g. by a surrounding `response.use { … }`);
116+
* any other non-success status closes the response and raises a [SerdeException]. Returns normally on
117+
* a 2xx so the caller proceeds to decode.
114118
*/
115119
private fun throwIfNotSuccess(response: Response) {
116120
val status = response.status
117121
if (status.isSuccess) return
118122
if (HttpExceptionFactory.isErrorStatus(status.code)) {
119-
// Hand the still-unconsumed error body to the mapped HttpException; the caught exception owns
120-
// reading and closing it. Do NOT close here or bodySnapshot()/typed error decode would fail.
121-
throw HttpExceptionFactory.fromResponse(response)
123+
// Buffers the error body (≤ 1 MiB) into the mapped HttpException and throws, so the caught
124+
// exception's body is readable even after the live response is closed. Always throws here.
125+
response.throwOnError()
122126
}
123127
response.close()
124128
throw SerdeException(

sdk-serde-jackson/src/test/kotlin/org/dexpace/sdk/serde/jackson/JsonResponseHandlerTest.kt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ import org.dexpace.sdk.core.http.response.ParsedResponse
1616
import org.dexpace.sdk.core.http.response.Response
1717
import org.dexpace.sdk.core.http.response.ResponseBody
1818
import org.dexpace.sdk.core.http.response.Status
19+
import org.dexpace.sdk.core.http.response.deserialize
1920
import org.dexpace.sdk.core.http.response.exception.HttpException
2021
import org.dexpace.sdk.core.io.BufferedSource
2122
import org.dexpace.sdk.core.io.Io
2223
import org.dexpace.sdk.core.serde.SerdeException
24+
import org.dexpace.sdk.core.serde.TypeRef
2325
import org.dexpace.sdk.io.OkioIoProvider
2426
import java.util.concurrent.atomic.AtomicInteger
2527
import kotlin.test.BeforeTest
@@ -140,6 +142,23 @@ class JsonResponseHandlerTest {
140142
assertEquals(500, ex.status.code)
141143
}
142144

145+
@Test
146+
fun `jsonHandlerOrThrow buffers the error body so it survives the response being closed`() {
147+
val serde = JacksonSerde.withDefaults()
148+
val handler = jsonHandlerOrThrow(serde, Dto::class.java)
149+
val resp = jsonResponse("""{"error":"boom"}""", status = Status.INTERNAL_SERVER_ERROR)
150+
// Mirrors handleWith: the handler runs inside response.use { }, so the live response (and its
151+
// body) is closed as the block unwinds via the thrown exception.
152+
val ex =
153+
assertFailsWith<HttpException> {
154+
resp.use { handler.handle(it) }
155+
}
156+
// The error body was buffered into the exception, so it is still readable after the outer
157+
// use{} closed the live response — no use-after-close.
158+
val body = ex.body ?: error("expected a buffered error body on the thrown HttpException")
159+
assertEquals("""{"error":"boom"}""", body.source().readString(Charsets.UTF_8))
160+
}
161+
143162
@Test
144163
fun `jsonHandlerOrThrow with a TypeReference decodes parametric types on 2xx and throws on 5xx`() {
145164
val serde = JacksonSerde.withDefaults()
@@ -154,6 +173,14 @@ class JsonResponseHandlerTest {
154173
}
155174
}
156175

176+
@Test
177+
fun `Response deserialize with a TypeRef decodes a parametric List via a JacksonSerde`() {
178+
val serde = JacksonSerde.withDefaults()
179+
val resp = jsonResponse("""[{"name":"a","score":1},{"name":"b","score":2}]""")
180+
val list: List<Dto> = resp.deserialize(serde, object : TypeRef<List<Dto>>() {})
181+
assertEquals(listOf(Dto("a", 1), Dto("b", 2)), list)
182+
}
183+
157184
@Test
158185
fun `jsonHandler composes with ParsedResponse for lazy parse-once`() {
159186
val serde = JacksonSerde.withDefaults()

0 commit comments

Comments
 (0)