Skip to content

Commit 35a1cab

Browse files
committed
fix(response): drop unused import, tighten ParsedResponse contract and handler docs
Remove the unused reified `deserialize` import in JsonResponseHandler — the only call site binds to the member `deserialize(stream, Class<T>)` overload, so the extension was dead and ktlint's no-unused-imports broke the build. Documentation and ergonomics follow-ups: - ParsedResponse.value() now states that handler failures of any type (commonly unchecked, e.g. SerdeException from the Jackson handler) propagate and are memoized, so callers do not assume IOException is the only escape. - Add a comment explaining the deliberate catch(Throwable) in the memoization path: the single-use body cannot be re-read, so even an Error is pinned rather than masked by a re-parse. - Make ParsedResponse's primary constructor internal (factories `of` / `parsedWith` remain the entry points), matching the immutable-model convention; api snapshot regenerated. - Warn on ResponseHandler.string() that it reads an unbounded body into memory, unsuitable for untrusted or large payloads. - Soften the DRAIN_CHUNK_BYTES comment so it cannot drift from the I/O segment size, and include the target type name in the null-body SerdeException message.
1 parent a735369 commit 35a1cab

5 files changed

Lines changed: 26 additions & 9 deletions

File tree

sdk-core/api/sdk-core.api

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,6 @@ public final class org/dexpace/sdk/core/http/response/LoggableResponseBody : org
11251125

11261126
public final class org/dexpace/sdk/core/http/response/ParsedResponse : java/io/Closeable {
11271127
public static final field Companion Lorg/dexpace/sdk/core/http/response/ParsedResponse$Companion;
1128-
public fun <init> (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/response/ResponseHandler;)V
11291128
public fun close ()V
11301129
public final fun getHeaders ()Lorg/dexpace/sdk/core/http/common/Headers;
11311130
public final fun getMessage ()Ljava/lang/String;

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ParsedResponse.kt

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import kotlin.concurrent.withLock
4646
* @param raw The underlying raw response. Header / status / metadata access reads from here.
4747
* @param handler Strategy that maps [raw] to the typed value on first [value] access.
4848
*/
49-
public class ParsedResponse<out T>(
49+
public class ParsedResponse<out T> internal constructor(
5050
public val raw: Response,
5151
private val handler: ResponseHandler<T>,
5252
) : Closeable {
@@ -80,8 +80,12 @@ public class ParsedResponse<out T>(
8080
* typically consumes and closes the body); subsequent calls return the same value, or
8181
* re-throw the same failure, without re-running the handler.
8282
*
83+
* Any failure the handler throws is memoized and re-thrown verbatim on every later call — not
84+
* just [IOException]. Handlers commonly throw **unchecked** exceptions (the Jackson `jsonHandler`
85+
* throws `SerdeException`), so callers should not assume the only escape is [IOException].
86+
*
8387
* @return The parsed value (which may be `null` if the handler produces `null`).
84-
* @throws IOException If the handler failed — the original failure is cached and re-thrown.
88+
* @throws IOException If the handler failed with an [IOException] — cached and re-thrown.
8589
*/
8690
@Throws(IOException::class)
8791
public fun value(): T {
@@ -94,6 +98,10 @@ public class ParsedResponse<out T>(
9498
try {
9599
Outcome.Success(handler.handle(raw))
96100
} catch (t: Throwable) {
101+
// Catch Throwable, not Exception, on purpose: once the handler has touched the
102+
// single-use body, re-running it would read an already-consumed stream. Even an
103+
// Error (e.g. an OOM mid-parse) is memoized so a later call re-throws it rather
104+
// than re-reading the body and masking the original failure.
97105
Outcome.Failure(t)
98106
}
99107
outcome = resolved

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseHandler.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ public fun interface ResponseHandler<out T> {
5757
* A handler that reads the entire response body as a UTF-8 [String] and closes the
5858
* response. A bodyless response (e.g. `204 No Content`) yields an empty string.
5959
*
60+
* **Unbounded.** This reads the whole body into a single in-memory [String] with no size
61+
* cap, so it is an unbounded-allocation vector against a hostile or misbehaving server.
62+
* Unlike the bounded body-logging path elsewhere in the SDK, it applies no limit — use it
63+
* only for trusted endpoints with bounded payloads, not for untrusted or large bodies.
64+
*
6065
* @return A stateless [String] handler.
6166
*/
6267
@JvmStatic
@@ -90,7 +95,10 @@ public fun interface ResponseHandler<out T> {
9095
}
9196
}
9297

93-
/** Per-read pump size for the discarding drain — matches the I/O segment size. */
98+
/**
99+
* Per-read pump size for the discarding drain — a reasonable chunk size. `read` treats it
100+
* as an upper bound, so the exact value is not load-bearing for correctness.
101+
*/
94102
private const val DRAIN_CHUNK_BYTES: Long = 8 * 1024
95103
}
96104
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import org.dexpace.sdk.core.http.response.Response
1212
import org.dexpace.sdk.core.http.response.ResponseHandler
1313
import org.dexpace.sdk.core.serde.Serde
1414
import org.dexpace.sdk.core.serde.SerdeException
15-
import org.dexpace.sdk.core.serde.deserialize
1615

1716
/**
1817
* A [ResponseHandler] that streams the response body through [serde]'s deserializer into a value
@@ -40,7 +39,7 @@ public fun <T> jsonHandler(
4039
type: Class<T>,
4140
): ResponseHandler<T> =
4241
ResponseHandler { response ->
43-
decode(response) { stream -> serde.deserializer.deserialize(stream, type) }
42+
decode(response, type.typeName) { stream -> serde.deserializer.deserialize(stream, type) }
4443
}
4544

4645
/**
@@ -60,7 +59,7 @@ public fun <T> jsonHandler(
6059
type: TypeReference<T>,
6160
): ResponseHandler<T> =
6261
ResponseHandler { response ->
63-
decode(response) { stream -> serde.deserializeAs(stream, type) }
62+
decode(response, type.type.typeName) { stream -> serde.deserializeAs(stream, type) }
6463
}
6564

6665
/**
@@ -70,13 +69,14 @@ public fun <T> jsonHandler(
7069
*/
7170
private inline fun <T> decode(
7271
response: Response,
72+
targetType: String,
7373
decoder: (java.io.InputStream) -> T,
7474
): T =
7575
response.use { resp ->
7676
val body =
7777
resp.body
7878
?: throw SerdeException(
79-
"Cannot deserialize a ${resp.status.code} response that has no body.",
79+
"Cannot deserialize a ${resp.status.code} response with no body into $targetType.",
8080
)
8181
try {
8282
decoder(body.source().inputStream())

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ class JsonResponseHandlerTest {
103103
.protocol(Protocol.HTTP_1_1)
104104
.status(Status.NO_CONTENT)
105105
.build()
106-
assertFailsWith<SerdeException> { jsonHandler(serde, Dto::class.java).handle(resp) }
106+
val ex = assertFailsWith<SerdeException> { jsonHandler(serde, Dto::class.java).handle(resp) }
107+
// The message names the target type so a failure log says what the decode was aiming for.
108+
assertEquals(true, ex.message?.contains(Dto::class.java.typeName) == true)
107109
}
108110

109111
@Test

0 commit comments

Comments
 (0)