From 2f1bb9f6f2f793d068cc575762314746ac416b96 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 13:43:22 +0200 Subject: [PATCH 01/14] docs: Add extras wildcard matching design --- .../2026-05-22-extras-wildcard-design.md | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-22-extras-wildcard-design.md diff --git a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md new file mode 100644 index 00000000..8ab77a3b --- /dev/null +++ b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md @@ -0,0 +1,126 @@ +# Extras Wildcard Matching — Design + +Date: 2026-05-22 +Status: Approved + +## Motivation + +Extras (registered via `OpenApiServer.Builder.extraRoute(path, handler)`) currently +require an exact path. Some real-world routes need pattern matching: + +- `/static/*` to serve any single file under `/static/` +- `/schemas/**/openapi.yaml` to expose the spec at any depth +- `/files/**` to serve a tree of static assets + +OpenAPI 3.1 has no wildcard syntax (only `{name}` templates), but extras are +explicitly outside the spec, so we are free to define a small wildcard syntax +just for them. + +## Syntax + +Two wildcards, usable anywhere in a path (not just trailing): + +- `*` — matches exactly one path segment, i.e. one or more characters with no `/`. +- `**` — matches zero or more characters, may cross `/` boundaries. + +Patterns containing neither `*` nor `**` are treated as exact paths (current +behaviour, no semantic change). + +The matched portion is NOT exposed to the handler. The handler receives the +existing `Request` shape; if it needs to inspect the URI it can already do so +via `Request.rawQuery()` and the underlying exchange. + +## Architecture + +Replace the current per-route `HttpServer.createContext(extraPath, …)` wiring +with a single shared `ExtrasRouter` registered at `/`. + +Why one router instead of per-route contexts: + +- JDK's `HttpContext` is literal-prefix only — it cannot host a wildcard. +- A unified router avoids JDK context collisions when multiple wildcards share + a static prefix (e.g. `/static/*` and `/static/legacy/**`). +- It removes the special-case `NotFoundHandler` registered at `/`; the + ExtrasRouter takes over the "no match → 404" role. + +The basePath context (e.g. `/api/v1`) keeps its existing registration. JDK +picks the longest-prefix context, so spec routes still win for basePath URIs. + +### ExtrasRouter + +New class `com.retailsvc.http.internal.ExtrasRouter` implementing +`HttpHandler`. Holds a list of compiled extras, each a record of +`(originalPath, Pattern compiled, RequestHandler handler)`. On each request: + +1. Look up by exact path first (O(1) map of original-path → handler) for the + no-wildcard case. +2. If no exact hit, iterate the wildcard list in registration order; first + regex match wins. +3. If nothing matches, render a 404 via the existing exception path. + +The existing `ExtraRouteAdapter` is reused per match — it already builds the +`Request`, invokes the handler, and renders the response. + +### Pattern compilation + +`PathPattern.compile(String raw)` returns a `Pattern` and a flag `hasWildcard`. +Compilation rules: + +- Split on `/`. For each segment: + - Literal segments → `Pattern.quote(segment)` + - `*` → `[^/]+` + - `**` → `.*` (across segments) + - Mixed segments like `prefix-*.json` are NOT supported in this iteration — + `*` and `**` must be a whole segment. Reject at compile time. +- Rejoin with `/`, anchor with `^` and `$`. + +Validation at boot: + +- Pattern must start with `/`. +- No empty segments (`//`). +- `**` may not appear adjacent to another `**` (`/foo/**/**/bar`). +- Compilation failures throw `IllegalStateException` from + `OpenApiServer.Builder.build()`. + +### Wiring changes in `OpenApiServer` + +- Remove the per-extra `httpServer.createContext(path, …)` loop. +- Build an `ExtrasRouter` from `handlerConfig.extras()`. +- Register it once at `/` with the `ExceptionFilter`. Drop the existing + `NotFoundHandler` registration — the router returns 404 itself when no + extra matches. +- The basePath context registration is unchanged. + +## Error handling + +Unchanged. Misses inside the router throw `NotFoundException`, which the +`ExceptionFilter` (still in front of the router) renders as the standard +problem+json 404. + +## Testing + +Tests added under `src/test/java/com/retailsvc/http/`: + +- Unit tests for `PathPattern`: + - exact path → no wildcard + - `/files/*` matches `/files/a` but not `/files/a/b` and not `/files/` + - `/files/**` matches `/files`, `/files/a`, `/files/a/b/c` + - `/schemas/**/openapi.yaml` matches `/schemas/a/openapi.yaml` and + `/schemas/a/b/openapi.yaml` but not `/schemas/openapi.yamlx` + - mixed-segment patterns rejected at compile + - empty segment rejected + - adjacent `**/**` rejected + +- Integration tests (`ExtrasWildcardIT`): + - `/static/*` serves matching paths, rejects deeper paths + - `/files/**` serves any depth + - `/schemas/**/openapi.yaml` serves the spec at various depths + - exact extras still work (regression for `ExactUrlMatchingIT` scenarios) + - basePath spec routes still take precedence over extras + +## Out of scope + +- Exposing matched portions as path parameters. +- Mid-segment wildcards (`prefix-*.json`). +- Per-method extras (extras still accept any method). +- Wildcards in spec paths (use OpenAPI `{name}` templates). From f935536ffe25815529679af8ef6ef1c7c3624991 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 13:46:17 +0200 Subject: [PATCH 02/14] docs: Add path-traversal protection to extras wildcard design --- .../2026-05-22-extras-wildcard-design.md | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md index 8ab77a3b..3c5fb653 100644 --- a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md +++ b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md @@ -91,11 +91,32 @@ Validation at boot: extra matches. - The basePath context registration is unchanged. +## Path-traversal protection + +Before any matching, the router validates the decoded request path +(`HttpExchange.getRequestURI().getPath()`, which is already +percent-decoded by the JDK) against these rules: + +- No segment equals `.` or `..`. +- No empty segment (no `//` in the path). +- No NUL byte (U+0000) anywhere in the path. + +A violation throws `BadRequestException` and the `ExceptionFilter` renders +the standard problem+json 400. The check runs once per request inside the +`ExtrasRouter`, before exact-or-wildcard dispatch, so even handlers that +chose to read the URI cannot see a traversal-laden path. + +The same validation does NOT run inside the basePath spec context — spec +paths are matched against an explicit template set, so a `..` segment +simply fails the exact/template match and yields a normal 404. Adding the +400 check there is out of scope (mentioned for clarity, not implemented). + ## Error handling -Unchanged. Misses inside the router throw `NotFoundException`, which the -`ExceptionFilter` (still in front of the router) renders as the standard -problem+json 404. +Unchanged for normal misses: a request that passes validation but matches +no extra throws `NotFoundException`, rendered by the `ExceptionFilter` as +problem+json 404. Traversal violations throw `BadRequestException` → 400 +(see above). ## Testing @@ -117,6 +138,8 @@ Tests added under `src/test/java/com/retailsvc/http/`: - `/schemas/**/openapi.yaml` serves the spec at various depths - exact extras still work (regression for `ExactUrlMatchingIT` scenarios) - basePath spec routes still take precedence over extras + - path-traversal: `/files/../etc/passwd`, `/files/%2e%2e/etc/passwd`, + `/files/.`, `/files//x` all return 400 ## Out of scope From e9f2509f026bcab1ae81ed9e79531671073a6c4b Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 13:48:54 +0200 Subject: [PATCH 03/14] docs: Harden extras path-traversal protection (raw+decoded checks) --- .../2026-05-22-extras-wildcard-design.md | 78 +++++++++++++++---- 1 file changed, 65 insertions(+), 13 deletions(-) diff --git a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md index 3c5fb653..60d947e0 100644 --- a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md +++ b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md @@ -93,23 +93,68 @@ Validation at boot: ## Path-traversal protection -Before any matching, the router validates the decoded request path -(`HttpExchange.getRequestURI().getPath()`, which is already -percent-decoded by the JDK) against these rules: +Before any matching, the router validates both the raw URI and the decoded +request path. The raw check catches encoded traversal tricks; the decoded +check catches literal traversal segments. Either failure throws +`BadRequestException` and the `ExceptionFilter` renders problem+json 400. -- No segment equals `.` or `..`. -- No empty segment (no `//` in the path). -- No NUL byte (U+0000) anywhere in the path. +### Raw URI rules (`HttpExchange.getRequestURI().getRawPath()`) -A violation throws `BadRequestException` and the `ExceptionFilter` renders -the standard problem+json 400. The check runs once per request inside the -`ExtrasRouter`, before exact-or-wildcard dispatch, so even handlers that -chose to read the URI cannot see a traversal-laden path. +Reject the request if the raw path contains any of the following — these +have no legitimate use in our routes and are common encoding tricks: + +- `%2e` or `%2E` (encoded `.`) — also defeats double-encoding (`%252e…`, + which decodes once to `%2e…` and would otherwise sneak past the + decoded-segment check). +- `%2f` or `%2F` (encoded `/`). +- `%5c` or `%5C` (encoded backslash). +- `%00` (encoded NUL — truncation attacks). +- A literal backslash `\` (some libraries treat it as a separator). +- Any control char in U+0000–U+001F or U+007F. + +### Decoded path rules (`HttpExchange.getRequestURI().getPath()`) + +After the JDK's single-pass percent-decoding, split on `/` and reject if: + +- Any segment equals `.` or `..`. +- Any segment is empty (`//` anywhere in the path). +- The decoded path contains NUL (U+0000) or any other control char + (U+0001–U+001F, U+007F). +- Decoding raises `URISyntaxException` or `IllegalArgumentException` + (malformed / overlong encoding) — caught and rethrown as + `BadRequestException`. + +### Order + +1. Apply raw-URI rules to `getRawPath()`. +2. Decode via `getPath()`; if decoding fails, 400. +3. Apply decoded-path rules. +4. Only then run exact-or-wildcard dispatch. + +This runs once per request inside `ExtrasRouter`, before any handler is +consulted, so even handlers that choose to read the URI cannot see a +traversal-laden path. + +### Handler responsibility + +The router stops traversal at the URI layer; it cannot police what a +handler does with the matched-but-not-exposed path. Any future handler +that maps a request portion to a filesystem location MUST also: + +- Resolve the target against a fixed base directory. +- Canonicalise via `Path.toRealPath()` and assert + `resolved.startsWith(baseReal)`. +- Refuse symlinks that escape the base. + +This document does not add such a handler, but the rule is recorded here +so it survives the next time someone adds one. + +### Out of scope (deliberate) The same validation does NOT run inside the basePath spec context — spec paths are matched against an explicit template set, so a `..` segment simply fails the exact/template match and yields a normal 404. Adding the -400 check there is out of scope (mentioned for clarity, not implemented). +400 check there is mentioned for clarity but not implemented. ## Error handling @@ -138,8 +183,15 @@ Tests added under `src/test/java/com/retailsvc/http/`: - `/schemas/**/openapi.yaml` serves the spec at various depths - exact extras still work (regression for `ExactUrlMatchingIT` scenarios) - basePath spec routes still take precedence over extras - - path-traversal: `/files/../etc/passwd`, `/files/%2e%2e/etc/passwd`, - `/files/.`, `/files//x` all return 400 + - path-traversal — all return 400: + - decoded: `/files/../etc/passwd`, `/files/./x`, `/files//x` + - single-encoded: `/files/%2e%2e/etc/passwd`, `/files/%2E/x` + - double-encoded: `/files/%252e%252e/etc/passwd` + - encoded slash: `/files/%2fetc/passwd` + - backslash: `/files/..\etc\passwd` (literal and `%5c`) + - NUL truncation: `/files/x%00.txt` + - control char: `/files/x%0a/y` + - malformed encoding: `/files/%zz` ## Out of scope From 07490a3b133f25f7b0bff51ee3cc5884c3ff877d Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 13:52:29 +0200 Subject: [PATCH 04/14] docs: Add extras wildcard implementation plan --- .../plans/2026-05-22-extras-wildcard.md | 907 ++++++++++++++++++ 1 file changed, 907 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-22-extras-wildcard.md diff --git a/docs/superpowers/plans/2026-05-22-extras-wildcard.md b/docs/superpowers/plans/2026-05-22-extras-wildcard.md new file mode 100644 index 00000000..0a5c512a --- /dev/null +++ b/docs/superpowers/plans/2026-05-22-extras-wildcard.md @@ -0,0 +1,907 @@ +# Extras Wildcard Matching Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `*` (single-segment) and `**` (any-depth) wildcards to extra routes, with strict path-traversal protection at the router layer. + +**Architecture:** Replace the per-route `httpServer.createContext(path)` loop and the `/` `NotFoundHandler` with a single `ExtrasRouter` registered at `/`. The router validates the URI against a hard-coded traversal blocklist (raw + decoded), then dispatches to the matching exact or wildcard extra via the existing `ExtraRouteAdapter`. Matching is done by precompiled `java.util.regex.Pattern` per extra. The matched portion is NOT exposed to the handler. + +**Tech Stack:** Java 25, JDK `com.sun.net.httpserver`, JUnit 5 + AssertJ + Mockito (existing test stack). + +**Spec:** `docs/superpowers/specs/2026-05-22-extras-wildcard-design.md` + +--- + +### Task 1: `PathPattern` — compile and match + +**Files:** +- Create: `src/main/java/com/retailsvc/http/internal/PathPattern.java` +- Test: `src/test/java/com/retailsvc/http/internal/PathPatternTest.java` + +- [ ] **Step 1: Write the failing test** + +```java +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; + +class PathPatternTest { + + @Test + void exactPathHasNoWildcardAndMatchesItself() { + PathPattern p = PathPattern.compile("/alive"); + assertThat(p.hasWildcard()).isFalse(); + assertThat(p.matches("/alive")).isTrue(); + assertThat(p.matches("/alive/")).isFalse(); + assertThat(p.matches("/alive232")).isFalse(); + } + + @Test + void singleStarMatchesOneSegment() { + PathPattern p = PathPattern.compile("/files/*"); + assertThat(p.hasWildcard()).isTrue(); + assertThat(p.matches("/files/a")).isTrue(); + assertThat(p.matches("/files/abc.txt")).isTrue(); + assertThat(p.matches("/files/")).isFalse(); + assertThat(p.matches("/files/a/b")).isFalse(); + } + + @Test + void doubleStarMatchesAnyDepth() { + PathPattern p = PathPattern.compile("/files/**"); + assertThat(p.matches("/files/")).isTrue(); + assertThat(p.matches("/files/a")).isTrue(); + assertThat(p.matches("/files/a/b/c")).isTrue(); + assertThat(p.matches("/files")).isFalse(); + assertThat(p.matches("/filesx/a")).isFalse(); + } + + @Test + void midPathDoubleStarSurroundedByLiterals() { + PathPattern p = PathPattern.compile("/schemas/**/openapi.yaml"); + assertThat(p.matches("/schemas/a/openapi.yaml")).isTrue(); + assertThat(p.matches("/schemas/a/b/openapi.yaml")).isTrue(); + assertThat(p.matches("/schemas/openapi.yaml")).isFalse(); + assertThat(p.matches("/schemas/a/openapi.yamlx")).isFalse(); + } + + @Test + void mixedSegmentRejected() { + assertThatThrownBy(() -> PathPattern.compile("/files/prefix-*.json")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be a whole segment"); + } + + @Test + void emptySegmentRejected() { + assertThatThrownBy(() -> PathPattern.compile("/files//a")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("empty segment"); + } + + @Test + void adjacentDoubleStarsRejected() { + assertThatThrownBy(() -> PathPattern.compile("/a/**/**/b")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("adjacent"); + } + + @Test + void mustStartWithSlash() { + assertThatThrownBy(() -> PathPattern.compile("files/*")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must start with '/'"); + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `mvn test -Dtest=PathPatternTest` +Expected: compile error — `PathPattern` does not exist. + +- [ ] **Step 3: Write minimal implementation** + +```java +package com.retailsvc.http.internal; + +import java.util.regex.Pattern; + +public final class PathPattern { + + private final String raw; + private final Pattern regex; + private final boolean wildcard; + + private PathPattern(String raw, Pattern regex, boolean wildcard) { + this.raw = raw; + this.regex = regex; + this.wildcard = wildcard; + } + + public static PathPattern compile(String raw) { + if (raw == null || !raw.startsWith("/")) { + throw new IllegalArgumentException("path must start with '/': " + raw); + } + String[] segments = raw.substring(1).split("/", -1); + StringBuilder rx = new StringBuilder("^"); + boolean hasWildcard = false; + String prev = null; + for (int i = 0; i < segments.length; i++) { + String seg = segments[i]; + if (seg.isEmpty() && !(i == segments.length - 1 && segments.length > 1 && raw.endsWith("/"))) { + throw new IllegalArgumentException("empty segment in path: " + raw); + } + if (seg.contains("*") && !seg.equals("*") && !seg.equals("**")) { + throw new IllegalArgumentException( + "'*' and '**' must be a whole segment, not " + seg + " in " + raw); + } + if ("**".equals(seg) && "**".equals(prev)) { + throw new IllegalArgumentException("adjacent '**' segments in " + raw); + } + rx.append("/"); + switch (seg) { + case "*" -> { + rx.append("[^/]+"); + hasWildcard = true; + } + case "**" -> { + rx.setLength(rx.length() - 1); + rx.append("(?:/.*)?"); + hasWildcard = true; + } + default -> rx.append(Pattern.quote(seg)); + } + prev = seg; + } + rx.append("$"); + return new PathPattern(raw, Pattern.compile(rx.toString()), hasWildcard); + } + + public boolean hasWildcard() { + return wildcard; + } + + public boolean matches(String path) { + return regex.matcher(path).matches(); + } + + public String raw() { + return raw; + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `mvn test -Dtest=PathPatternTest` +Expected: 8 tests, 0 failures. + +- [ ] **Step 5: Commit** + +```bash +git add src/main/java/com/retailsvc/http/internal/PathPattern.java \ + src/test/java/com/retailsvc/http/internal/PathPatternTest.java +git commit -m "feat: Add PathPattern for extras wildcard matching" +``` + +--- + +### Task 2: `ExtrasPathValidator` — traversal protection + +**Files:** +- Create: `src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java` +- Test: `src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java` + +- [ ] **Step 1: Write the failing test** + +```java +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.retailsvc.http.BadRequestException; +import java.net.URI; +import org.junit.jupiter.api.Test; + +class ExtrasPathValidatorTest { + + @Test + void plainPathPasses() { + URI uri = URI.create("/files/a/b.txt"); + assertThat(ExtrasPathValidator.validateAndDecode(uri)).isEqualTo("/files/a/b.txt"); + } + + @Test + void dotDotSegmentRejected() { + assertReject("/files/../etc/passwd"); + } + + @Test + void singleDotSegmentRejected() { + assertReject("/files/./x"); + } + + @Test + void emptySegmentRejected() { + assertReject("/files//x"); + } + + @Test + void encodedDotRejected() { + assertReject("/files/%2e%2e/etc/passwd"); + assertReject("/files/%2E/x"); + } + + @Test + void doubleEncodedDotRejected() { + assertReject("/files/%252e%252e/etc/passwd"); + } + + @Test + void encodedSlashRejected() { + assertReject("/files/%2fetc/passwd"); + assertReject("/files/%2Fetc/passwd"); + } + + @Test + void backslashRejected() { + assertReject("/files/x%5cy"); + assertReject("/files/x%5Cy"); + } + + @Test + void literalBackslashRejected() { + URI uri = URI.create("/files/x\\y"); + assertThatThrownBy(() -> ExtrasPathValidator.validateAndDecode(uri)) + .isInstanceOf(BadRequestException.class); + } + + @Test + void nulByteRejected() { + assertReject("/files/x%00.txt"); + } + + @Test + void controlCharRejected() { + assertReject("/files/x%0ay"); + } + + @Test + void malformedEncodingRejected() { + assertReject("/files/%zz"); + } + + private void assertReject(String raw) { + URI uri = URI.create(raw); + assertThatThrownBy(() -> ExtrasPathValidator.validateAndDecode(uri)) + .isInstanceOf(BadRequestException.class); + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `mvn test -Dtest=ExtrasPathValidatorTest` +Expected: compile error — `ExtrasPathValidator` does not exist. + +- [ ] **Step 3: Write minimal implementation** + +```java +package com.retailsvc.http.internal; + +import com.retailsvc.http.BadRequestException; +import java.net.URI; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.regex.Pattern; + +public final class ExtrasPathValidator { + + private static final Pattern ENCODED_BLOCKLIST = + Pattern.compile("(?i)%(?:2e|2f|5c|00|0[1-9a-f]|1[0-9a-f]|7f)"); + + private ExtrasPathValidator() {} + + public static String validateAndDecode(URI uri) { + String raw = uri.getRawPath(); + if (raw == null) { + throw new BadRequestException("missing path"); + } + if (ENCODED_BLOCKLIST.matcher(raw).find()) { + throw new BadRequestException("path contains disallowed percent-encoded sequence"); + } + if (raw.indexOf('\\') >= 0) { + throw new BadRequestException("path contains backslash"); + } + for (int i = 0; i < raw.length(); i++) { + char c = raw.charAt(i); + if (c < 0x20 || c == 0x7f) { + throw new BadRequestException("path contains control character"); + } + } + + String decoded; + try { + decoded = URLDecoder.decode(raw, StandardCharsets.UTF_8); + } catch (IllegalArgumentException e) { + throw new BadRequestException("malformed percent-encoding"); + } + + for (int i = 0; i < decoded.length(); i++) { + char c = decoded.charAt(i); + if (c < 0x20 || c == 0x7f) { + throw new BadRequestException("decoded path contains control character"); + } + } + + String[] segments = decoded.substring(decoded.startsWith("/") ? 1 : 0).split("/", -1); + for (int i = 0; i < segments.length; i++) { + String s = segments[i]; + if (s.isEmpty() && i != segments.length - 1) { + throw new BadRequestException("empty path segment"); + } + if (".".equals(s) || "..".equals(s)) { + throw new BadRequestException("path traversal segment"); + } + } + + return decoded; + } +} +``` + +Note: `URLDecoder.decode` treats `+` as space. This is acceptable here — we re-emit decoded only for matching against pre-decoded extra paths that themselves never contain `+`. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `mvn test -Dtest=ExtrasPathValidatorTest` +Expected: 12 tests, 0 failures. + +- [ ] **Step 5: Commit** + +```bash +git add src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java \ + src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java +git commit -m "feat: Add ExtrasPathValidator for traversal protection" +``` + +--- + +### Task 3: `ExtrasRouter` — dispatch with exact and wildcard matching + +**Files:** +- Create: `src/main/java/com/retailsvc/http/internal/ExtrasRouter.java` +- Test: `src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java` + +- [ ] **Step 1: Write the failing test** + +```java +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.retailsvc.http.BadRequestException; +import com.retailsvc.http.GsonTypeMapper; +import com.retailsvc.http.NotFoundException; +import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.Response; +import com.retailsvc.http.TypeMapper; +import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpExchange; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; + +class ExtrasRouterTest { + + @Test + void exactMatchDispatches() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put("/alive", req -> { + hit.set("alive"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/alive"); + + assertThat(hit.get()).isEqualTo("alive"); + } + + @Test + void exactMatchRequiresExactPath() { + Map extras = new LinkedHashMap<>(); + extras.put("/alive", req -> Response.empty()); + ExtrasRouter router = newRouter(extras); + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> invoke(router, "/alive232")) + .isInstanceOf(NotFoundException.class); + org.assertj.core.api.Assertions.assertThatThrownBy(() -> invoke(router, "/alive/34")) + .isInstanceOf(NotFoundException.class); + } + + @Test + void singleStarMatchesOneSegment() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put("/static/*", req -> { + hit.set("static"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/static/style.css"); + assertThat(hit.get()).isEqualTo("static"); + } + + @Test + void doubleStarMatchesAnyDepth() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put("/files/**", req -> { + hit.set("files"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/files/a/b/c"); + assertThat(hit.get()).isEqualTo("files"); + } + + @Test + void exactWinsOverWildcard() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put("/files/**", req -> { + hit.set("wild"); + return Response.empty(); + }); + extras.put("/files/special", req -> { + hit.set("exact"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/files/special"); + assertThat(hit.get()).isEqualTo("exact"); + } + + @Test + void noMatchThrowsNotFound() { + ExtrasRouter router = newRouter(Map.of()); + org.assertj.core.api.Assertions.assertThatThrownBy(() -> invoke(router, "/nope")) + .isInstanceOf(NotFoundException.class); + } + + @Test + void traversalRejected() { + Map extras = new LinkedHashMap<>(); + extras.put("/files/**", req -> Response.empty()); + ExtrasRouter router = newRouter(extras); + + org.assertj.core.api.Assertions.assertThatThrownBy( + () -> invoke(router, "/files/../etc/passwd")) + .isInstanceOf(BadRequestException.class); + } + + private static ExtrasRouter newRouter(Map extras) { + Map mappers = Map.of("application/json", new GsonTypeMapper()); + return new ExtrasRouter(extras, new ResponseRenderer(mappers)); + } + + private static void invoke(ExtrasRouter router, String path) throws Exception { + HttpExchange ex = mock(HttpExchange.class); + when(ex.getRequestMethod()).thenReturn("GET"); + when(ex.getRequestURI()).thenReturn(URI.create(path)); + when(ex.getRequestHeaders()).thenReturn(new Headers()); + when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(new byte[0])); + when(ex.getResponseHeaders()).thenReturn(new Headers()); + when(ex.getResponseBody()).thenReturn(new ByteArrayOutputStream()); + router.handle(ex); + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `mvn test -Dtest=ExtrasRouterTest` +Expected: compile error — `ExtrasRouter` does not exist. + +- [ ] **Step 3: Write minimal implementation** + +```java +package com.retailsvc.http.internal; + +import com.retailsvc.http.NotFoundException; +import com.retailsvc.http.Request; +import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.Response; +import com.retailsvc.http.spec.HttpMethod; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import java.io.IOException; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +public final class ExtrasRouter implements HttpHandler { + + private record Entry(PathPattern pattern, RequestHandler handler) {} + + private final Map exact; + private final List wildcards; + private final ResponseRenderer renderer; + + public ExtrasRouter(Map extras, ResponseRenderer renderer) { + this.renderer = renderer; + Map exactBuilder = new LinkedHashMap<>(); + List wildcardBuilder = new ArrayList<>(); + for (Map.Entry e : extras.entrySet()) { + PathPattern p = PathPattern.compile(e.getKey()); + if (p.hasWildcard()) { + wildcardBuilder.add(new Entry(p, e.getValue())); + } else { + exactBuilder.put(p.raw(), e.getValue()); + } + } + this.exact = Map.copyOf(exactBuilder); + this.wildcards = List.copyOf(wildcardBuilder); + } + + @Override + public void handle(HttpExchange exchange) throws IOException { + String decoded = ExtrasPathValidator.validateAndDecode(exchange.getRequestURI()); + + RequestHandler hit = exact.get(decoded); + if (hit == null) { + for (Entry e : wildcards) { + if (e.pattern().matches(decoded)) { + hit = e.handler(); + break; + } + } + } + if (hit == null) { + throw new NotFoundException(exchange.getRequestMethod() + " " + decoded); + } + + byte[] body = exchange.getRequestBody().readAllBytes(); + HttpMethod method = HttpMethod.parse(exchange.getRequestMethod()); + var headers = exchange.getRequestHeaders(); + Request request = + new Request( + body, + null, + null, + null, + Map.of(), + exchange.getRequestURI().getRawQuery(), + headers::getFirst, + Map.of(), + method); + Response response = hit.handle(request); + renderer.render(exchange, response); + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `mvn test -Dtest=ExtrasRouterTest` +Expected: 7 tests, 0 failures. + +- [ ] **Step 5: Commit** + +```bash +git add src/main/java/com/retailsvc/http/internal/ExtrasRouter.java \ + src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java +git commit -m "feat: Add ExtrasRouter with exact and wildcard matching" +``` + +--- + +### Task 4: Wire `ExtrasRouter` into `OpenApiServer` + +**Files:** +- Modify: `src/main/java/com/retailsvc/http/OpenApiServer.java:137-145` +- Delete: `src/main/java/com/retailsvc/http/internal/ExtraRouteAdapter.java` (replaced by `ExtrasRouter` — its `Request`-building logic is inlined into the router) +- Delete: `src/test/java/com/retailsvc/http/internal/ExtraRouteAdapterTest.java` +- Modify (if still referenced): `src/main/java/com/retailsvc/http/internal/NotFoundHandler.java` — keep file but the registration at `/` goes away. + +- [ ] **Step 1: Replace the extras registration block** + +Open `OpenApiServer.java`. Locate lines 137-145 (currently): + +```java + for (Map.Entry e : handlerConfig.extras().entrySet()) { + HttpContext extraCtx = httpServer.createContext(e.getKey()); + extraCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); + extraCtx.setHandler(new ExtraRouteAdapter(e.getValue(), renderer)); + } + + if (!"/".equals(basePath)) { + httpServer.createContext("/", new NotFoundHandler()); + } +``` + +Replace with: + +```java + ExtrasRouter extrasRouter = new ExtrasRouter(handlerConfig.extras(), renderer); + if (!"/".equals(basePath)) { + HttpContext extrasCtx = httpServer.createContext("/", extrasRouter); + extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); + } else { + // basePath is "/"; spec context already owns "/". Extras may only be + // registered alongside a non-"/" basePath, so reject at build time. + if (!handlerConfig.extras().isEmpty()) { + throw new IllegalStateException( + "extras cannot be registered when basePath is '/'"); + } + } +``` + +Update imports in `OpenApiServer.java`: + +- Remove: `import com.retailsvc.http.internal.ExtraRouteAdapter;` +- Remove: `import com.retailsvc.http.internal.NotFoundHandler;` +- Add: `import com.retailsvc.http.internal.ExtrasRouter;` + +- [ ] **Step 2: Delete obsolete files** + +```bash +git rm \ + src/main/java/com/retailsvc/http/internal/ExtraRouteAdapter.java \ + src/test/java/com/retailsvc/http/internal/ExtraRouteAdapterTest.java \ + src/main/java/com/retailsvc/http/internal/NotFoundHandler.java +``` + +- [ ] **Step 3: Run unit tests** + +Run: `mvn test` +Expected: all unit tests pass. + +- [ ] **Step 4: Run integration tests** + +Run: `mvn verify` +Expected: all IT tests pass, including the existing `ExtraHandlersIT`. + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat: Replace per-route extras wiring with ExtrasRouter" +``` + +--- + +### Task 5: Integration test `ExtrasWildcardIT` + +**Files:** +- Create: `src/test/java/com/retailsvc/http/ExtrasWildcardIT.java` + +- [ ] **Step 1: Write the failing test** + +```java +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class ExtrasWildcardIT extends ServerBaseTest { + + @Test + void singleStarMatchesOneSegment() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/static/*", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/static/x.css").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/static/a/b").statusCode()).isEqualTo(HTTP_NOT_FOUND); + assertThat(get(client, s, "/static/").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void doubleStarMatchesAnyDepth() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/files/**", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/files/a").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/files/a/b/c").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/filesx/a").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void midPathDoubleStar() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/schemas/**/openapi.yaml", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/schemas/a/openapi.yaml").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/schemas/a/b/openapi.yaml").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/schemas/openapi.yaml").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void exactExtraStillWorks() throws Exception { + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/alive", Handlers.aliveHandler()) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/alive").statusCode()).isEqualTo(204); + assertThat(get(client, s, "/alive232").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void traversalReturns400() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/files/**", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/files/../etc/passwd").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/%2e%2e/etc/passwd").statusCode()) + .isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/%252e%252e/etc/passwd").statusCode()) + .isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/%2fetc").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/x%5cy").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/x%00").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/x%0ay").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/%zz").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files//x").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/.").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + } + } + + @Test + void basePathSpecRouteWinsOverExtras() throws Exception { + RequestHandler greedy = req -> Response.of(HTTP_OK, "should not see this"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/**", greedy) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/api/v1/data").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/totally-not-a-spec-route").statusCode()).isEqualTo(HTTP_OK); + } + } + + private HttpResponse get(HttpClient client, OpenApiServer s, String path) + throws Exception { + var req = + HttpRequest.newBuilder() + .uri(URI.create("http://localhost:" + s.listenPort() + path)) + .GET() + .build(); + return client.send(req, BodyHandlers.ofString()); + } +} +``` + +- [ ] **Step 2: Run integration tests** + +Run: `mvn verify -Dit.test=ExtrasWildcardIT` +Expected: all tests pass. + +- [ ] **Step 3: Run full build** + +Run: `mvn verify` +Expected: all tests pass; no surefire/failsafe regressions. + +- [ ] **Step 4: Commit** + +```bash +git add src/test/java/com/retailsvc/http/ExtrasWildcardIT.java +git commit -m "test: Add integration tests for extras wildcard matching" +``` + +--- + +### Task 6: README documentation + +**Files:** +- Modify: `README.md` + +- [ ] **Step 1: Locate the extras section** + +Run: `grep -n "extraRoute\|Extras" README.md | head` +Identify the section that documents `extraRoute`. + +- [ ] **Step 2: Add wildcard subsection** + +Append to the extras section: + +```markdown +### Wildcards in extra routes + +Extra routes accept two wildcard tokens (these are *not* part of OpenAPI; +they apply only to extras, which are outside the spec): + +- `*` — matches exactly one path segment (no `/`). +- `**` — matches zero or more characters, may cross `/` boundaries. + +Both must appear as whole segments (`/files/*`, `/files/**`, +`/schemas/**/openapi.yaml`). Mixed-segment patterns like `prefix-*.json` +are rejected at boot. + +The matched portion is not exposed to the handler. If you map a wildcard +extra to a filesystem location, canonicalise via `Path.toRealPath()` and +assert `resolved.startsWith(baseReal)` to prevent escape — the router +blocks `.`, `..`, encoded `%2e`/`%2f`/`%5c`/`%00`, control characters and +malformed encoding with a 400, but cannot police what the handler does +with the matched path. +``` + +- [ ] **Step 3: Commit** + +```bash +git add README.md +git commit -m "docs: Document extras wildcard syntax in README" +``` + +--- + +## Done + +Branch `feat/extras-wildcard` is ready to push. Open a PR; the user opens it manually (gh CLI cannot create PRs in this repo). From 04ce19a5830062534cf7e55249b8ab70b0e4f4ef Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:11:22 +0200 Subject: [PATCH 05/14] feat: Add PathPattern for extras wildcard matching --- .../retailsvc/http/internal/PathPattern.java | 73 ++++++++++++++++++ .../http/internal/PathPatternTest.java | 75 +++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 src/main/java/com/retailsvc/http/internal/PathPattern.java create mode 100644 src/test/java/com/retailsvc/http/internal/PathPatternTest.java diff --git a/src/main/java/com/retailsvc/http/internal/PathPattern.java b/src/main/java/com/retailsvc/http/internal/PathPattern.java new file mode 100644 index 00000000..1f8eeae4 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/PathPattern.java @@ -0,0 +1,73 @@ +package com.retailsvc.http.internal; + +import java.util.regex.Pattern; + +public final class PathPattern { + + private final String raw; + private final Pattern regex; + private final boolean wildcard; + + private PathPattern(String raw, Pattern regex, boolean wildcard) { + this.raw = raw; + this.regex = regex; + this.wildcard = wildcard; + } + + public static PathPattern compile(String raw) { + if (raw == null || !raw.startsWith("/")) { + throw new IllegalArgumentException("path must start with '/': " + raw); + } + String[] segments = raw.substring(1).split("/", -1); + StringBuilder rx = new StringBuilder("^"); + boolean hasWildcard = false; + String prev = null; + for (int i = 0; i < segments.length; i++) { + String seg = segments[i]; + if (seg.isEmpty() + && !(i == segments.length - 1 && segments.length > 1 && raw.endsWith("/"))) { + throw new IllegalArgumentException("empty segment in path: " + raw); + } + if (seg.contains("*") && !seg.equals("*") && !seg.equals("**")) { + throw new IllegalArgumentException( + "'*' and '**' must be a whole segment, not " + seg + " in " + raw); + } + if ("**".equals(seg) && "**".equals(prev)) { + throw new IllegalArgumentException("adjacent '**' segments in " + raw); + } + boolean trailing = i == segments.length - 1; + switch (seg) { + case "*" -> { + rx.append("/[^/]+"); + hasWildcard = true; + } + case "**" -> { + if (trailing) { + // Slash is required; anything (including empty string) may follow it. + rx.append("/.*"); + } else { + // At least one character and a slash must appear before the next segment. + rx.append("/.+"); + } + hasWildcard = true; + } + default -> rx.append("/").append(Pattern.quote(seg)); + } + prev = seg; + } + rx.append("$"); + return new PathPattern(raw, Pattern.compile(rx.toString()), hasWildcard); + } + + public boolean hasWildcard() { + return wildcard; + } + + public boolean matches(String path) { + return regex.matcher(path).matches(); + } + + public String raw() { + return raw; + } +} diff --git a/src/test/java/com/retailsvc/http/internal/PathPatternTest.java b/src/test/java/com/retailsvc/http/internal/PathPatternTest.java new file mode 100644 index 00000000..ebccdd1b --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/PathPatternTest.java @@ -0,0 +1,75 @@ +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; + +class PathPatternTest { + + @Test + void exactPathHasNoWildcardAndMatchesItself() { + PathPattern p = PathPattern.compile("/alive"); + assertThat(p.hasWildcard()).isFalse(); + assertThat(p.matches("/alive")).isTrue(); + assertThat(p.matches("/alive/")).isFalse(); + assertThat(p.matches("/alive232")).isFalse(); + } + + @Test + void singleStarMatchesOneSegment() { + PathPattern p = PathPattern.compile("/files/*"); + assertThat(p.hasWildcard()).isTrue(); + assertThat(p.matches("/files/a")).isTrue(); + assertThat(p.matches("/files/abc.txt")).isTrue(); + assertThat(p.matches("/files/")).isFalse(); + assertThat(p.matches("/files/a/b")).isFalse(); + } + + @Test + void doubleStarMatchesAnyDepth() { + PathPattern p = PathPattern.compile("/files/**"); + assertThat(p.matches("/files/")).isTrue(); + assertThat(p.matches("/files/a")).isTrue(); + assertThat(p.matches("/files/a/b/c")).isTrue(); + assertThat(p.matches("/files")).isFalse(); + assertThat(p.matches("/filesx/a")).isFalse(); + } + + @Test + void midPathDoubleStarSurroundedByLiterals() { + PathPattern p = PathPattern.compile("/schemas/**/openapi.yaml"); + assertThat(p.matches("/schemas/a/openapi.yaml")).isTrue(); + assertThat(p.matches("/schemas/a/b/openapi.yaml")).isTrue(); + assertThat(p.matches("/schemas/openapi.yaml")).isFalse(); + assertThat(p.matches("/schemas/a/openapi.yamlx")).isFalse(); + } + + @Test + void mixedSegmentRejected() { + assertThatThrownBy(() -> PathPattern.compile("/files/prefix-*.json")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be a whole segment"); + } + + @Test + void emptySegmentRejected() { + assertThatThrownBy(() -> PathPattern.compile("/files//a")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("empty segment"); + } + + @Test + void adjacentDoubleStarsRejected() { + assertThatThrownBy(() -> PathPattern.compile("/a/**/**/b")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("adjacent"); + } + + @Test + void mustStartWithSlash() { + assertThatThrownBy(() -> PathPattern.compile("files/*")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must start with '/'"); + } +} From e227af4524fd64d2cfff54be09b4c0dfd6b68ca9 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:14:50 +0200 Subject: [PATCH 06/14] docs: Document extras wildcard syntax in README --- README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/README.md b/README.md index 98d976d9..74f397b3 100644 --- a/README.md +++ b/README.md @@ -905,6 +905,25 @@ Built-in helpers: opened and closed per request, and the handler owns its lifecycle). Throws `IllegalArgumentException` at construction if the resource or file is missing. +### Wildcards in extra routes + +Extra routes accept two wildcard tokens (these are *not* part of OpenAPI; +they apply only to extras, which are outside the spec): + +- `*` — matches exactly one path segment (no `/`). +- `**` — matches zero or more characters, may cross `/` boundaries. + +Both must appear as whole segments (`/files/*`, `/files/**`, +`/schemas/**/openapi.yaml`). Mixed-segment patterns like `prefix-*.json` +are rejected at boot. + +The matched portion is not exposed to the handler. If you map a wildcard +extra to a filesystem location, canonicalise via `Path.toRealPath()` and +assert `resolved.startsWith(baseReal)` to prevent escape — the router +blocks `.`, `..`, encoded `%2e`/`%2f`/`%5c`/`%00`, control characters and +malformed encoding with a 400, but cannot police what the handler does +with the matched path. + ### Health endpoint `Handlers.healthHandler(probe)` mounts a readiness endpoint that aggregates per-dependency From 49632af1ee061298a00b5a0abcb87de676c2ec71 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:15:57 +0200 Subject: [PATCH 07/14] docs: Clarify mid-path vs trailing ** semantics in extras spec --- docs/superpowers/specs/2026-05-22-extras-wildcard-design.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md index 60d947e0..1287c7ce 100644 --- a/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md +++ b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md @@ -21,7 +21,11 @@ just for them. Two wildcards, usable anywhere in a path (not just trailing): - `*` — matches exactly one path segment, i.e. one or more characters with no `/`. -- `**` — matches zero or more characters, may cross `/` boundaries. +- `**` — matches any number of characters including `/`. At a trailing + position (`/files/**`) it may match zero or more characters and so also + matches the bare prefix path (`/files/`). Between two literal segments + (`/schemas/**/openapi.yaml`) it must match at least one full intermediate + segment — the surrounding slashes still need real content between them. Patterns containing neither `*` nor `**` are treated as exact paths (current behaviour, no semantic change). From 75e8b59024d8cf3f7289ca689928545e95e4a15e Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:18:42 +0200 Subject: [PATCH 08/14] feat: Add ExtrasPathValidator for traversal protection --- .../http/internal/ExtrasPathValidator.java | 61 ++++++++++++++ .../internal/ExtrasPathValidatorTest.java | 84 +++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java create mode 100644 src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java diff --git a/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java new file mode 100644 index 00000000..ac83546d --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java @@ -0,0 +1,61 @@ +package com.retailsvc.http.internal; + +import com.retailsvc.http.BadRequestException; +import java.net.URI; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.regex.Pattern; + +public final class ExtrasPathValidator { + + private static final Pattern ENCODED_BLOCKLIST = + Pattern.compile("(?i)%(?:25|2e|2f|5c|00|0[1-9a-f]|1[0-9a-f]|7f)"); + + private ExtrasPathValidator() {} + + public static String validateAndDecode(URI uri) { + String raw = uri.getRawPath(); + if (raw == null) { + throw new BadRequestException("missing path"); + } + if (ENCODED_BLOCKLIST.matcher(raw).find()) { + throw new BadRequestException("path contains disallowed percent-encoded sequence"); + } + if (raw.indexOf('\\') >= 0) { + throw new BadRequestException("path contains backslash"); + } + for (int i = 0; i < raw.length(); i++) { + char c = raw.charAt(i); + if (c < 0x20 || c == 0x7f) { + throw new BadRequestException("path contains control character"); + } + } + + String decoded; + try { + decoded = URLDecoder.decode(raw, StandardCharsets.UTF_8); + } catch (IllegalArgumentException e) { + throw new BadRequestException("malformed percent-encoding"); + } + + for (int i = 0; i < decoded.length(); i++) { + char c = decoded.charAt(i); + if (c < 0x20 || c == 0x7f) { + throw new BadRequestException("decoded path contains control character"); + } + } + + String[] segments = decoded.substring(decoded.startsWith("/") ? 1 : 0).split("/", -1); + for (int i = 0; i < segments.length; i++) { + String s = segments[i]; + if (s.isEmpty() && i != segments.length - 1) { + throw new BadRequestException("empty path segment"); + } + if (".".equals(s) || "..".equals(s)) { + throw new BadRequestException("path traversal segment"); + } + } + + return decoded; + } +} diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java new file mode 100644 index 00000000..cdc1b664 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java @@ -0,0 +1,84 @@ +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.retailsvc.http.BadRequestException; +import java.net.URI; +import org.junit.jupiter.api.Test; + +class ExtrasPathValidatorTest { + + @Test + void plainPathPasses() { + URI uri = URI.create("/files/a/b.txt"); + assertThat(ExtrasPathValidator.validateAndDecode(uri)).isEqualTo("/files/a/b.txt"); + } + + @Test + void dotDotSegmentRejected() { + assertReject("/files/../etc/passwd"); + } + + @Test + void singleDotSegmentRejected() { + assertReject("/files/./x"); + } + + @Test + void emptySegmentRejected() { + assertReject("/files//x"); + } + + @Test + void encodedDotRejected() { + assertReject("/files/%2e%2e/etc/passwd"); + assertReject("/files/%2E/x"); + } + + @Test + void doubleEncodedDotRejected() { + assertReject("/files/%252e%252e/etc/passwd"); + } + + @Test + void encodedSlashRejected() { + assertReject("/files/%2fetc/passwd"); + assertReject("/files/%2Fetc/passwd"); + } + + @Test + void backslashRejected() { + assertReject("/files/x%5cy"); + assertReject("/files/x%5Cy"); + } + + @Test + void literalBackslashRejected() throws Exception { + URI uri = new URI("/files/x%5cy"); + assertThatThrownBy(() -> ExtrasPathValidator.validateAndDecode(uri)) + .isInstanceOf(BadRequestException.class); + } + + @Test + void nulByteRejected() { + assertReject("/files/x%00.txt"); + } + + @Test + void controlCharRejected() { + assertReject("/files/x%0ay"); + } + + @Test + void doubleEncodedPercentRejected() { + assertReject("/files/%25xx"); + assertReject("/files/%2500"); + } + + private void assertReject(String raw) { + URI uri = URI.create(raw); + assertThatThrownBy(() -> ExtrasPathValidator.validateAndDecode(uri)) + .isInstanceOf(BadRequestException.class); + } +} From d25b35e4d03a0bfd35714608c99fbd25ca5b8598 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:27:30 +0200 Subject: [PATCH 09/14] fix: Use URI.getPath() in ExtrasPathValidator; remove duplicate test --- .../retailsvc/http/internal/ExtrasPathValidator.java | 10 +++------- .../http/internal/ExtrasPathValidatorTest.java | 7 ------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java index ac83546d..adb90030 100644 --- a/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java +++ b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java @@ -2,8 +2,6 @@ import com.retailsvc.http.BadRequestException; import java.net.URI; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; import java.util.regex.Pattern; public final class ExtrasPathValidator { @@ -31,11 +29,9 @@ public static String validateAndDecode(URI uri) { } } - String decoded; - try { - decoded = URLDecoder.decode(raw, StandardCharsets.UTF_8); - } catch (IllegalArgumentException e) { - throw new BadRequestException("malformed percent-encoding"); + String decoded = uri.getPath(); + if (decoded == null) { + throw new BadRequestException("missing path"); } for (int i = 0; i < decoded.length(); i++) { diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java index cdc1b664..de3023b5 100644 --- a/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java +++ b/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java @@ -53,13 +53,6 @@ void backslashRejected() { assertReject("/files/x%5Cy"); } - @Test - void literalBackslashRejected() throws Exception { - URI uri = new URI("/files/x%5cy"); - assertThatThrownBy(() -> ExtrasPathValidator.validateAndDecode(uri)) - .isInstanceOf(BadRequestException.class); - } - @Test void nulByteRejected() { assertReject("/files/x%00.txt"); From 67d38d3e6d27e65fee960d0e019cdb64ad2a460c Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:29:53 +0200 Subject: [PATCH 10/14] feat: Add ExtrasRouter with exact and wildcard matching --- .../retailsvc/http/internal/ExtrasRouter.java | 75 ++++++++++ .../http/internal/ExtrasRouterTest.java | 138 ++++++++++++++++++ 2 files changed, 213 insertions(+) create mode 100644 src/main/java/com/retailsvc/http/internal/ExtrasRouter.java create mode 100644 src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java diff --git a/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java b/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java new file mode 100644 index 00000000..62e06a41 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java @@ -0,0 +1,75 @@ +package com.retailsvc.http.internal; + +import com.retailsvc.http.NotFoundException; +import com.retailsvc.http.Request; +import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.Response; +import com.retailsvc.http.spec.HttpMethod; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import java.io.IOException; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** Dispatches extra-route requests using exact and wildcard path matching. */ +public final class ExtrasRouter implements HttpHandler { + + private record Entry(PathPattern pattern, RequestHandler handler) {} + + private final Map exact; + private final List wildcards; + private final ResponseRenderer renderer; + + public ExtrasRouter(Map extras, ResponseRenderer renderer) { + this.renderer = renderer; + Map exactBuilder = new LinkedHashMap<>(); + List wildcardBuilder = new ArrayList<>(); + for (Map.Entry e : extras.entrySet()) { + PathPattern p = PathPattern.compile(e.getKey()); + if (p.hasWildcard()) { + wildcardBuilder.add(new Entry(p, e.getValue())); + } else { + exactBuilder.put(p.raw(), e.getValue()); + } + } + this.exact = Map.copyOf(exactBuilder); + this.wildcards = List.copyOf(wildcardBuilder); + } + + @Override + public void handle(HttpExchange exchange) throws IOException { + String decoded = ExtrasPathValidator.validateAndDecode(exchange.getRequestURI()); + + RequestHandler hit = exact.get(decoded); + if (hit == null) { + for (Entry e : wildcards) { + if (e.pattern().matches(decoded)) { + hit = e.handler(); + break; + } + } + } + if (hit == null) { + throw new NotFoundException(exchange.getRequestMethod() + " " + decoded); + } + + byte[] body = exchange.getRequestBody().readAllBytes(); + HttpMethod method = HttpMethod.parse(exchange.getRequestMethod()); + var headers = exchange.getRequestHeaders(); + Request request = + new Request( + body, + null, + null, + null, + Map.of(), + exchange.getRequestURI().getRawQuery(), + headers::getFirst, + Map.of(), + method); + Response response = hit.handle(request); + renderer.render(exchange, response); + } +} diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java new file mode 100644 index 00000000..b8c17e7c --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java @@ -0,0 +1,138 @@ +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.retailsvc.http.BadRequestException; +import com.retailsvc.http.GsonTypeMapper; +import com.retailsvc.http.NotFoundException; +import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.Response; +import com.retailsvc.http.TypeMapper; +import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpExchange; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; + +class ExtrasRouterTest { + + @Test + void exactMatchDispatches() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put( + "/alive", + req -> { + hit.set("alive"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/alive"); + + assertThat(hit.get()).isEqualTo("alive"); + } + + @Test + void exactMatchRequiresExactPath() { + Map extras = new LinkedHashMap<>(); + extras.put("/alive", req -> Response.empty()); + ExtrasRouter router = newRouter(extras); + + assertThatThrownBy(() -> invoke(router, "/alive232")).isInstanceOf(NotFoundException.class); + assertThatThrownBy(() -> invoke(router, "/alive/34")).isInstanceOf(NotFoundException.class); + } + + @Test + void singleStarMatchesOneSegment() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put( + "/static/*", + req -> { + hit.set("static"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/static/style.css"); + assertThat(hit.get()).isEqualTo("static"); + } + + @Test + void doubleStarMatchesAnyDepth() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put( + "/files/**", + req -> { + hit.set("files"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/files/a/b/c"); + assertThat(hit.get()).isEqualTo("files"); + } + + @Test + void exactWinsOverWildcard() throws Exception { + AtomicReference hit = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put( + "/files/**", + req -> { + hit.set("wild"); + return Response.empty(); + }); + extras.put( + "/files/special", + req -> { + hit.set("exact"); + return Response.empty(); + }); + ExtrasRouter router = newRouter(extras); + + invoke(router, "/files/special"); + assertThat(hit.get()).isEqualTo("exact"); + } + + @Test + void noMatchThrowsNotFound() { + ExtrasRouter router = newRouter(Map.of()); + assertThatThrownBy(() -> invoke(router, "/nope")).isInstanceOf(NotFoundException.class); + } + + @Test + void traversalRejected() { + Map extras = new LinkedHashMap<>(); + extras.put("/files/**", req -> Response.empty()); + ExtrasRouter router = newRouter(extras); + + assertThatThrownBy(() -> invoke(router, "/files/../etc/passwd")) + .isInstanceOf(BadRequestException.class); + } + + private static ExtrasRouter newRouter(Map extras) { + Map mappers = Map.of("application/json", new GsonTypeMapper()); + return new ExtrasRouter(extras, new ResponseRenderer(mappers)); + } + + private static void invoke(ExtrasRouter router, String path) throws Exception { + HttpExchange ex = mock(HttpExchange.class); + when(ex.getRequestMethod()).thenReturn("GET"); + when(ex.getRequestURI()).thenReturn(URI.create(path)); + when(ex.getRequestHeaders()).thenReturn(new Headers()); + when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(new byte[0])); + when(ex.getResponseHeaders()).thenReturn(new Headers()); + when(ex.getResponseBody()).thenReturn(new ByteArrayOutputStream()); + router.handle(ex); + } +} From 378add9ca75f7012a964b696d74db4d1a088dd6a Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:35:52 +0200 Subject: [PATCH 11/14] feat: Replace per-route extras wiring with ExtrasRouter Wire the single ExtrasRouter at "/" instead of creating one HttpContext per extra route. Delete ExtraRouteAdapter and NotFoundHandler, whose logic now lives in ExtrasRouter. Guard against extras when basePath is "/". --- .../com/retailsvc/http/OpenApiServer.java | 15 ++--- .../http/internal/ExtraRouteAdapter.java | 56 ------------------ .../http/internal/NotFoundHandler.java | 18 ------ .../http/internal/ExtraRouteAdapterTest.java | 58 ------------------- 4 files changed, 6 insertions(+), 141 deletions(-) delete mode 100644 src/main/java/com/retailsvc/http/internal/ExtraRouteAdapter.java delete mode 100644 src/main/java/com/retailsvc/http/internal/NotFoundHandler.java delete mode 100644 src/test/java/com/retailsvc/http/internal/ExtraRouteAdapterTest.java diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index ec54134b..671b269e 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -6,9 +6,8 @@ import com.retailsvc.http.internal.DispatchHandler; import com.retailsvc.http.internal.ExceptionFilter; -import com.retailsvc.http.internal.ExtraRouteAdapter; +import com.retailsvc.http.internal.ExtrasRouter; import com.retailsvc.http.internal.FormTypeMapper; -import com.retailsvc.http.internal.NotFoundHandler; import com.retailsvc.http.internal.PemSslContext; import com.retailsvc.http.internal.RequestPreparationFilter; import com.retailsvc.http.internal.ResponseRenderer; @@ -134,14 +133,12 @@ record HandlerConfig( handlerConfig.decorators(), renderer)); - for (Map.Entry e : handlerConfig.extras().entrySet()) { - HttpContext extraCtx = httpServer.createContext(e.getKey()); - extraCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); - extraCtx.setHandler(new ExtraRouteAdapter(e.getKey(), e.getValue(), renderer)); - } - if (!"/".equals(basePath)) { - httpServer.createContext("/", new NotFoundHandler()); + ExtrasRouter extrasRouter = new ExtrasRouter(handlerConfig.extras(), renderer); + HttpContext extrasCtx = httpServer.createContext("/", extrasRouter); + extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); + } else if (!handlerConfig.extras().isEmpty()) { + throw new IllegalStateException("extras cannot be registered when basePath is '/'"); } httpServer.start(); diff --git a/src/main/java/com/retailsvc/http/internal/ExtraRouteAdapter.java b/src/main/java/com/retailsvc/http/internal/ExtraRouteAdapter.java deleted file mode 100644 index 901b60aa..00000000 --- a/src/main/java/com/retailsvc/http/internal/ExtraRouteAdapter.java +++ /dev/null @@ -1,56 +0,0 @@ -package com.retailsvc.http.internal; - -import com.retailsvc.http.NotFoundException; -import com.retailsvc.http.Request; -import com.retailsvc.http.RequestHandler; -import com.retailsvc.http.Response; -import com.retailsvc.http.spec.HttpMethod; -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpHandler; -import java.io.IOException; -import java.util.Map; - -/** - * Bridges an extra-route {@link RequestHandler} to the underlying JDK {@link HttpHandler}. - * - *

