From d22b0282e429f24dc792b190302047132552b325 Mon Sep 17 00:00:00 2001 From: Mohsen Zainalpour Date: Wed, 5 Aug 2026 23:26:32 -0400 Subject: [PATCH] fix(intercept): reject serve responses the engine's action cannot deliver (#207) `InterceptImpl.toServeStub` projected an `IsSpec` down to the engine's intercept serve action and silently discarded everything that action cannot carry: behaviors (wait/decorate/repeat/copy/lookup/shellTransform), the `_rift` extensions (templated/script and latency/error/tcp faults), binary body mode, and the 2nd+ values of a multi-valued header. Registration returned success, so a fault-injection test stayed green while asserting against a plain 200 the author never asked for. The engine cannot carry them: `ServeStub` in `intercept_rules.rs` is exactly `{statusCode, headers: HashMap, body}` and its deserializer has no `deny_unknown_fields`, so posting the extra fields would be accepted and ignored just as quietly. Carrying them through is an engine change, not an SDK one, so this rejects instead. `requireDeliverable` collects every offending construct in one pass and throws a single `InvalidDefinition` naming all of them and pointing at `redirectTo(imposter)`, which reaches a real imposter and has full stub fidelity. It runs before the rule reaches the transport, so a rejected rule is never registered, and both entry points (`Intercept.serve` and `InterceptRuleBuilder.serve`) share the one guard site. The accepted set's wire output is unchanged. `InvalidDefinition` is widened to cover an SDK-side rejection as well as the engine's HTTP 400; the sealed leaf set is untouched, so the cross-SDK contract holds. rift-scala took the same client-side approach in achird-labs/rift-scala#147. A reflection canary pins the record components of the model types the guard enumerates, so adding a field there fails the build instead of silently reopening this bug. Closes #207 --- docs/intercept.md | 24 +- .../io/github/achirdlabs/rift/Intercept.java | 15 +- .../github/achirdlabs/rift/InterceptImpl.java | 67 +++- .../achirdlabs/rift/InterceptRuleBuilder.java | 11 +- .../rift/error/InvalidDefinition.java | 7 +- .../achirdlabs/rift/error/RiftException.java | 3 +- .../rift/InterceptServeGuardTest.java | 373 ++++++++++++++++++ 7 files changed, 493 insertions(+), 7 deletions(-) create mode 100644 rift-java-core/src/test/java/io/github/achirdlabs/rift/InterceptServeGuardTest.java diff --git a/docs/intercept.md b/docs/intercept.md index 6d80b30..61c7ab1 100644 --- a/docs/intercept.md +++ b/docs/intercept.md @@ -55,7 +55,29 @@ intercept.redirectTo("api.partner.com", partnerImposter); ``` `serve`'s response is an `IsSpec` — the same response builder the DSL uses for stub responses -(`RiftDsl.ok()`, `okJson(...)`, `status(code)`, and so on). `redirectTo` rides the same +(`RiftDsl.ok()`, `okJson(...)`, `status(code)`, and so on) — but the engine's serve action is +narrower than that builder. It carries **only** a numeric status code, **single-valued** headers, +and a text body. + +A response using anything beyond that is **rejected** with an `InvalidDefinition`, and the rule is +not registered: + +- any behavior — `after`/`waitMs`, `decorate`, `repeat`, `copy`, `lookup`, `shellTransform`; +- any `_rift` extension — `templated()`, a script, or a fault (`withLatencyFault`, + `withErrorFault`, `withTcpFault`); +- a binary body (`withBinaryBody`), which the serve action can only carry as its base64 text; +- a repeated header — `withHeader(name, a, b)`. + +```java +// Throws InvalidDefinition: the serve action has no fault concept, so without this the rule +// would register happily and then answer a plain 200. +intercept.serve("example.com", status(200).withTextBody("b").withTcpFault(Fault.CONNECTION_RESET_BY_PEER)); + +// Point at an imposter instead — it is a real stub, so every construct above works there. +intercept.redirectTo("example.com", faultyImposter); +``` + +`redirectTo` rides the same wire-level `forward` action as `forward` itself, pointed at `imposter.port()`; a rule read back from `intercept.rules()` therefore only ever reports `RuleKind.SERVE` or `RuleKind.FORWARD` — the engine has no way to echo back that a `forward` action originated from `redirectTo`. diff --git a/rift-java-core/src/main/java/io/github/achirdlabs/rift/Intercept.java b/rift-java-core/src/main/java/io/github/achirdlabs/rift/Intercept.java index 24dac51..d94aaab 100644 --- a/rift-java-core/src/main/java/io/github/achirdlabs/rift/Intercept.java +++ b/rift-java-core/src/main/java/io/github/achirdlabs/rift/Intercept.java @@ -28,7 +28,20 @@ public interface Intercept extends AutoCloseable { /** A {@link ProxySelector} routing every request through this intercept — convenience for {@code java.net.http.HttpClient}. */ ProxySelector proxySelector(); - /** Adds a rule answering requests to {@code host} directly with {@code response}, without contacting the real host. */ + /** + * Adds a rule answering requests to {@code host} directly with {@code response}, without + * contacting the real host. + * + *

