Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/implementation-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,8 @@ defaults (per Square: `FAIL_ON_UNKNOWN_PROPERTIES=false`, `WRITE_DATES_AS_TIMEST
> Superseded by #30 (pagination unification): `Page` now exposes the raw per-page `Response` and is `Closeable` (materialized `items` and derived `statusCode` / `headers` / `request` survive `close()`), strategies return `PageInfo` (`nextRequest == null` = end of stream), and `SimplePage` was removed.

**Status: shipped.** `Page`, `Paginator`, `PaginationStrategy`, and the three strategies
(`Cursor` / `PageNumber` / `LinkHeader`) are in `sdk-core/.../pagination`, alongside
helper types `SimplePage` and `RequestRebuilder`. `Paginator` gained a `maxPages` safety cap
(`Cursor` / `PageNumber` / `LinkHeader`) are in `sdk-core/.../pagination`, alongside the
helper type `RequestRebuilder`. `Paginator` gained a `maxPages` safety cap
(default `Long.MAX_VALUE`) beyond the original sketch, to bound runaway iteration against servers
that never advance their cursor.

Expand Down
6 changes: 3 additions & 3 deletions docs/refs-comparison.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ below records where each scheme's design was sourced from:
### Pagination

`Paginator<T>` + `Page<T>` ship in `sdk-core`, driven by a `PaginationStrategy` (cursor,
page-number, token, link-header), and `pagination.PagedIterable` wraps the result. Reference
page-number, link-header), and `pagination.PagedIterable` wraps the result. Reference
designs we drew on:

- **Square**: `SyncPagingIterable<T>` (`Iterable<T>` lazy iterator), `SyncPage<T>` (per-page holder), `BiDirectionalPage<T>` (forward + backward cursors), `CustomPager<T>` (user-implementation stub for HATEOAS).
Expand All @@ -240,7 +240,7 @@ designs we drew on:
**What shipped, and what's left:**

1. `Paginator<T>` and `Page<T>` are in `sdk-core`. `iterateAll()` returns a lazy `Iterable<T>`; `streamAll()` returns a Java 8 `Stream<T>`. Each call hands back an independent iterator with its own state.
2. The strategy is injected via `PaginationStrategy`, with concrete impls covering cursor (`next_cursor` / `prev_cursor`), page-number, token, and link-header (RFC 8288). A `maxPages` cap guards against servers that never advance their cursor.
2. The strategy is injected via `PaginationStrategy`, with three concrete impls: cursor (`CursorPaginationStrategy`, forward-only), page-number, and link-header (RFC 8288). Token-style APIs (`next_page_token`, `pageToken`, …) reuse `CursorPaginationStrategy` with a configurable query-param name, so no separate token strategy ships (see `architecture.md`). A `maxPages` cap guards against servers that never advance their cursor.
3. Async variants for `sdk-async-coroutines` (`Flow<T>`) and `sdk-async-reactor` (`Flux<T>`) are not yet built.
4. `BiDirectionalPage` is deferred until a real API needs it; Square's pattern is good when needed.

Expand Down Expand Up @@ -333,7 +333,7 @@ adapters where noted):
- **Idempotency-key step.** Auto-injects `Idempotency-Key: UUID.randomUUID()` for `POST`/`PUT`/`PATCH`; caller-set header wins; pluggable key strategy. [`pipeline/step/IdempotencyKeyStep.kt`]
- **Auth.** `Credential` family + RFC 7235 challenge parsing + Basic/Digest/Composite `ChallengeHandler`s + `AuthStep` pillar. [`auth/`, `http/pipeline/steps/`]
- **`sdk-serde-jackson` adapter.** Kotlin + JSR-310 + Jdk8 modules; `FAIL_ON_UNKNOWN_PROPERTIES` and `WRITE_DATES_AS_TIMESTAMPS` disabled; `Tristate<T>` via `TristateModule`.
- **Pagination primitives.** `Paginator<T>` + `Page<T>` + `PaginationStrategy` (cursor / page-number / token / link-header) with a `maxPages` cap; `PagedIterable` wrapper.
- **Pagination primitives.** `Paginator<T>` + `Page<T>` + `PaginationStrategy` (cursor / page-number / link-header) with a `maxPages` cap; `PagedIterable` wrapper.
- **Client identity header.** `ClientIdentityStep` building the `dexpace-sdk/<ver> jvm/<javaver>` token line.
- **Tracer event vocabulary + metrics seam.** `HttpTracer` with named retry/request/response events; `Meter`/`LongCounter`/`DoubleHistogram` separate from tracing.
- **SSE streaming.** WHATWG reader in `sdk-core`; backpressured `Flux<ServerSentEvent>` in `sdk-async-reactor`.
Expand Down
10 changes: 10 additions & 0 deletions sdk-core/api/sdk-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ public final class org/dexpace/sdk/core/http/common/Headers$Builder {
public final fun add (Lorg/dexpace/sdk/core/http/common/HttpHeaderName;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/Headers$Builder;
public final fun add (Lorg/dexpace/sdk/core/http/common/HttpHeaderName;Ljava/util/List;)Lorg/dexpace/sdk/core/http/common/Headers$Builder;
public final fun addAll (Lorg/dexpace/sdk/core/http/common/Headers;)Lorg/dexpace/sdk/core/http/common/Headers$Builder;
public final fun addUnsafeNonAscii (Ljava/lang/String;Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/Headers$Builder;
public final fun build ()Lorg/dexpace/sdk/core/http/common/Headers;
public final fun remove (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/common/Headers$Builder;
public final fun remove (Lorg/dexpace/sdk/core/http/common/HttpHeaderName;)Lorg/dexpace/sdk/core/http/common/Headers$Builder;
Expand Down Expand Up @@ -1831,6 +1832,15 @@ public final class org/dexpace/sdk/core/instrumentation/ClientLogger {
public final class org/dexpace/sdk/core/instrumentation/ClientLogger$Companion {
}

public final class org/dexpace/sdk/core/instrumentation/DroppedHeaderLogging : java/lang/Enum {
public static final field ONCE_PER_HEADER Lorg/dexpace/sdk/core/instrumentation/DroppedHeaderLogging;
public static final field PER_OCCURRENCE Lorg/dexpace/sdk/core/instrumentation/DroppedHeaderLogging;
public static final field VERBOSE_ONLY Lorg/dexpace/sdk/core/instrumentation/DroppedHeaderLogging;
public static fun getEntries ()Lkotlin/enums/EnumEntries;
public static fun valueOf (Ljava/lang/String;)Lorg/dexpace/sdk/core/instrumentation/DroppedHeaderLogging;
public static fun values ()[Lorg/dexpace/sdk/core/instrumentation/DroppedHeaderLogging;
}

public abstract interface class org/dexpace/sdk/core/instrumentation/HttpTracer {
public fun attemptFailed (Ljava/lang/Throwable;Ljava/lang/Long;)V
public fun attemptRetriesExhausted (Ljava/lang/Throwable;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,33 @@ package org.dexpace.sdk.core.http.common
* and `0x7F`), which covers CR, LF, and NUL. An embedded `\r`/`\n` is the same
* request/header-splitting vector guarded against for header values: once the name is
* serialised an attacker could inject a new header or a second request. A NUL or other control
* character is illegal in a field-name, and the two reference transports handle it differently at
* their raw API (OkHttp's `addHeader` throws unchecked, the JDK builder drops it); their adapters
* now catch and drop uniformly, but a splitting vector should never get that far. Validating here
* rejects it loudly at construction — fast, uniform, and transport-independent.
* character is illegal in a field-name; validating here rejects it loudly at construction —
* fast, uniform, and transport-independent.
* - **Any non-ASCII byte** (code point `>= 0x80`). RFC 7230 field-names are ASCII `token`s, and
* every shipped reference transport (OkHttp, the JDK `HttpClient`) rejects a non-ASCII name at
* its own model layer, so a name no transport can put on the wire is refused here at
* construction rather than accepted by the model and then silently dropped mid-dispatch.
*
* Policy: the control-character set is intentionally narrower than RFC 7230's full `tchar`
* allow-list — restricting names to `tchar` would reject some non-ASCII names that certain
* transports accept, whereas the control-character set is illegal everywhere and covers the
* splitting/injection surface. This mirrors the conservative stance taken for values in
* Policy: the accepted set is printable ASCII (`0x20`–`0x7E`) — still wider than RFC 7230's
* `tchar`, so a name a transport's stricter grammar rejects (e.g. an interior space) is left for
* the transport to drop rather than pre-judged here. Control characters and non-ASCII bytes, which
* no transport can encode, are rejected outright. This mirrors the stance taken for values in
* [requireValidHeaderValues].
*
* @return the trimmed, validated name
* @throws IllegalArgumentException if the trimmed name is blank or contains a control character
* @throws IllegalArgumentException if the trimmed name is blank, or contains a control character or
* a non-ASCII byte
*/
@JvmSynthetic
internal fun requireValidHeaderName(rawName: String): String {
val trimmed = rawName.trim()
require(trimmed.isNotEmpty()) { "Header name must not be blank." }
trimmed.forEach { ch ->
require(!isProhibitedInName(ch.code)) {
"Header name '${escapeControlCharacters(rawName)}' must not contain control characters " +
"(carriage return, line feed, NUL, or other C0/DEL bytes); " +
"such characters enable request/header splitting."
"Header name '${escapeControlCharacters(rawName)}' must be printable ASCII: it must not " +
"contain control characters (carriage return, line feed, NUL, or other C0/DEL bytes) " +
"or non-ASCII bytes; control characters enable request/header splitting, and no " +
"reference transport can encode a non-ASCII name."
}
}
return trimmed
Expand All @@ -70,12 +74,19 @@ internal fun requireValidHeaderName(rawName: String): String {
* broader control-character set closes the same splitting/injection surface the name check does
* while staying narrower than the strict field-value grammar.
*
* Non-ASCII (for example UTF-8) bytes are NOT rejected — that is the conservative stance shared
* with the name check: a value some transports accept is not refused at the model layer. [name]
* only labels the error message; the value itself is never echoed, so a secret or oversized value
* is not leaked into a log line.
* Non-ASCII bytes (code point `>= 0x80`) are rejected as well, matching OkHttp's field-value
* grammar (HTAB plus printable ASCII `0x20`–`0x7E`). The shipped transports are not unanimous here
* — the JDK `HttpClient` will serialise obs-text (`0x80`+) on the wire — so the model deliberately
* takes the *most restrictive* shipped transport's stance: a value it accepts is sendable by every
* shipped transport, and a value only some can encode is refused at construction rather than
* silently dropped by the stricter one mid-dispatch. This governs **outbound, caller-set** values;
* a header parsed from an already-received response takes the lenient
* [requireValidInboundHeaderValue] path (via `Headers.Builder.addUnsafeNonAscii`), which preserves
* the obs-text a server may legitimately send. [name] only labels the error message; the value
* itself is never echoed, so a secret or oversized value is not leaked into a log line.
*
* @throws IllegalArgumentException if any value contains a prohibited control character
* @throws IllegalArgumentException if any value contains a prohibited control character or a
* non-ASCII byte
*/
@JvmSynthetic
internal fun requireValidHeaderValues(
Expand All @@ -85,22 +96,61 @@ internal fun requireValidHeaderValues(
values.forEach { value ->
value.forEach { ch ->
require(!isProhibitedInValue(ch.code)) {
"Header value for '$name' must not contain control characters (carriage return, " +
"line feed, NUL, or other C0/DEL bytes, except horizontal tab); " +
"such characters enable request/header splitting."
"Header value for '$name' must be ASCII: it must not contain control characters " +
"(carriage return, line feed, NUL, or other C0/DEL bytes, except horizontal tab) " +
"or non-ASCII bytes; control characters enable request/header splitting, and " +
"OkHttp's field-value grammar rejects a non-ASCII value."
}
}
}
}

/** Whether [code] is a control character prohibited in a header name — the full C0 range and DEL. */
private fun isProhibitedInName(code: Int): Boolean = code <= LAST_C0_CONTROL || code == DEL_CONTROL
/**
* Validates the [value] of an *inbound* (response) header [name] leniently. Like OkHttp's
* `addUnsafeNonAscii`/`addLenient` read path, the ASCII restriction is dropped so **non-ASCII bytes
* are permitted**; unlike that path — which does no value validation at all — control characters are
* still rejected here as defence-in-depth against a misbehaving transport (a stray CR/LF/NUL has no
* place in a parsed field-value). RFC 7230 explicitly allows obs-text (`0x80`+) in field-content,
* and a server legitimately puts Latin-1 bytes in values such as a `Content-Disposition` filename;
* applying the stricter outbound grammar of [requireValidHeaderValues] to a response would silently
* strip those headers. [name] only labels the error message; the value itself is never echoed.
*
* @throws IllegalArgumentException if [value] contains a prohibited control character
*/
@JvmSynthetic
internal fun requireValidInboundHeaderValue(
name: String,
value: String,
) {
value.forEach { ch ->
require(!isProhibitedInInboundValue(ch.code)) {
"Inbound header value for '$name' must not contain control characters (carriage " +
"return, line feed, NUL, or other C0/DEL bytes, except horizontal tab)."
}
}
}

/**
* Whether [code] is a control character prohibited in a header value — the same set as for a name,
* minus horizontal tab (`0x09`), which RFC 7230 permits as field-value whitespace.
* Whether [code] is prohibited in a header name — the full C0 range, DEL, and every non-ASCII byte
* (`>= 0x80`). `code >= DEL_CONTROL` covers DEL (`0x7F`) and all of `0x80`+ in one comparison.
*/
private fun isProhibitedInValue(code: Int): Boolean =
private fun isProhibitedInName(code: Int): Boolean = code <= LAST_C0_CONTROL || code >= DEL_CONTROL

/**
* Whether [code] is prohibited in an outbound header value — the same set as for a name, minus
* horizontal tab (`0x09`), which RFC 7230 permits as field-value whitespace. Shared with [MediaType]
* so a media type's rendered form and the header value it becomes agree on the accepted byte set.
*/
@JvmSynthetic
internal fun isProhibitedInValue(code: Int): Boolean =
(code <= LAST_C0_CONTROL && code != HORIZONTAL_TAB) || code >= DEL_CONTROL

/**
* Whether [code] is prohibited in an *inbound* header value: control characters (`0x00`–`0x1F`
* except HTAB, plus DEL `0x7F`) are still rejected, but non-ASCII bytes (`0x80`+) are permitted —
* the obs-text a server may legitimately send. See [requireValidInboundHeaderValue].
*/
private fun isProhibitedInInboundValue(code: Int): Boolean =
(code <= LAST_C0_CONTROL && code != HORIZONTAL_TAB) || code == DEL_CONTROL

/**
Expand All @@ -126,7 +176,12 @@ private const val HORIZONTAL_TAB: Int = 0x09
/** Highest code point in the C0 control range (US, `0x1F`); everything at or below is illegal in a name. */
private const val LAST_C0_CONTROL: Int = 0x1F

/** The DEL control character (`0x7F`), the lone control code above the C0 range. */
/**
* The DEL control character (`0x7F`). Used as the low bound of the rejected high range: for a name
* and an outbound value, DEL and every byte above it (the non-ASCII bytes, `0x80`+) are prohibited
* (`code >= DEL_CONTROL`); an inbound value rejects only DEL itself (`code == DEL_CONTROL`),
* permitting obs-text.
*/
private const val DEL_CONTROL: Int = 0x7F

/** Radix for rendering a control character's code point as the hex digits of a `\uXXXX` escape. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,36 @@ public data class Headers private constructor(
headersMap.computeIfAbsent(canonicalKey(trimmedName)) { mutableListOf() }.addAll(values)
}

/**
* Adds an inbound (already-received) header, permitting non-ASCII (obs-text) bytes in
* [value] that the strict outbound [add] grammar rejects.
*
* A transport uses this when copying a *response*'s headers into the model. RFC 7230 allows
* obs-text (`0x80`+) in a field-value — a server legitimately puts Latin-1 bytes in values
* such as a `Content-Disposition` filename — and both reference transports surface such
* values on read, so validating them with the outbound grammar would silently drop
* legitimate response headers. The name is still validated strictly (control and non-ASCII
* bytes rejected); in the value, only the ASCII restriction is relaxed — control characters
* are still rejected. Like OkHttp's `addUnsafeNonAscii` this relaxes the value's ASCII
* restriction; unlike it (whose `addLenient` validates nothing on the value), the
* control-character guard is kept here as defence-in-depth.
*
* @param name the header name
* @param value the header value, which may contain non-ASCII bytes
* @return this builder
* @throws IllegalArgumentException if [name] is blank or contains a control/non-ASCII byte,
* or [value] contains a prohibited control character
*/
public fun addUnsafeNonAscii(
name: String,
value: String,
): Builder =
apply {
val trimmedName = requireValidHeaderName(name)
requireValidInboundHeaderValue(trimmedName, value)
headersMap.computeIfAbsent(canonicalKey(trimmedName)) { mutableListOf() }.add(value)
}

/**
* Adds a header with the specified typed name and value.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import java.util.concurrent.ConcurrentHashMap
* first caller to intern a given name "wins"; subsequent lookups with different casing
* yield the same shared instance.
*
* Whitespace is trimmed from the input before interning, and the name is validated: a blank name
* or one carrying an interior control character is rejected (see [fromString]).
* Whitespace is trimmed from the input before interning, and the name is validated: a blank name,
* or one carrying an interior control character or a non-ASCII byte, is rejected (see [fromString]).
*
* Designed for Java 8 bytecode compatibility — no APIs newer than Java 8 are used.
*/
Expand Down Expand Up @@ -220,12 +220,14 @@ public class HttpHeaderName private constructor(
*
* The name is validated up front by [requireValidHeaderName]: a blank name, or one whose
* trimmed form contains an interior control character (CR, LF, NUL, or any other C0/DEL
* byte), is rejected with an [IllegalArgumentException]. This is the same guard the
* String-keyed [Headers.Builder] API applies, so an interned name carried through the typed
* header API is guaranteed control-character-free and cannot reach a transport as a
* header-splitting vector.
* byte) or a non-ASCII byte (code point >= 0x80), is rejected with an
* [IllegalArgumentException]. This is the same guard the String-keyed [Headers.Builder] API
* applies, so an interned name carried through the typed header API is guaranteed to be
* printable ASCII — control-character-free (it cannot reach a transport as a header-splitting
* vector) and free of the non-ASCII bytes no reference transport can encode.
*
* @throws IllegalArgumentException if [name] is blank or contains a control character
* @throws IllegalArgumentException if [name] is blank, or contains a control character or a
* non-ASCII byte
*/
@JvmStatic
public fun fromString(name: String): HttpHeaderName {
Expand Down
Loading
Loading