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 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). 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..1287c7ce --- /dev/null +++ b/docs/superpowers/specs/2026-05-22-extras-wildcard-design.md @@ -0,0 +1,205 @@ +# 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 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). + +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. + +## Path-traversal protection + +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. + +### Raw URI rules (`HttpExchange.getRequestURI().getRawPath()`) + +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 mentioned for clarity but not implemented. + +## Error handling + +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 + +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 + - 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 + +- 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). 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/ExtrasPathValidator.java b/src/main/java/com/retailsvc/http/internal/ExtrasPathValidator.java new file mode 100644 index 00000000..ac370122 --- /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.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(); + 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"); + } + 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"); + } + rejectControlChars(raw, "path contains control character"); + } + + 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(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]; + if (s.isEmpty() && i != segments.length - 1) { + throw new BadRequestException("empty path segment"); + } + if (".".equals(s) || "..".equals(s)) { + throw new BadRequestException("path traversal segment"); + } + } + } +} 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/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/main/java/com/retailsvc/http/internal/PathPattern.java b/src/main/java/com/retailsvc/http/internal/PathPattern.java new file mode 100644 index 00000000..d7ab40a7 --- /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]; + validateSegment(seg, prev, i, segments.length, raw); + boolean trailing = i == segments.length - 1; + 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; + } + + public boolean matches(String path) { + return regex.matcher(path).matches(); + } + + public String raw() { + return raw; + } +} 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..74b19e3e --- /dev/null +++ b/src/test/java/com/retailsvc/http/ExtrasWildcardIT.java @@ -0,0 +1,144 @@ +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_NO_CONTENT; +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(HTTP_NO_CONTENT); + 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 (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); + } + } + + @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()); + } +} 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'); - } -} 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..de3023b5 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ExtrasPathValidatorTest.java @@ -0,0 +1,77 @@ +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 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); + } +} 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); + } +} 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 '/'"); + } +}