The engine's serve action carries only a numeric {@code statusCode}, single-valued + * {@code headers} and a text {@code body}. A response using anything else — any behavior + * ({@code wait}/{@code decorate}/{@code repeat}/{@code copy}/{@code lookup}/{@code + * shellTransform}), any {@code _rift} extension ({@code templated}, {@code script}, or a + * latency/error/TCP fault), a binary body, or a repeated header — is rejected here rather than + * silently dropped. Use {@link #redirectTo} to reach an imposter, which has full stub fidelity. + * + * @throws io.github.achirdlabs.rift.error.InvalidDefinition if {@code response} carries a + * construct the serve action cannot deliver; the rule is not registered + */ InterceptRule serve(String host, IsSpec response); /** Adds a rule forwarding requests to {@code host} on to {@code hostPort} (a {@code host:port} on localhost). */ diff --git a/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptImpl.java b/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptImpl.java index 69d3130..d2ed499 100644 --- a/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptImpl.java +++ b/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptImpl.java @@ -2,6 +2,7 @@ import io.github.achirdlabs.rift.dsl.IsSpec; import io.github.achirdlabs.rift.error.CommunicationError; +import io.github.achirdlabs.rift.error.InvalidDefinition; import io.github.achirdlabs.rift.json.JsonArray; import io.github.achirdlabs.rift.json.JsonNumber; import io.github.achirdlabs.rift.json.JsonObject; @@ -10,6 +11,7 @@ import io.github.achirdlabs.rift.model.IsResponse; import io.github.achirdlabs.rift.model.Predicate; import io.github.achirdlabs.rift.model.Response; +import io.github.achirdlabs.rift.model.ResponseMode; import io.github.achirdlabs.rift.transport.RiftTransport; import java.net.InetSocketAddress; @@ -217,13 +219,16 @@ public void close() { * {@code statusCode}, single-valued {@code headers}, and a plain-text {@code body} (see * {@code ServeStub} in {@code intercept_rules.rs}) — narrower than the full {@code is} response * shape a stub uses (multi-value headers, a structured JSON body, behaviors, faults). Anything - * beyond status/headers/body on the given response is not carried over: the intercept Serve - * action has no behaviors/faults concept. + * beyond status/headers/body is rejected rather than dropped — see + * {@link #requireDeliverable}. + * + * @throws InvalidDefinition if the response carries a construct the serve action cannot deliver */ private static JsonObject toServeStub(IsSpec response) { if (!(response.build() instanceof Response.Is is)) { throw new IllegalStateException("unreachable: IsSpec.build() always returns Response.Is"); } + requireDeliverable(is); IsResponse ir = is.is(); JsonObject.Builder builder = JsonObject.builder(); builder.put("statusCode", JsonNumber.of(statusAsInt(ir.statusCode()))); @@ -240,6 +245,64 @@ private static JsonObject toServeStub(IsSpec response) { return builder.build(); } + /** + * Rejects a response the intercept {@code serve} action cannot deliver. + * + *

The engine's {@code ServeStub} is only {@code {statusCode, headers, body}} with + * single-valued headers, and its deserializer does not use {@code deny_unknown_fields} — + * so a richer response posted here is accepted with a {@code 200} and then silently ignored at + * request time. That is worse than a rejection: a fault-injection test written against a + * {@code serve} rule stays green while asserting on the success response it never asked for. + * + *

