donghyeon.kang/clean-architecture-backend-template
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

feat(httpclient): close the platform review's P0/P1/P2 findings

Browse Source 
The review found one defect shape repeated across the platform: surfaces
that were declared, bound, and documented, but that nothing read. An
operator configuring fullUrlRecording, bodyLogging, retry.policy,
validatedDnsPinning, timeout.dns, or any of ten declared metric names got a
guarantee the code never delivered. Every such surface is now in exactly one
of three states -- wired for real, rejected at startup, or registered in a
test-enforced gap list with its reason. No silent no-ops remain.

P0:
- Activate the platform from bootstrap behind app.httpclient.enabled, with a
  single auto-configuration importing the nine child configurations.
- Give the platform a strict, repository-level ENV contract: 74 leaf fields
  derived from the settings record tree, unknown APP_HTTPCLIENT_* rejected.
- Route typed HTTP service clients through the call kernel via
  KernelHttpExchangeAdapter, so they stop bypassing platform policy.
- Pin dynamic-target DNS resolution to the socket for the life of a call,
  closing the resolve-then-connect TOCTOU / rebinding window.
- Actually transmit the idempotency key, and make retry eligibility depend on
  transmission rather than on merely holding one.
- Reject reactive authentication and reactive redirect at startup instead of
  declaring support that does not function.
- Fix the Reactor-only Stable contract row so the lane stops failing.
- Stop advertising HTTP/3 on a transport that negotiates HTTP/1.

P1 covers execution and retry accounting, redirect security (per-hop target
guarding, sensitive-header stripping, 303 body handling), runtime rotation
and transport resource ownership keyed by generation, dynamic-target
hardening (subdomain matching, global-unicast classification, strict CIDR
parsing), protocol intent, pool and timeout wiring, streaming and body
limits, observability parity, and OAuth single-flight refresh on a bounded
pool with a bounded wait.

P2 covers configuration and documentation drift, the Gradle check wiring for
the four hermetic lanes, and the CI gate matrix.

Two test-quality defects surfaced while closing these: the HTTP/2 stream
saturation test ran against cleartext HTTP/1.1 while asserting nothing about
the protocol, and an OAuth contention test slept on a latch that could fire
before the callers it meant to observe. Both now assert what their names
claim.

Verification run: :adapter:outbound:httpclient:check and :app-bootstrap:check
(checkstyle, spotless, spotbugs, and the four hermetic lanes),
verifyCleanArchitectureDependencies, verifyEnvKeys, verifyOneTypePerFile,
verifyDependencyLocks, the documentation and gate-matrix verifiers, and the
performance lane against a real TLS+ALPN HTTP/2 server.

