Skip to content

Commit 5acb4af

Browse files
thcedthced
authored
feat: Fail fast at boot when handlers and spec disagree (#88)
The default exception handler routed MissingOperationHandlerException to a bare 500 at request time, so an OpenAPI operation declared in the spec but missing from the handler map only surfaced when a client hit it. ZAP picked this up against the example launcher and flagged seven Server Error / Application Error Disclosure findings. Validate handler/spec wiring at OpenApiServer.Builder.build() and throw IllegalStateException with the offending operationIds when: - a spec operationId has no registered handler, or - a handler is registered for an operationId not in the spec. Once the boot check is in place the dispatch-time null check, the MissingOperationHandlerException class, and its unit test are unreachable, so they are deleted. The example ServerLauncher now registers stub handlers for every operation in src/test/resources/openapi.json so the demo (and the ZAP scan) covers the full surface. ServerBaseTest grows a stubAllHandlers helper so the existing tests can keep registering the subset they care about and pick up stubs for the rest. Re-running ZAP against the updated launcher: no Server Error / no Application Error Disclosure findings; only header hardening warnings remain.
1 parent 4e48ba2 commit 5acb4af

15 files changed

Lines changed: 226 additions & 136 deletions

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Request flow when `OpenApiServer` boots (`src/main/java/com/retailsvc/http/OpenA
3030
3. Three filters run in order on every request:
3131
- `ExceptionFilter` — wraps the chain; delegates uncaught exceptions to the user-supplied `ExceptionHandler` (default in `Handlers`).
3232
- `RequestPreparationFilter` — reads the raw request body, stashes it as an exchange attribute, runs OpenAPI parameter + body validation via `DefaultValidator`, and stores the resolved `operationId` on the exchange.
33-
- `DispatchHandler` — looks up the `HttpHandler` registered for that `operationId` in the user-supplied map and invokes it. Missing handler → `MissingOperationHandlerException`.
33+
- `DispatchHandler` — looks up the `HttpHandler` registered for that `operationId` in the user-supplied map and invokes it. Handler coverage is verified at boot, so the lookup never returns `null`.
3434

3535
Key abstractions:
3636

‎src/main/java/com/retailsvc/http/MissingOperationHandlerException.java‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎src/main/java/com/retailsvc/http/OpenApiServer.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Map;
3434
import java.util.Optional;
3535
import java.util.Set;
36+
import java.util.TreeSet;
3637
import java.util.stream.Collectors;
3738
import org.slf4j.Logger;
3839
import org.slf4j.LoggerFactory;
@@ -339,6 +340,7 @@ public OpenApiServer build() throws IOException {
339340
if (!externalAuth) {
340341
validateSecurityWiring(spec, securityValidators);
341342
}
343+
validateHandlerWiring(spec, handlers);
342344
Map<String, TypeMapper> resolved = resolveBodyMappers(bodyMappers);
343345
ExceptionHandler effectiveExceptionHandler =
344346
exceptionHandler != null ? exceptionHandler : Handlers.defaultExceptionHandler();
@@ -356,6 +358,25 @@ public OpenApiServer build() throws IOException {
356358
spec, resolved, handlerConfig, port, bindAddress, shutdownTimeoutSeconds);
357359
}
358360

361+
private static void validateHandlerWiring(Spec spec, Map<String, RequestHandler> handlers) {
362+
Set<String> specOps = new TreeSet<>();
363+
for (Operation op : spec.operations()) {
364+
specOps.add(op.operationId());
365+
}
366+
Set<String> missing = new TreeSet<>(specOps);
367+
missing.removeAll(handlers.keySet());
368+
if (!missing.isEmpty()) {
369+
throw new IllegalStateException(
370+
"no handler registered for spec operationId(s): " + missing);
371+
}
372+
Set<String> unknown = new TreeSet<>(handlers.keySet());
373+
unknown.removeAll(specOps);
374+
if (!unknown.isEmpty()) {
375+
throw new IllegalStateException(
376+
"handler registered for unknown operationId(s) not in spec: " + unknown);
377+
}
378+
}
379+
359380
private static void validateSecurityWiring(Spec spec, Map<String, SchemeValidator> validators) {
360381
Set<String> referenced = new LinkedHashSet<>();
361382
for (Operation op : spec.operations()) {

‎src/main/java/com/retailsvc/http/internal/DispatchHandler.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.retailsvc.http.internal;
22

3-
import com.retailsvc.http.MissingOperationHandlerException;
43
import com.retailsvc.http.Request;
54
import com.retailsvc.http.RequestHandler;
65
import com.retailsvc.http.RequestInterceptor;
@@ -37,9 +36,6 @@ public DispatchHandler(
3736
public void handle(HttpExchange exchange) throws IOException {
3837
Request request = CURRENT.get();
3938
RequestHandler handler = handlers.get(request.operationId());
40-
if (handler == null) {
41-
throw new MissingOperationHandlerException(request.operationId());
42-
}
4339
Response response = invoke(0, request, handler);
4440
for (ResponseDecorator decorator : decorators) {
4541
response = decorator.decorate(request, response);

‎src/test/java/com/retailsvc/http/AfterResponseHookIT.java‎

Lines changed: 45 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.retailsvc.http;
22

3+
import static com.retailsvc.http.ServerBaseTest.stubAllHandlers;
34
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
45
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
56
import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
@@ -31,6 +32,8 @@ class AfterResponseHookIT {
3132
private static final String OK_PATH = "/api/v1/data";
3233
private static final String NOT_FOUND_PATH = "/api/v1/does-not-exist";
3334

35+
private static final Spec SPEC = loadSpec();
36+
3437
private static Spec loadSpec() {
3538
Gson gson = new Gson();
3639
try (InputStream in = AfterResponseHookIT.class.getResourceAsStream("/openapi.json")) {
@@ -45,7 +48,7 @@ private static Spec loadSpec() {
4548

4649
private static OpenApiServer.Builder baseBuilder() {
4750
return OpenApiServer.builder()
48-
.spec(loadSpec())
51+
.spec(SPEC)
4952
.port(0)
5053
.securityValidator("apiKeyAuth", (req, cred) -> Optional.empty())
5154
.securityValidator("bearerAuth", (req, cred) -> Optional.empty())
@@ -64,7 +67,9 @@ void globalHookFiresAfterSuccessfulResponse() throws Exception {
6467

6568
try (OpenApiServer server =
6669
baseBuilder()
67-
.handlers(Map.of(OK_OPERATION_ID, req -> Response.status(HTTP_NO_CONTENT)))
70+
.handlers(
71+
stubAllHandlers(
72+
SPEC, Map.of(OK_OPERATION_ID, req -> Response.status(HTTP_NO_CONTENT))))
6873
.afterResponseHook(
6974
(req, resp) -> {
7075
capturedRequest.set(req);
@@ -96,21 +101,23 @@ void perRequestRunnablesFireInOrder() throws Exception {
96101
try (OpenApiServer server =
97102
baseBuilder()
98103
.handlers(
99-
Map.of(
100-
OK_OPERATION_ID,
101-
req -> {
102-
req.afterResponse(
103-
() -> {
104-
log.add("first");
105-
latch.countDown();
106-
});
107-
req.afterResponse(
108-
() -> {
109-
log.add("second");
110-
latch.countDown();
111-
});
112-
return Response.status(HTTP_NO_CONTENT);
113-
}))
104+
stubAllHandlers(
105+
SPEC,
106+
Map.of(
107+
OK_OPERATION_ID,
108+
req -> {
109+
req.afterResponse(
110+
() -> {
111+
log.add("first");
112+
latch.countDown();
113+
});
114+
req.afterResponse(
115+
() -> {
116+
log.add("second");
117+
latch.countDown();
118+
});
119+
return Response.status(HTTP_NO_CONTENT);
120+
})))
114121
.build()) {
115122

116123
HttpClient.newHttpClient()
@@ -130,7 +137,9 @@ void hookExceptionDoesNotAffectClientOrOtherHooks() throws Exception {
130137

131138
try (OpenApiServer server =
132139
baseBuilder()
133-
.handlers(Map.of(OK_OPERATION_ID, req -> Response.status(HTTP_NO_CONTENT)))
140+
.handlers(
141+
stubAllHandlers(
142+
SPEC, Map.of(OK_OPERATION_ID, req -> Response.status(HTTP_NO_CONTENT))))
134143
.afterResponseHook(
135144
(req, resp) -> {
136145
throw new RuntimeException("boom");
@@ -162,11 +171,13 @@ void hookFiresOnHandlerException() throws Exception {
162171
try (OpenApiServer server =
163172
baseBuilder()
164173
.handlers(
165-
Map.of(
166-
OK_OPERATION_ID,
167-
req -> {
168-
throw new RuntimeException("kapow");
169-
}))
174+
stubAllHandlers(
175+
SPEC,
176+
Map.of(
177+
OK_OPERATION_ID,
178+
req -> {
179+
throw new RuntimeException("kapow");
180+
})))
170181
.afterResponseHook(
171182
(req, resp) -> {
172183
capturedResponse.set(resp);
@@ -194,7 +205,9 @@ void preRequestFailureSkipsHooks() throws Exception {
194205

195206
try (OpenApiServer server =
196207
baseBuilder()
197-
.handlers(Map.of(OK_OPERATION_ID, req -> Response.status(HTTP_NO_CONTENT)))
208+
.handlers(
209+
stubAllHandlers(
210+
SPEC, Map.of(OK_OPERATION_ID, req -> Response.status(HTTP_NO_CONTENT))))
198211
.afterResponseHook((req, resp) -> log.add("fired"))
199212
.build()) {
200213

@@ -219,12 +232,14 @@ void hookSeesScopedRequestAndSameThreadAsHandler() throws Exception {
219232
try (OpenApiServer server =
220233
baseBuilder()
221234
.handlers(
222-
Map.of(
223-
OK_OPERATION_ID,
224-
req -> {
225-
handlerThread.set(Thread.currentThread());
226-
return Response.status(HTTP_NO_CONTENT);
227-
}))
235+
stubAllHandlers(
236+
SPEC,
237+
Map.of(
238+
OK_OPERATION_ID,
239+
req -> {
240+
handlerThread.set(Thread.currentThread());
241+
return Response.status(HTTP_NO_CONTENT);
242+
})))
228243
.afterResponseHook(
229244
(req, resp) -> {
230245
hookScopedRequest.set(DispatchHandler.CURRENT.get());
@@ -244,33 +259,4 @@ void hookSeesScopedRequestAndSameThreadAsHandler() throws Exception {
244259
assertThat(hookThread.get()).isSameAs(handlerThread.get());
245260
}
246261
}
247-
248-
@Test
249-
void hookFiresWhenHandlerIsMissing() throws Exception {
250-
CountDownLatch latch = new CountDownLatch(1);
251-
AtomicReference<Response> capturedResponse = new AtomicReference<>();
252-
253-
try (OpenApiServer server =
254-
baseBuilder()
255-
.handlers(Map.of()) // no handler registered for OK_OPERATION_ID
256-
.afterResponseHook(
257-
(req, resp) -> {
258-
capturedResponse.set(resp);
259-
latch.countDown();
260-
})
261-
.build()) {
262-
263-
HttpResponse<Void> resp =
264-
HttpClient.newHttpClient()
265-
.send(
266-
HttpRequest.newBuilder(uri(server, OK_PATH)).GET().build(),
267-
BodyHandlers.discarding());
268-
269-
assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue();
270-
assertThat(resp.statusCode()).isEqualTo(HTTP_INTERNAL_ERROR);
271-
assertThat(capturedResponse.get()).isNotNull();
272-
assertThat(capturedResponse.get().status()).isEqualTo(HTTP_INTERNAL_ERROR);
273-
assertThat(capturedResponse.get().body()).isNull();
274-
}
275-
}
276262
}

‎src/test/java/com/retailsvc/http/DecoratorAndInterceptorIT.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ void responseDecoratorAddsHeadersOnEveryResponse() throws Exception {
2323
server =
2424
newBuilder()
2525
.spec(spec)
26-
.handlers(Map.of("get-data", ok, "post-data", ok))
26+
.handlers(stubAllHandlers(Map.of("get-data", ok, "post-data", ok)))
2727
.responseDecorator((req, resp) -> resp.withHeader("X-Correlation-Id", "decorator-cid"))
2828
.responseDecorator((req, resp) -> resp.withHeader("X-Op", req.operationId()))
2929
.port(0)
@@ -42,7 +42,7 @@ void decoratorHeaderOverridesHandlerHeader() throws Exception {
4242
server =
4343
newBuilder()
4444
.spec(spec)
45-
.handlers(Map.of("get-data", ok, "post-data", ok))
45+
.handlers(stubAllHandlers(Map.of("get-data", ok, "post-data", ok)))
4646
.responseDecorator((req, resp) -> resp.withHeader("X-Op", "decorator-wins"))
4747
.port(0)
4848
.build();
@@ -58,7 +58,7 @@ void interceptorBindsScopedValueVisibleToHandler() throws Exception {
5858
server =
5959
newBuilder()
6060
.spec(spec)
61-
.handlers(Map.of("get-data", echoTenant, "post-data", echoTenant))
61+
.handlers(stubAllHandlers(Map.of("get-data", echoTenant, "post-data", echoTenant)))
6262
.interceptor((request, next) -> ScopedValue.where(TENANT, "acme").call(next::proceed))
6363
.port(0)
6464
.build();
@@ -77,7 +77,7 @@ void interceptorsRunInRegistrationOrder() throws Exception {
7777
server =
7878
newBuilder()
7979
.spec(spec)
80-
.handlers(Map.of("get-data", ok, "post-data", ok))
80+
.handlers(stubAllHandlers(Map.of("get-data", ok, "post-data", ok)))
8181
.interceptor(
8282
(request, next) -> {
8383
trace.add("outer-before");

‎src/test/java/com/retailsvc/http/ExtraHandlersIT.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ void aliveExtraReturns204AndBypassesValidation() throws Exception {
1515
try (var s =
1616
newBuilder()
1717
.spec(spec)
18-
.handlers(Map.of())
18+
.handlers(stubAllHandlers(Map.of()))
1919
.port(0)
2020
.extraRoute("/alive", Handlers.aliveHandler())
2121
.build();
@@ -38,7 +38,7 @@ void resourceHandlerServesClasspathResource() throws Exception {
3838
try (var s =
3939
newBuilder()
4040
.spec(spec)
41-
.handlers(Map.of())
41+
.handlers(stubAllHandlers(Map.of()))
4242
.port(0)
4343
.extraRoute("/openapi.yaml", Handlers.resourceHandler("/openapi.yaml"))
4444
.build();
@@ -63,7 +63,12 @@ void textHtmlResponseIsSerializedByDefaultMapper() throws Exception {
6363
req -> Response.of(200, "<h1>hi</h1>").withContentType("text/html; charset=UTF-8");
6464

6565
try (var s =
66-
newBuilder().spec(spec).handlers(Map.of()).port(0).extraRoute("/page", html).build();
66+
newBuilder()
67+
.spec(spec)
68+
.handlers(stubAllHandlers(Map.of()))
69+
.port(0)
70+
.extraRoute("/page", html)
71+
.build();
6772
var client = httpClient()) {
6873

6974
var req =
@@ -88,7 +93,12 @@ void extraHandlerExceptionFlowsThroughExceptionHandler() throws Exception {
8893
};
8994

9095
try (var s =
91-
newBuilder().spec(spec).handlers(Map.of()).port(0).extraRoute("/boom", boom).build();
96+
newBuilder()
97+
.spec(spec)
98+
.handlers(stubAllHandlers(Map.of()))
99+
.port(0)
100+
.extraRoute("/boom", boom)
101+
.build();
92102
var client = httpClient()) {
93103

94104
var req =

0 commit comments

Comments
 (0)