Every offending construct is collected in one pass so a caller learns about all of them at + * once rather than one exception per round trip. Ordering is deterministic: behaviors keep their + * declaration order and headers are insertion-ordered by {@code JsonSupport.orderedCopy}. + * + * @throws InvalidDefinition naming every offending construct, and pointing at + * {@link Intercept#redirectTo}, which reaches a real imposter and so has full stub fidelity + */ + static void requireDeliverable(Response.Is is) { + List undeliverable = new ArrayList<>(); + is.behaviors().entries().forEach(behavior -> undeliverable.add("_behaviors." + behavior.key())); + is.rift().ifPresent(rift -> { + rift.fault().ifPresent(fault -> { + if (fault.latency().isPresent()) { + undeliverable.add("_rift.fault.latency (withLatencyFault)"); + } + if (fault.error().isPresent()) { + undeliverable.add("_rift.fault.error (withErrorFault)"); + } + if (fault.tcp().isPresent()) { + undeliverable.add("_rift.fault.tcp (withTcpFault)"); + } + }); + if (rift.script().isPresent()) { + undeliverable.add("_rift.script"); + } + if (rift.templated()) { + undeliverable.add("_rift.templated (templated)"); + } + }); + IsResponse ir = is.is(); + if (ir.mode() == ResponseMode.BINARY) { + undeliverable.add("a binary body (_mode=binary, withBinaryBody)"); + } + ir.headers().forEach((name, values) -> { + if (values.size() > 1) { + undeliverable.add("repeated header '" + name + "'"); + } + }); + is.extra().keySet().forEach(key -> undeliverable.add("response key '" + key + "'")); + ir.extra().keySet().forEach(key -> undeliverable.add("is response key '" + key + "'")); + + if (!undeliverable.isEmpty()) { + throw new InvalidDefinition("intercept serve cannot deliver " + String.join(", ", undeliverable) + + " — the engine's serve action carries only statusCode, single-valued headers and body, so" + + " the rule would be registered and then answer a response you did not ask for." + + " Use redirectTo(imposter) for full stub fidelity."); + } + } + private static int statusAsInt(String statusCode) { try { return Integer.parseInt(statusCode); diff --git a/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptRuleBuilder.java b/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptRuleBuilder.java index 6a5ec63..bfd1aed 100644 --- a/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptRuleBuilder.java +++ b/rift-java-core/src/main/java/io/github/achirdlabs/rift/InterceptRuleBuilder.java @@ -40,7 +40,16 @@ public InterceptRuleBuilder when(RequestMatch match) { return this; } - /** Answers matching requests inline with {@code response}; the real host is never contacted. */ + /** + * Answers matching requests inline with {@code response}; the real host is never contacted. + * + *

Restricted to what the engine's serve action can carry — status, single-valued headers and + * a text body. See {@link Intercept#serve} for the full deliverable set; anything outside it is + * rejected rather than silently dropped. + * + * @throws io.github.achirdlabs.rift.error.InvalidDefinition if {@code response} carries a + * construct the serve action cannot deliver; the rule is not registered + */ public InterceptRule serve(IsSpec response) { return intercept.addServeRule(host, predicates, response, RuleKind.SERVE); } diff --git a/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/InvalidDefinition.java b/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/InvalidDefinition.java index 46d57f4..7da42d6 100644 --- a/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/InvalidDefinition.java +++ b/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/InvalidDefinition.java @@ -1,6 +1,11 @@ package io.github.achirdlabs.rift.error; -/** The engine rejected an imposter/stub/config definition (HTTP 400). */ +/** + * A definition was rejected: either by the engine (HTTP 400), or by this SDK before it was sent, + * when the target wire format cannot carry what the definition asks for — see + * {@link io.github.achirdlabs.rift.Intercept#serve}, whose action is narrower than the response + * builder it accepts. + */ public final class InvalidDefinition extends RiftException { public InvalidDefinition(String message) { diff --git a/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/RiftException.java b/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/RiftException.java index 19a32df..736f1d7 100644 --- a/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/RiftException.java +++ b/rift-java-core/src/main/java/io/github/achirdlabs/rift/error/RiftException.java @@ -7,7 +7,8 @@ * base class. * *