Not executed, and tracked rather than claimed: Docker/Toxiproxy fault
injection, JMH, a real QUIC/HTTP3 server, a real Spring Framework 6.2
distribution (now a delegated-pending gate), live OAuth/TLS/proxy/DNS
integration, and a whole-repository check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
DongHyeonka
2026-08-11 16:49:31 +09:00
co-authored by Claude Opus 5
parent 5f10b791d3
commit 0cd959a494
148 changed files with 26812 additions and 2368 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/httpclient/configuration-reference.md
+228
View File
@@ -0,0 +1,228 @@
# HTTP Client Platform — Configuration Reference
Every outbound call resolves exactly one **Named Client Profile**. The whole capability lives under
the `app.httpclient` prefix: profiles under `app.httpclient.clients[N]`, Dynamic Target policies
under `app.httpclient.dynamic-targets[N]`.
Design §30.1 forbids a production profile from inheriting large framework defaults. Anything a
production deployment must decide has either no default or an unusable one, and
`HttpClientStartupValidator` fails the context rather than guessing.
## The master switch
| Property | Type | Default | Environment |
|---|---|---|---|
| `app.httpclient.enabled` | boolean | `false` | `APP_HTTPCLIENT_ENABLED` |
Off is the shipped state and it is a structural one. `HttpClientPlatformAutoConfiguration` lives in
a package the composition root's component scan excludes, so while the switch is absent or false the
class is never processed and neither is anything it imports: no property is bound, and no transport
provider, connection pool, TLS context, credential, thread, gateway or actuator endpoint exists. A
malformed HTTP client setting cannot fail the startup of a deployment that never wanted outbound
HTTP.
Anything that is not exactly `true` — `yes`, `1`, blank — leaves the platform off. Turning it on
with no client declared is a startup failure carrying `HTTPCLIENT_ACTIVE_WITHOUT_CLIENTS`: a
platform with nothing to call still holds transport providers and gateways no caller can reach.
## Declaring clients from the environment
Clients are an indexed list carrying their own `name`, not a map keyed by name. A map key becomes a
segment of the environment variable and the relaxed binder normalises it, so `payment-api` and
`payment_api` would arrive as one entry with nothing said about the one that was lost. Both a
duplicate name and a name that collides once normalised fail startup.
```dotenv
APP_HTTPCLIENT_ENABLED=true
APP_HTTPCLIENT_CLIENTS_0_NAME=payment
APP_HTTPCLIENT_CLIENTS_0_BASE_URL=https://payment.example
APP_HTTPCLIENT_CLIENTS_0_ALLOWED_HOSTS_0=payment.example
APP_HTTPCLIENT_CLIENTS_0_ALLOWED_PORTS_0=443
APP_HTTPCLIENT_CLIENTS_0_REQUEST_MAX_BODY_BYTES=1048576
APP_HTTPCLIENT_CLIENTS_0_TLS_PROFILE_ID=payment
APP_HTTPCLIENT_DYNAMIC_TARGETS_0_NAME=webhook
APP_HTTPCLIENT_DYNAMIC_TARGETS_0_ALLOWED_SCHEMES_0=https
```
`docs/httpclient/env-fields.yaml` is the registry of accepted variable names. It is
derived from the settings record and held to it in both directions, and the platform refuses to
start on an `APP_HTTPCLIENT_` variable that is not in it — so
`APP_HTTPCLIENT_CLIENTS_0_TIMEUOT_TOTAL_CALL` fails startup instead of silently leaving the client
on its default budget. Unknown keys supplied through a configuration file rather than the
environment are refused by strict binding for the same reason.
Only `APP_HTTPCLIENT_ENABLED` appears in `src/.env` and `docs/registries/env-keys.yaml`. It is the
one key with a deployment-independent value; templating an indexed client in `application.yml` would
materialise a nameless client in every deployment, which the aggregate validation refuses.
## `app.httpclient.clients[N]`
| Property | Type | Default | Notes |
|---|---|---|---|
| `name` | string | — | Required, unique, and distinct from every other name once normalised for the environment |
| `mode` | `TRUSTED` \| `DYNAMIC` | `TRUSTED` | A dynamic profile may not carry a default credential |
| `base-url` | URI | — | Required for a trusted profile; no userinfo, no query |
| `allowed-hosts` | list | empty | Required in production |
| `allowed-ports` | list | empty | Compared against the effective port |
| `api` | `REST_CLIENT` \| `WEB_CLIENT` | `REST_CLIENT` | Decides blocking or reactive runtime |
| `transport` | `APACHE` \| `JDK` \| `REACTOR_NETTY` \| `JETTY` \| `SIMPLE` | `APACHE` | `SIMPLE` is rejected in production |
| `protocols` | list | `HTTP_1_1` | The default transport is Apache, whose classic client is HTTP/1.1 only; a profile that wants HTTP/2 declares it together with a transport that can deliver it. `HTTP_3` requires the experimental acknowledgement |
| `experimental-acknowledgement` | string | — | Must equal `I_ACCEPT_HTTP3_EXPERIMENTAL_SEMANTICS` |
### `pool`
| Property | Default | Meaning |
|---|---|---|
| `max-total-connections` | `50` | Socket ceiling for the runtime |
| `max-connections-per-route` | `25` | Per-upstream ceiling |
| `max-pending-acquires` | `100` | Waiting-request memory ceiling |
| `pending-acquire-timeout` | `200ms` | Pool or stream wait ceiling |
| `max-idle-time` | `30s` | Idle eviction |
| `max-life-time` | `5m` | Picks up DNS, load-balancer, and certificate changes |
| `validate-after-inactivity` | `5s` | Stale and half-open detection |
| `eviction-interval` | `15s` | Background cleanup |
| `shutdown-timeout` | `5s` | Drain deadline before forced close |
| `requires-route-pool` | `false` | Set when route-scoped limits are mandatory; the JDK transport then refuses the profile |
| `requires-bounded-pending-queue` | `false` | Same, for a bounded pending queue |
### `timeout`
| Property | Default | Meaning |
|---|---|---|
| `dns` | `300ms` | Hostname resolution |
| `connect` | `500ms` | Socket connect |
| `tls-handshake` | `1s` | TLS and ALPN |
| `proxy-connect` | `500ms` | Proxy socket or CONNECT |
| `request-write-idle` | `1s` | No progress writing the request |
| `response-header` | `2s` | Until final response headers |
| `read-idle` | `3s` | Between response chunks |
| `total-call` | `4s` | The whole logical call, including retry backoff |
| `streaming-idle` | `30s` | Silence on a long-lived stream |
`total-call` must not be shorter than `connect` or `response-header`; the validator emits
`INVALID_TIMEOUT_BUDGET` otherwise.
### `redirect`, `request`, `response`
| Property | Default | Meaning |
|---|---|---|
| `redirect.enabled` | `false` | Engine redirect handling is always off; the platform follows hops itself |
| `redirect.max-hops` | `0` | Enabling redirects with zero hops is a configuration error |
| `redirect.allow-cross-origin` | `false` | When enabled, credentials are stripped on the hop |
| `request.max-body-bytes` | `0` | Required in production |
| `request.compression` | `false` | |
| `response.max-wire-bytes` | `5242880` | Bytes on the wire |
| `response.max-decoded-bytes` | `10485760` | Bytes after decoding; hard maximum is 64 MiB |
| `response.allowed-content-types` | JSON + problem+json | Empty means "any" |
### `authentication`
| Property | Default | Meaning |
|---|---|---|
| `type` | `NONE` | One of the design §20.1 methods |
| `registration-id` | — | Required for OAuth2 |
| `scopes` | empty | Part of the token cache key |
| `audience` | — | Part of the token cache key |
| `header-name` | — | Required for `API_KEY_HEADER`; must be on the allowlist |
| `secret-reference` | — | Resolved by the deployment's secret loader, never a literal |
### `retry`
| Property | Default | Meaning |
|---|---|---|
| `policy` | `none` | Named policy for reporting |
| `max-attempts` | `1` | Attempts, not retries |
| `base-backoff` | `50ms` | |
| `max-backoff` | `200ms` | |
| `jitter` | `FULL` | `NONE` \| `FULL` \| `DECORRELATED` |
| `retry-after` | `HONOR` | `HONOR` \| `IGNORE` \| `CAP` |
| `budget` | — | Shared token bucket name |
### `tls`
| Property | Default | Meaning |
|---|---|---|
| `profile-id` | — | Required in production; the only TLS identifier the actuator exposes |
| `protocols` | `TLSv1.3, TLSv1.2` | Anything else is rejected |
| `hostname-verification` | `true` | Setting it false fails startup |
| `trust-all` | `false` | Exists only so the unsafe intent is rejectable; nothing acts on `true` |
| `allow-plain-http` | `false` | Plaintext fallback fails startup in production |
| `trust-material-reference` | — | Custom CA, resolved by the secret loader |
| `key-material-reference` | — | Client certificate for mTLS |
### `proxy` and `observability`
| Property | Default | Meaning |
|---|---|---|
| `proxy.enabled` | `false` | |
| `proxy.host` / `proxy.port` / `proxy.type` | — / `0` / `HTTP` | |
| `proxy.credential-provider` | — | Proxy authentication is separate from target authentication |
| `proxy.connect-timeout` | `500ms` | Recorded as its own metric |
| `proxy.import-ambient-no-proxy` | `false` | Ambient `NO_PROXY` never widens a validated profile |
| `observability.operation-name-required` | `true` | |
| `observability.full-url-recording` | `false` | |
| `observability.body-logging` | `false` | |
## `app.httpclient.dynamic-targets[N]`
| Property | Default | Meaning |
|---|---|---|
| `name` | — | Required, unique, and subject to the same normalisation rule as a client name |
| `allowed-schemes` | `https` | |
| `allowed-ports` | `443` | |
| `allowed-host-suffixes` | empty | |
| `allowed-hosts` | empty | Empty means "any host that survives address validation" |
| `max-redirect-hops` | `0` | Each hop repeats the full validation flow |
| `trace-propagation` | `false` | Off by default for dynamic targets |
| `blocked-cidrs` | empty | Organisation-defined internal ranges |
## Startup violation codes
`TRUSTED_BASE_URL_REQUIRED`, `BASE_URL_USERINFO_FORBIDDEN`, `BASE_URL_QUERY_FORBIDDEN`,
`PLAINTEXT_PRODUCTION_TARGET`, `ALLOWED_HOST_MISMATCH`, `ALLOWED_PORT_MISMATCH`,
`REDIRECT_POLICY_INVALID`, `REDIRECT_CROSS_ORIGIN_CREDENTIAL_POLICY_REQUIRED`,
`INVALID_TIMEOUT_BUDGET`, `RESPONSE_HARD_MAXIMUM_EXCEEDED`, `PRODUCTION_SIMPLE_FACTORY_FORBIDDEN`,
`JDK_FINE_GRAINED_POOL_UNSUPPORTED`, `HTTP3_STABLE_FORBIDDEN`,
`DYNAMIC_TARGET_TRANSPORT_UNSUPPORTED`, `DYNAMIC_DEFAULT_CREDENTIAL_FORBIDDEN`,
`OAUTH2_REGISTRATION_REQUIRED`, `API_KEY_HEADER_NAME_REQUIRED`, `TRUST_ALL_FORBIDDEN`,
`HOSTNAME_VERIFICATION_REQUIRED`, `PLAINTEXT_FALLBACK_FORBIDDEN`, `TLS_PROTOCOL_FORBIDDEN`,
`RETRY_BACKOFF_REQUIRED`, `MISSING_PRODUCTION_SETTING`, `DUPLICATE_CLIENT_NAME`,
`HTTPCLIENT_ACTIVE_WITHOUT_CLIENTS`, `DYNAMIC_BASE_URL_REQUIRED`,
`DYNAMIC_TARGET_PROXY_UNSUPPORTED`, `REACTIVE_AUTHENTICATION_UNSUPPORTED`,
`HTTP2_REQUIRED_TRANSPORT_UNSUPPORTED`, `POOL_ROUTE_EXCEEDS_TOTAL`,
`TLS_PROTOCOL_SET_REQUIRED`, `REACTIVE_REDIRECT_UNSUPPORTED`,
`RETRY_POLICY_CONTRADICTS_ATTEMPTS`, `FULL_URL_RECORDING_FORBIDDEN`, `BODY_LOGGING_FORBIDDEN`,
`DNS_TIMEOUT_UNSUPPORTED`, `PROXY_CREDENTIAL_UNSUPPORTED`, `PROXY_AMBIENT_NO_PROXY_UNSUPPORTED`.
The last three name settings the platform binds but cannot yet honour. Neither the Apache classic
client nor the JDK client exposes a DNS-resolution timeout, and no proxy-credential path exists, so
a non-default value is refused rather than accepted and ignored. Leaving the defaults alone is
unaffected — only a deliberate, unmet request fails.
Three of these are about a guarantee that used to be silently unmet rather than refused:
- `HTTP2_REQUIRED_TRANSPORT_UNSUPPORTED` — declaring `protocols: [HTTP_2]` alone states that HTTP/2
is required. Only `REACTOR_NETTY` can be configured to offer H2 and nothing else; the JDK client
treats it as a preference and negotiates HTTP/1.1, and Apache's classic client is HTTP/1.1 only.
- `POOL_ROUTE_EXCEEDS_TOTAL` — a per-route ceiling above the total is incoherent, and on Reactor,
where the per-route knob is the only one that exists, it silently becomes the effective limit.
- `TLS_PROTOCOL_SET_REQUIRED` — an empty `tls.protocols` used to pass and then let the JVM choose,
so emptying the list to "tighten" a profile loosened it.
- `REACTIVE_REDIRECT_UNSUPPORTED` — engine redirect following is disabled on every transport and
only the blocking stack has a coordinator that follows hops with per-hop re-validation. A
`WEB_CLIENT` profile with `redirect.enabled=true` did not follow redirects; the caller received the
3xx as an ordinary response. Refused until the reactive coordinator exists.
- `RETRY_POLICY_CONTRADICTS_ATTEMPTS` — `retry.policy` was read by nothing on the execution path, so
the actuator could report `none` for a profile retrying three times. The two settings must now
agree: `policy: none` requires `max-attempts: 1`, and any other policy requires more than one.
- `FULL_URL_RECORDING_FORBIDDEN` / `BODY_LOGGING_FORBIDDEN` — both settings were bindable and inert.
Recording an expanded URL puts path identifiers and query strings into unbounded metric tags;
recording bodies puts someone else's data into logs. Representable so the intent is rejectable,
refused under a production profile.
`DYNAMIC_TARGET_PROXY_UNSUPPORTED` is worth spelling out: a forward proxy resolves the hostname on
its own side, so the addresses this platform validated and pinned are not the addresses the
connection reaches. The SSRF defence would be present, correct, and bypassed — so the combination is
refused rather than served with a guarantee it cannot keep.