Builds a {@link Request} with {@code operationId=null}, empty path-params, empty principals, - * raw body bytes, raw query, and the parsed HTTP method, then renders the returned {@link Response} - * through the shared {@link ResponseRenderer}. OpenAPI validation, body parsing, and security are - * intentionally bypassed — extras are by definition outside the spec. - */ -public final class ExtraRouteAdapter implements HttpHandler { - - private final String path; - private final RequestHandler handler; - private final ResponseRenderer renderer; - - public ExtraRouteAdapter(String path, RequestHandler handler, ResponseRenderer renderer) { - this.path = path; - this.handler = handler; - this.renderer = renderer; - } - - @Override - public void handle(HttpExchange exchange) throws IOException { - String requested = exchange.getRequestURI().getPath(); - if (!path.equals(requested)) { - throw new NotFoundException(exchange.getRequestMethod() + " " + requested); - } - byte[] body = exchange.getRequestBody().readAllBytes(); - HttpMethod method = HttpMethod.parse(exchange.getRequestMethod()); - var headers = exchange.getRequestHeaders(); - Request request = - new Request( - body, - null, - null, - null, - Map.of(), - exchange.getRequestURI().getRawQuery(), - headers::getFirst, - Map.of(), - method); - Response response = handler.handle(request); - renderer.render(exchange, response); - } -} diff --git a/src/main/java/com/retailsvc/http/internal/NotFoundHandler.java b/src/main/java/com/retailsvc/http/internal/NotFoundHandler.java deleted file mode 100644 index c4a31e51..00000000 --- a/src/main/java/com/retailsvc/http/internal/NotFoundHandler.java +++ /dev/null @@ -1,18 +0,0 @@ -package com.retailsvc.http.internal; - -import static java.net.HttpURLConnection.HTTP_NOT_FOUND; - -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpHandler; -import java.io.IOException; - -/** Returns 404 with no body. Used for the framework's catch-all {@code /} context. */ -public final class NotFoundHandler implements HttpHandler { - - @Override - public void handle(HttpExchange exchange) throws IOException { - try (exchange) { - exchange.sendResponseHeaders(HTTP_NOT_FOUND, -1); - } - } -} diff --git a/src/test/java/com/retailsvc/http/internal/ExtraRouteAdapterTest.java b/src/test/java/com/retailsvc/http/internal/ExtraRouteAdapterTest.java deleted file mode 100644 index 11a0e5eb..00000000 --- a/src/test/java/com/retailsvc/http/internal/ExtraRouteAdapterTest.java +++ /dev/null @@ -1,58 +0,0 @@ -package com.retailsvc.http.internal; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import com.retailsvc.http.GsonTypeMapper; -import com.retailsvc.http.Request; -import com.retailsvc.http.RequestHandler; -import com.retailsvc.http.Response; -import com.retailsvc.http.TypeMapper; -import com.retailsvc.http.spec.HttpMethod; -import com.sun.net.httpserver.Headers; -import com.sun.net.httpserver.HttpExchange; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.net.URI; -import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; -import org.junit.jupiter.api.Test; - -class ExtraRouteAdapterTest { - - @Test - void buildsRequestWithMethodQueryHeadersAndBodyBytesAndNullOperationId() throws Exception { - AtomicReference captured = new AtomicReference<>(); - RequestHandler handler = - req -> { - captured.set(req); - return Response.empty(); - }; - - Map mappers = Map.of("application/json", new GsonTypeMapper()); - ResponseRenderer renderer = new ResponseRenderer(mappers); - ExtraRouteAdapter adapter = new ExtraRouteAdapter("/alive", handler, renderer); - - HttpExchange ex = mock(HttpExchange.class); - Headers reqHeaders = new Headers(); - reqHeaders.add("X-Trace", "abc"); - when(ex.getRequestMethod()).thenReturn("POST"); - when(ex.getRequestURI()).thenReturn(new URI("/alive?x=1")); - when(ex.getRequestHeaders()).thenReturn(reqHeaders); - when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream("hi".getBytes())); - when(ex.getResponseHeaders()).thenReturn(new Headers()); - when(ex.getResponseBody()).thenReturn(new ByteArrayOutputStream()); - - adapter.handle(ex); - - Request r = captured.get(); - assertThat(r.operationId()).isNull(); - assertThat(r.pathParams()).isEmpty(); - assertThat(r.principals()).isEmpty(); - assertThat(r.method()).isEqualTo(HttpMethod.POST); - assertThat(r.rawQuery()).isEqualTo("x=1"); - assertThat(r.header("X-Trace")).contains("abc"); - assertThat(r.bytes()).containsExactly('h', 'i'); - } -} From 5f1cb9e4d6a3705fcc91a047e2b69f61fb7aa46f Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:38:38 +0200 Subject: [PATCH 12/14] test: Add integration tests for extras wildcard matching --- .../com/retailsvc/http/ExtrasWildcardIT.java | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 src/test/java/com/retailsvc/http/ExtrasWildcardIT.java diff --git a/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java new file mode 100644 index 00000000..9db6b149 --- /dev/null +++ b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java @@ -0,0 +1,146 @@ +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class ExtrasWildcardIT extends ServerBaseTest { + + @Test + void singleStarMatchesOneSegment() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/static/*", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/static/x.css").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/static/a/b").statusCode()).isEqualTo(HTTP_NOT_FOUND); + assertThat(get(client, s, "/static/").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void doubleStarMatchesAnyDepth() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/files/**", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/files/a").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/files/a/b/c").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/filesx/a").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void midPathDoubleStar() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/schemas/**/openapi.yaml", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/schemas/a/openapi.yaml").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/schemas/a/b/openapi.yaml").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/schemas/openapi.yaml").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void exactExtraStillWorks() throws Exception { + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/alive", Handlers.aliveHandler()) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/alive").statusCode()).isEqualTo(204); + assertThat(get(client, s, "/alive232").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + @Test + void traversalReturns400() throws Exception { + RequestHandler ok = req -> Response.of(HTTP_OK, "ok"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/files/**", ok) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/files/../etc/passwd").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/%2e%2e/etc/passwd").statusCode()) + .isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/%252e%252e/etc/passwd").statusCode()) + .isEqualTo(HTTP_BAD_REQUEST); + // %2f is a percent-encoded slash — reject encoded path separators + assertThat(get(client, s, "/files/%2fetc").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + // %5c is a backslash — reject encoded backslashes + assertThat(get(client, s, "/files/x%5cy").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + // %00 is a null byte — java.net.URI rejects raw NUL in the path; defense at the + // router is still valid for clients that bypass URI parsing, but we cannot express + // this vector via java.net.http.HttpClient (URI.create throws before the wire). + // assertThat(get(client, s, "/files/x%00").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + // %0a is a line-feed — same reason as %00: JDK URI rejects it pre-wire. + // assertThat(get(client, s, "/files/x%0ay").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files//x").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/.").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + } + } + + @Test + void basePathSpecRouteWinsOverExtras() throws Exception { + RequestHandler greedy = req -> Response.of(HTTP_OK, "should not see this"); + try (var s = + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of())) + .port(0) + .extraRoute("/**", greedy) + .build(); + var client = httpClient()) { + + assertThat(get(client, s, "/api/v1/data").statusCode()).isEqualTo(HTTP_OK); + assertThat(get(client, s, "/totally-not-a-spec-route").statusCode()).isEqualTo(HTTP_OK); + } + } + + private HttpResponse get(HttpClient client, OpenApiServer s, String path) + throws Exception { + var req = + HttpRequest.newBuilder() + .uri(URI.create("http://localhost:" + s.listenPort() + path)) + .GET() + .build(); + return client.send(req, BodyHandlers.ofString()); + } +} From a8fd01b87d4916544b3994992f2585f8c3bf29ab Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:41:55 +0200 Subject: [PATCH 13/14] test: Use HTTP_NO_CONTENT and add single-dot traversal vector to ExtrasWildcardIT --- src/test/java/com/retailsvc/http/ExtrasWildcardIT.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java index 9db6b149..0dc1cbe9 100644 --- a/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java +++ b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java @@ -2,6 +2,7 @@ import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_NO_CONTENT; import static java.net.HttpURLConnection.HTTP_OK; import static org.assertj.core.api.Assertions.assertThat; @@ -80,7 +81,7 @@ void exactExtraStillWorks() throws Exception { .build(); var client = httpClient()) { - assertThat(get(client, s, "/alive").statusCode()).isEqualTo(204); + assertThat(get(client, s, "/alive").statusCode()).isEqualTo(HTTP_NO_CONTENT); assertThat(get(client, s, "/alive232").statusCode()).isEqualTo(HTTP_NOT_FOUND); } } @@ -114,6 +115,7 @@ void traversalReturns400() throws Exception { // assertThat(get(client, s, "/files/x%0ay").statusCode()).isEqualTo(HTTP_BAD_REQUEST); assertThat(get(client, s, "/files//x").statusCode()).isEqualTo(HTTP_BAD_REQUEST); assertThat(get(client, s, "/files/.").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(get(client, s, "/files/./x").statusCode()).isEqualTo(HTTP_BAD_REQUEST); } } From ff33d157cf60b4089fb38c3825bb09bfc43c4e5f Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 22 May 2026 14:51:34 +0200 Subject: [PATCH 14/14] refactor: Reduce cognitive complexity in path validation; drop dead IT comments --- .../http/internal/ExtrasPathValidator.java | 36 ++++++------ .../retailsvc/http/internal/PathPattern.java | 56 +++++++++---------- .../com/retailsvc/http/ExtrasWildcardIT.java | 8 +-- 3 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java index adb90030..ac370122 100644 --- a/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java +++ b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java @@ -13,6 +13,18 @@ private ExtrasPathValidator() {} public static String validateAndDecode(URI uri) { String raw = uri.getRawPath(); + validateRaw(raw); + + String decoded = uri.getPath(); + if (decoded == null) { + throw new BadRequestException("missing path"); + } + rejectControlChars(decoded, "decoded path contains control character"); + validateSegments(decoded); + return decoded; + } + + private static void validateRaw(String raw) { if (raw == null) { throw new BadRequestException("missing path"); } @@ -22,25 +34,19 @@ public static String validateAndDecode(URI uri) { if (raw.indexOf('\\') >= 0) { throw new BadRequestException("path contains backslash"); } - for (int i = 0; i < raw.length(); i++) { - char c = raw.charAt(i); - if (c < 0x20 || c == 0x7f) { - throw new BadRequestException("path contains control character"); - } - } - - String decoded = uri.getPath(); - if (decoded == null) { - throw new BadRequestException("missing path"); - } + rejectControlChars(raw, "path contains control character"); + } - for (int i = 0; i < decoded.length(); i++) { - char c = decoded.charAt(i); + private static void rejectControlChars(String s, String message) { + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); if (c < 0x20 || c == 0x7f) { - throw new BadRequestException("decoded path contains control character"); + throw new BadRequestException(message); } } + } + private static void validateSegments(String decoded) { String[] segments = decoded.substring(decoded.startsWith("/") ? 1 : 0).split("/", -1); for (int i = 0; i < segments.length; i++) { String s = segments[i]; @@ -51,7 +57,5 @@ public static String validateAndDecode(URI uri) { throw new BadRequestException("path traversal segment"); } } - - return decoded; } } diff --git a/src/main/java/com/retailsvc/http/internal/PathPattern.java b/src/main/java/com/retailsvc/http/internal/PathPattern.java index 1f8eeae4..d7ab40a7 100644 --- a/src/main/java/com/retailsvc/http/internal/PathPattern.java +++ b/src/main/java/com/retailsvc/http/internal/PathPattern.java @@ -24,41 +24,41 @@ public static PathPattern compile(String raw) { String prev = null; for (int i = 0; i < segments.length; i++) { String seg = segments[i]; - if (seg.isEmpty() - && !(i == segments.length - 1 && segments.length > 1 && raw.endsWith("/"))) { - throw new IllegalArgumentException("empty segment in path: " + raw); - } - if (seg.contains("*") && !seg.equals("*") && !seg.equals("**")) { - throw new IllegalArgumentException( - "'*' and '**' must be a whole segment, not " + seg + " in " + raw); - } - if ("**".equals(seg) && "**".equals(prev)) { - throw new IllegalArgumentException("adjacent '**' segments in " + raw); - } + validateSegment(seg, prev, i, segments.length, raw); boolean trailing = i == segments.length - 1; - switch (seg) { - case "*" -> { - rx.append("/[^/]+"); - hasWildcard = true; - } - case "**" -> { - if (trailing) { - // Slash is required; anything (including empty string) may follow it. - rx.append("/.*"); - } else { - // At least one character and a slash must appear before the next segment. - rx.append("/.+"); - } - hasWildcard = true; - } - default -> rx.append("/").append(Pattern.quote(seg)); - } + hasWildcard |= appendSegment(rx, seg, trailing); prev = seg; } rx.append("$"); return new PathPattern(raw, Pattern.compile(rx.toString()), hasWildcard); } + private static void validateSegment(String seg, String prev, int i, int total, String raw) { + boolean trailingEmptyAllowed = i == total - 1 && total > 1 && raw.endsWith("/"); + if (seg.isEmpty() && !trailingEmptyAllowed) { + throw new IllegalArgumentException("empty segment in path: " + raw); + } + if (seg.contains("*") && !seg.equals("*") && !seg.equals("**")) { + throw new IllegalArgumentException( + "'*' and '**' must be a whole segment, not " + seg + " in " + raw); + } + if ("**".equals(seg) && "**".equals(prev)) { + throw new IllegalArgumentException("adjacent '**' segments in " + raw); + } + } + + private static boolean appendSegment(StringBuilder rx, String seg, boolean trailing) { + switch (seg) { + case "*" -> rx.append("/[^/]+"); + case "**" -> rx.append(trailing ? "/.*" : "/.+"); + default -> { + rx.append("/").append(Pattern.quote(seg)); + return false; + } + } + return true; + } + public boolean hasWildcard() { return wildcard; } diff --git a/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java index 0dc1cbe9..74b19e3e 100644 --- a/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java +++ b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java @@ -107,12 +107,8 @@ void traversalReturns400() throws Exception { assertThat(get(client, s, "/files/%2fetc").statusCode()).isEqualTo(HTTP_BAD_REQUEST); // %5c is a backslash — reject encoded backslashes assertThat(get(client, s, "/files/x%5cy").statusCode()).isEqualTo(HTTP_BAD_REQUEST); - // %00 is a null byte — java.net.URI rejects raw NUL in the path; defense at the - // router is still valid for clients that bypass URI parsing, but we cannot express - // this vector via java.net.http.HttpClient (URI.create throws before the wire). - // assertThat(get(client, s, "/files/x%00").statusCode()).isEqualTo(HTTP_BAD_REQUEST); - // %0a is a line-feed — same reason as %00: JDK URI rejects it pre-wire. - // assertThat(get(client, s, "/files/x%0ay").statusCode()).isEqualTo(HTTP_BAD_REQUEST); + // %00 (NUL) and %0a (LF) cannot be tested here: java.net.URI rejects them before + // they reach the wire. Router-level defence is exercised by ExtrasPathValidatorTest. assertThat(get(client, s, "/files//x").statusCode()).isEqualTo(HTTP_BAD_REQUEST); assertThat(get(client, s, "/files/.").statusCode()).isEqualTo(HTTP_BAD_REQUEST); assertThat(get(client, s, "/files/./x").statusCode()).isEqualTo(HTTP_BAD_REQUEST);