refactor: Reduce cognitive complexity in path validation; drop dead IT comments

Author: thced

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;
   }
 }

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;
   }

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);