Add Filter for measuring request response time

[Cavve]

Implemented a filter that measures how long each incoming HTTP request takes to process. The filter should record the start time before delegating to the next filter in the chain, then calculate the elapsed time once the response is ready. It should log the total duration in milliseconds together with the request method and path.

Summary by CodeRabbit

• New Features

  • Full Java HTTP server: request parsing, response writing, plugin system, filters (global/route), static file serving, IP access control, YAML configuration, Docker packaging.

• CI / Chores

  • Added CI and image-publish workflows, Maven wrapper, container build support, formatting config, and packaging updates.

• Documentation

  • Expanded README, ADRs, and usage notes with architecture and run-with-Docker guidance.

• Tests

  • Extensive unit and integration tests for parser, routing, filters, handlers, and utilities.

[coderabbitai]

Warning

Rate limit exceeded

@Cavve has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 32 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Adds a full Java HTTP server implementation with request parsing/writing, filter-based pipeline, router/plugin dispatch, static file serving, YAML config, logging, build/CI and Docker workflows, frontend static assets, extensive documentation (ADRs), and a comprehensive test suite.

Changes

Cohort / File(s)	Summary
Build & Tooling pom.xml, .editorconfig, .mvn/wrapper/maven-wrapper.properties, mvnw, mvnw.cmd	Project coordinates and Java level bumped (to org.juv25d / Java 25). Adds Maven Wrapper, editor config, Spotless, PIT, Shade plugin, and wrapper scripts.
CI & Release .github/workflows/ci.yml, .github/workflows/docker-release.yml, Dockerfile, .gitignore	Adds CI workflow (tests, Spotless), Docker multi-arch release workflow, multi-stage Dockerfile, and ignores META-INF.
Application Entrypoint src/main/java/org/juv25d/App.java, src/main/java/org/example/App.java	New main class org.juv25d.App added; old org.example.App removed. App wires config, logging, pipeline, router, and starts Server.
Server & Connection Handling src/main/java/org/juv25d/Server.java, src/main/java/org/juv25d/ConnectionHandler.java, src/main/java/org/juv25d/ConnectionHandlerFactory.java, src/main/java/org/juv25d/DefaultConnectionHandlerFactory.java	Introduces Server listening loop, virtual-thread per connection, ConnectionHandler parsing requests and invoking pipeline, and factory to create handlers.
HTTP Core src/main/java/org/juv25d/http/HttpParser.java, src/main/java/org/juv25d/http/HttpRequest.java, src/main/java/org/juv25d/http/HttpResponse.java, src/main/java/org/juv25d/http/HttpResponseWriter.java	Adds request parser, immutable HttpRequest record, mutable HttpResponse, and response writer that serializes HTTP/1.1 with correct headers and Content-Length handling.
Pipeline & Filters src/main/java/org/juv25d/Pipeline.java, src/main/java/org/juv25d/filter/..., src/main/java/org/juv25d/filter/annotation/..., src/main/java/org/juv25d/FilterRegistration.java	Implements filter pipeline with global and route-specific filters, ordering via FilterRegistration, FilterChainImpl, and lifecycle hooks plus annotations for Global/Route.
Filter Implementations src/main/java/org/juv25d/filter/IpFilter.java, .../LoggingFilter.java, .../TimingFilter.java	Adds IP whitelist/blacklist filter (403 handling), simple logging filter, and timing filter (elapsed logging).
Router & Plugins src/main/java/org/juv25d/router/SimpleRouter.java, src/main/java/org/juv25d/router/Router.java, src/main/java/org/juv25d/plugin/*	Adds Router interface and SimpleRouter (exact + wildcard matching), Plugin interface, StaticFilesPlugin, NotFoundPlugin, and HelloPlugin.
Static File Serving & MIME src/main/java/org/juv25d/handler/StaticFileHandler.java, src/main/java/org/juv25d/handler/MimeTypeResolver.java, src/main/resources/static/*	Adds classpath-based static file handler with path traversal prevention, MIME resolver, and comprehensive static frontend assets (HTML/CSS/JS/README).
Config & Logging Utilities src/main/java/org/juv25d/util/ConfigLoader.java, src/main/java/org/juv25d/logging/ServerLogging.java, src/main/resources/application-properties.yml	YAML config loader (server.port, root dir, log level) and centralized logger setup using system/env/app-config precedence.
Documentation & ADRs README.md, docs/adr/ADR-001-static-file-serving-architecture.md, docs/adr/README.md, docs/adr/TEMPLATE.md, docs/notes/pipeline-usage.md	Replaces README with project guide and adds ADRs, ADR template, and notes describing architecture and pipeline usage.
Tests src/test/java/org/juv25d/**/*.java	Adds many tests covering HttpParser, HttpResponseWriter, Pipeline/filter behavior, Router resolution, StaticFileHandler security/content, MimeTypeResolver, logging, and integration-level tests.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant ConnHandler as ConnectionHandler
    participant Parser as HttpParser
    participant Pipeline
    participant Filters as FilterChain
    participant Router
    participant Plugin

    Client->>Server: TCP connect + send HTTP request
    Server->>ConnHandler: create & start handler (virtual thread)
    ConnHandler->>Parser: parse(InputStream)
    Parser-->>ConnHandler: HttpRequest
    ConnHandler->>Pipeline: createChain(HttpRequest)
    Pipeline-->>ConnHandler: FilterChainImpl
    ConnHandler->>Filters: doFilter(req, res)

    rect rgba(100, 150, 255, 0.5)
        Note over Filters: Execute ordered filters (global + route-specific)
        Filters->>Filters: filter[0].doFilter(...)
        Filters->>Filters: filter[1].doFilter(...)
    end

    Filters->>Router: resolve(HttpRequest)
    Router-->>Filters: Plugin
    Filters->>Plugin: handle(req, res)
    Plugin-->>Filters: (response set/modified)
    Filters-->>ConnHandler: chain complete
    ConnHandler->>ConnHandler: HttpResponseWriter.write(out, response)
    ConnHandler-->>Client: HTTP/1.1 response bytes

Loading

sequenceDiagram
    participant Pipeline
    participant Global as GlobalFilters
    participant Route as RouteFilters
    participant Router

    Pipeline->>Global: collect ordered global filters
    Pipeline->>Route: collect matching route filters (exact + wildcard)
    Note over Global,Route: Merge and sort by order
    Pipeline->>Router: create FilterChainImpl(filters, router)
    Router-->>Pipeline: FilterChainImpl returned
    Note over Pipeline: Execution will run filters in sorted order then Router.resolve -> Plugin.handle

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hopped through files with tiny paws,
A server built without a pause.
Filters, plugins, routes in line,
Static pages served — how fine!
A carrot-shaped commit, crisp and bright!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.74% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: adding a TimingFilter that measures request-response time. It directly aligns with the primary objective and the TimingFilter implementation present in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 15

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (13)

.mvn/wrapper/maven-wrapper.properties-3-3 (1)

3-3: ⚠️ Potential issue | 🟡 Minor

Maven version in wrapper (3.8.7) drifts from the Docker build image (3.9.x).

Local ./mvnw runs will download and use Maven 3.8.7, while the Dockerfile build stage pulls maven:3.9-eclipse-temurin-25. This can cause subtle build differences (plugin behaviour, dependency resolution) between local and CI/Docker builds. Align both to the same version, preferably the already-used 3.9.x line.

🔧 Proposed fix

-distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.7/apache-maven-3.8.7-bin.zip
+distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.9/apache-maven-3.9.9-bin.zip

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.mvn/wrapper/maven-wrapper.properties at line 3, Update the Maven wrapper
version to match the Docker build image by editing the distributionUrl property
in .mvn/wrapper/maven-wrapper.properties (the distributionUrl entry currently
points to apache-maven/3.8.7); change it to the 3.9.x artifact that matches the
Dockerfile's maven:3.9-eclipse-temurin-25 image so local ./mvnw uses the same
Maven release as CI/Docker.

src/main/resources/static/readme.html-18-18 (1)

18-18: ⚠️ Potential issue | 🟡 Minor

Missing semicolons on HTML numeric character references.

&#128269 and &#128270 should be 🔍 and 🔎 respectively. Most browsers recover, but omitting the semicolon is invalid HTML and can cause parsing ambiguity when followed by certain characters.

Proposed fix

-        <h1>&#128269 ReadMe &#128270</h1>
+        <h1>&#128269; ReadMe &#128270;</h1>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/readme.html` at line 18, The HTML uses numeric
character references without trailing semicolons in the <h1> element (&#128269
and &#128270); update those references to include semicolons (&#128269; and
&#128270;) so the browser parses them as valid character references. Locate the
<h1> line in readme.html and add the missing semicolons after each numeric
entity to fix the HTML validity.

src/main/resources/static/index.html-46-48 (1)

46-48: ⚠️ Potential issue | 🟡 Minor

Inconsistent script path: js/purify.min.js is missing the leading /.

Lines 46 and 48 use absolute paths (/js/marked.min.js, /js/app.js), but line 47 uses a relative path (js/purify.min.js). This will break if the page is ever served at a nested URL path. Keep paths consistent.

Proposed fix

 <script src="/js/marked.min.js"></script>
-<script src="js/purify.min.js"></script>
+<script src="/js/purify.min.js"></script>
 <script src="/js/app.js"></script>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/index.html` around lines 46 - 48, Update the
inconsistent script src for purify by changing the relative path
"js/purify.min.js" to the absolute path "/js/purify.min.js" so it matches the
other script tags ("/js/marked.min.js" and "/js/app.js"); locate the <script>
tag referencing purify.min.js in index.html and update its src to include the
leading slash to avoid breakage when served from nested URLs.

src/main/resources/static/readme.html-36-38 (1)

36-38: ⚠️ Potential issue | 🟡 Minor

Same inconsistent script path as index.html.

Line 37 uses relative js/purify.min.js while lines 36 and 38 use absolute paths. Apply the same fix here.

Proposed fix

 <script src="/js/marked.min.js"></script>
-<script src="js/purify.min.js"></script>
+<script src="/js/purify.min.js"></script>
 <script src="/js/app.js"></script>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/readme.html` around lines 36 - 38, The script tag
for purify is using a relative path while the others use absolute paths; update
the src for purify.min.js to use the absolute path "/js/purify.min.js" so it
matches the other script includes (refer to the script elements loading
marked.min.js, purify.min.js, and app.js in the readme.html snippet).

src/main/resources/static/js/app.js-72-81 (1)

72-81: ⚠️ Potential issue | 🟡 Minor

Click handler intercepts external .html links.

Any <a> whose href ends in .html—including absolute cross-origin URLs—will be captured and fetched via navigate(), which will fail or produce confusing behaviour. Add an origin or relative-path guard.

Proposed fix

 document.addEventListener("click", (e) => {
     const link = e.target.closest("a");
     if (!link) return;
 
     const href = link.getAttribute("href");
     if (!href || !href.endsWith(".html")) return;
+    if (href.startsWith("http") || href.startsWith("//")) return;
 
     e.preventDefault();
     navigate(href);
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/js/app.js` around lines 72 - 81, The click handler
currently captures any <a> with href ending in ".html" including cross-origin
links; update the handler in the event listener that uses link, href, and
navigate to first resolve the href into a URL (e.g. new URL(href,
window.location.href)) and bail out if url.origin !== window.location.origin
(thus only handling same-origin or relative links); keep the existing check for
href ending with ".html", and only call e.preventDefault() and navigate(href)
after the same-origin check so external absolute URLs are not intercepted.

src/test/java/org/juv25d/logging/ServerLoggingTest.java-123-141 (1)

123-141: ⚠️ Potential issue | 🟡 Minor

Test is fragile when LOG_LEVEL environment variable is set.

ServerLogging.configure() checks System.getenv("LOG_LEVEL") before falling back to ConfigLoader. This test asserts the level equals ConfigLoader's value, but if LOG_LEVEL is set in the environment (e.g., CI runner, Docker), the actual level will differ from the expected value and the test will fail.

Since environment variables can't be cleared portably in Java without third-party libraries, consider documenting this assumption or using a dedicated test logger that bypasses the env lookup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/logging/ServerLoggingTest.java` around lines 123 -
141, The test fails if LOG_LEVEL is set in the environment because
ServerLogging.configure(Logger) reads System.getenv("LOG_LEVEL"); add a
test-friendly overload ServerLogging.configure(Logger, boolean ignoreEnv) (or a
configureUsingConfigLoader(Logger) helper) that skips the getenv lookup and
forces use of ConfigLoader.getInstance().getLogLevel(), then update the test
method logger_shouldUseLogLevelFromApplicationProperties to call the new
overload with ignoreEnv=true so the assertion against
ConfigLoader.getInstance().getLogLevel() is deterministic.

src/main/java/org/juv25d/logging/ServerLogging.java-21-25 (1)

21-25: ⚠️ Potential issue | 🟡 Minor

ConsoleHandler level is not set — sub-INFO levels will be silently dropped.

ConsoleHandler defaults to Level.INFO. If the logger level is set to FINE, FINER, or FINEST, the handler will still filter those messages out. Set the handler level to match (or use Level.ALL) so the logger level is the sole gate.

Proposed fix

 if (logger.getHandlers().length == 0) {
     ConsoleHandler handler = new ConsoleHandler();
     handler.setFormatter(new SimpleFormatter());
+    handler.setLevel(Level.ALL);
     logger.addHandler(handler);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/logging/ServerLogging.java` around lines 21 - 25,
The ConsoleHandler created in the ServerLogging initialization is left at its
default Level.INFO which will drop any log records below INFO even if the Logger
(logger) is set to a finer level; update the handler's level (e.g., call
handler.setLevel(logger.getLevel()) or handler.setLevel(Level.ALL)) after
creating the ConsoleHandler so the handler does not filter out sub-INFO messages
and the logger level becomes the effective gate for log records.

.github/workflows/docker-release.yml-37-37 (1)

37-37: ⚠️ Potential issue | 🟡 Minor

org.opencontainers.image.source must be a URL, not a repository path.

${{ github.repository }} expands to owner/repo, which is not a valid URL. The OCI image spec defines org.opencontainers.image.source as a URL pointing to the source repository.

🔧 Proposed fix

-          labels: org.opencontainers.image.source=${{ github.repository }}
+          labels: org.opencontainers.image.source=https://github.com/${{ github.repository }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/docker-release.yml at line 37, The label
org.opencontainers.image.source currently uses ${{ github.repository }} which
yields "owner/repo" (not a URL); update that label to provide a valid repository
URL instead (for example use ${{ github.repository_url }} or construct one via
${{ github.server_url }}/${{ github.repository }}) so the OCI metadata contains
a proper URL.

src/main/java/org/juv25d/plugin/NotFoundPlugin.java-10-14 (1)

10-14: ⚠️ Potential issue | 🟡 Minor

Specify charset in getBytes() and set a Content-Type header.

Two issues:

1. getBytes() uses the platform-default charset. On non-UTF-8 JVM environments the response body will be mis-encoded. Use StandardCharsets.UTF_8 explicitly.
2. No Content-Type header is set, so HTTP clients cannot reliably interpret the response body.

🛡️ Proposed fix

+import java.nio.charset.StandardCharsets;
 
 public class NotFoundPlugin implements Plugin {
     `@Override`
     public void handle(HttpRequest req, HttpResponse res) throws IOException {
         res.setStatusCode(404);
         res.setStatusText("Not Found");
-        res.setBody("404 - Resource Not Found".getBytes());
+        res.setHeader("Content-Type", "text/plain; charset=utf-8");
+        res.setBody("404 - Resource Not Found".getBytes(StandardCharsets.UTF_8));
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/plugin/NotFoundPlugin.java` around lines 10 - 14, In
NotFoundPlugin.handle, avoid platform-default encoding and missing content type:
change the body bytes call to use StandardCharsets.UTF_8 (e.g., "404 - Resource
Not Found".getBytes(StandardCharsets.UTF_8)) and add a Content-Type header on
the HttpResponse (e.g., res.setHeader("Content-Type", "text/plain;
charset=UTF-8")) so clients correctly decode the response; update the
res.setBody and add res.setHeader calls accordingly.

src/main/java/org/juv25d/handler/StaticFileHandler.java-44-46 (1)

44-46: ⚠️ Potential issue | 🟡 Minor

405 response is missing the required Allow header

Per RFC 7231 §6.5.5, a 405 Method Not Allowed response MUST include an Allow header field listing the supported methods.

♻️ Proposed fix

     if (!request.method().equalsIgnoreCase("GET")) {
-        return createErrorResponse(405, "Method Not Allowed");
+        HttpResponse resp = createErrorResponse(405, "Method Not Allowed");
+        resp.setHeader("Allow", "GET");
+        return resp;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/handler/StaticFileHandler.java` around lines 44 -
46, The 405 branch in StaticFileHandler returns a response without the required
Allow header; update the code that handles non-GET requests to include an Allow:
GET header in the response (either by calling a variant of createErrorResponse
that accepts headers or by adding the header to the Response object before
returning). Specifically modify the if
(!request.method().equalsIgnoreCase("GET")) branch in StaticFileHandler to build
the 405 response and set the "Allow" header to "GET" (referencing the
createErrorResponse helper or the Response builder you use) so the returned 405
includes the required Allow field.

src/main/java/org/juv25d/filter/IpFilter.java-44-53 (1)

44-53: ⚠️ Potential issue | 🟡 Minor

Avoid reflecting the client IP in the response body.

Including the IP in the 403 body ("403 Forbidden: IP not allowed (" + ip + ")") can leak internal network details or proxy-forwarded addresses to the client. A generic "403 Forbidden" message is safer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/filter/IpFilter.java` around lines 44 - 53, The
forbidden(HttpResponse res, String ip) method in IpFilter currently reflects the
client IP in the response body; change the body to a generic message (e.g., "403
Forbidden") and recompute the byte[] and Content-Length accordingly so no
client-controlled IP is echoed. Keep the same status code/text and headers but
remove the ip variable from the response string and any logging that writes it
to the response body.

docs/adr/ADR-001-static-file-serving-architecture.md-33-33 (1)

33-33: ⚠️ Potential issue | 🟡 Minor

Stale class name references: SocketServer → Server.

The ADR references SocketServer (lines 33, 198, 203), but the actual class in the codebase is Server (src/main/java/org/juv25d/Server.java). These references should be updated to match the implementation.

Also applies to: 198-203

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/ADR-001-static-file-serving-architecture.md` at line 33, Update ADR
references that mention the nonexistent class SocketServer to use the actual
implementation class name Server; search for occurrences of "SocketServer" in
the ADR (notably around the noted lines) and replace them with "Server" so the
document matches the codebase's class name (Server) and avoids stale references.

docs/adr/ADR-001-static-file-serving-architecture.md-238-238 (1)

238-238: ⚠️ Potential issue | 🟡 Minor

Broken placeholder link.

The issue link uses a placeholder URL (https://github.com/your-repo/issues/18) that should be replaced with the actual repository URL.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/ADR-001-static-file-serving-architecture.md` at line 238, Replace
the broken placeholder URL in the markdown link for "[Issue `#18`: GET handling
for static files]" with the real repository issue URL (e.g.
https://github.com/<org-or-user>/<repo>/issues/18) so the link resolves
correctly; locate the link text "[Issue `#18`: GET handling for static files]" in
ADR-001-static-file-serving-architecture.md and update only the href portion to
the actual repo/issues path.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Container runs as root — add a non-root USER.

Java 25 is the latest LTS release, so the base image choice is solid. However, neither the build nor the runtime stage declares a USER, so all processes (including the JVM serving requests) run as root inside the container. This violates the principle of least privilege and is flagged by Trivy (DS-0002).

🔒 Proposed fix

 FROM eclipse-temurin:25-jre-alpine

 WORKDIR /app

 COPY --from=build /app/target/*.jar app.jar
+
+RUN addgroup -S appgroup && adduser -S appuser -G appgroup
+USER appuser
+
 ENTRYPOINT ["java", "-jar", "app.jar"]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 11 - 17, The container runs as root; create and
switch to a non-root user in the Dockerfile runtime stage: add commands in the
final stage (after WORKDIR and COPY --from=build ...) to create a dedicated
user/group (e.g., appuser), chown the application files (app.jar and /app) to
that user, and use USER appuser before the ENTRYPOINT so the JVM does not run as
root; ensure any JVM-required permissions are granted to that user and keep
ENTRYPOINT ["java","-jar","app.jar"] unchanged.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

JUnit Jupiter 6.0.2 release Maven Central

💡 Result:

JUnit Jupiter 6.0.2 is published on Maven Central (released January 6, 2026). (docs.junit.org)

Maven coordinates (aggregator):

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter</artifactId>
  <version>6.0.2</version>
  <scope>test</scope>
</dependency>

The 6.0.2 version is also available for core modules like junit-jupiter-api and junit-jupiter-engine on Maven Central. (repo.maven.apache.org)

Citations:

• 1: https://docs.junit.org/snapshot/release-notes/?utm_source=openai
• 2: https://repo.maven.apache.org/maven2/org/junit/jupiter/junit-jupiter-api/?utm_source=openai

---

🏁 Script executed:

# Find and read the full pom.xml to understand the project context
find . -name "pom.xml" -type f | head -5

Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template

Length of output: 117

---

🏁 Script executed:

cat -n ./pom.xml

Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template

Length of output: 8751

---

🌐 Web query:

pitest-junit5-plugin 1.2.2 JUnit 6 compatibility

💡 Result:

org.pitest:pitest-junit5-plugin:1.2.2 is not JUnit 6–compatible in any officially supported/guaranteed sense.

• The plugin docs state that 1.2.2 is built against JUnit Platform 1.x and “you may encounter issues if you use it with a different version”, and that it “automatically work[s] with JUnit platform 1.5.0 to 1.10.0-M1 (and probably above)” when used via the PIT Maven/Gradle integrations. That range is JUnit 5-era Platform, not JUnit 6. [1]
• JUnit 6.0.0 (released Sep 30, 2025) moves to a single unified 6.x version line (Platform/Jupiter/Vintage all 6.0.0) and includes breaking changes/new launcher APIs, i.e., it’s a major-line shift from Platform 1.x to Platform 6.x. [2][3]

What to do: if you’re upgrading to JUnit 6, you’ll need a pitest-junit5-plugin version that explicitly states support for JUnit Platform 6.x (the plugin README currently does not for 1.2.2, and even its “automatic” range is still expressed in Platform 1.x terms). [1]

Sources
[1] pitest/pitest-junit5-plugin README (versions + supported JUnit Platform range) (github.com)
[2] JUnit 6.0.0 release notes (release date + unified versioning + platform changes) (docs.junit.org)
[3] JUnit 6.0.0 ships Sep 30, 2025 (background/summary) (infoq.com)

Citations:

• 1: https://github.com/pitest/pitest-junit5-plugin?utm_source=openai
• 2: https://docs.junit.org/6.0.1/release-notes/index.html?utm_source=openai
• 3: https://www.infoq.com/news/2025/10/junit6-java17-kotlin/?utm_source=openai

---

🌐 Web query:

pitest 1.22.0 JUnit 6 support

💡 Result:

PIT 1.22.0 does not have “JUnit 6” support.

• PIT core supports JUnit 4 out of the box; JUnit 5 requires the separate pitest-junit5-plugin. [1]
• The pitest-junit5-plugin is built around JUnit Platform 1.x (it notes compatibility with roughly 1.5.0 → 1.10.0-M1). That does not cover the JUnit 6 / Platform 6.x line. [2]
• JUnit 6.0.0 was released on September 30, 2025. [3]
• PIT 1.22.0 (released Nov 18, 2025) only mentions a test-filter extension point—nothing about JUnit 6/Platform 6. [4][5]

What to do: if you need PIT today, keep tests on JUnit 5 (Platform 1.x) (e.g., JUnit 5.14.x) and use pitest-junit5-plugin; otherwise you’ll need to wait for (or contribute) an updated PIT JUnit plugin that explicitly supports JUnit Platform 6.x. [2][3]

Sources:
[1] PIT FAQ (JUnit support)
[2] pitest-junit5-plugin README (supported JUnit Platform range)
[3] JUnit 6.0.0 release notes (release date / version line)
[4] PIT GitHub releases (1.22.0 notes)
[5] Maven Central listing showing PIT 1.22.0 publish date

---

Downgrade JUnit Jupiter to 5.x or remove pitest dependency — pitest-junit5-plugin 1.2.2 does not support JUnit 6.

JUnit Jupiter 6.0.2 (released January 6, 2026) and Java 25 (September 2025) are both valid and available. However, pitest-junit5-plugin 1.2.2 is built against JUnit Platform 1.x (JUnit 5 era) and does not support JUnit Platform 6.x introduced in JUnit 6.0.0. This incompatibility will prevent pitest mutations from running. Either downgrade to JUnit 5.14.x or wait for an updated pitest plugin with JUnit 6 support.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pom.xml` around lines 12 - 14, The project currently sets
junit.jupiter.version to 6.0.2 which is incompatible with pitest-junit5-plugin
1.2.2; update the pom so the junit.jupiter.version Maven property is downgraded
to a JUnit 5 release (e.g., 5.14.x) or remove/replace the pitest-junit5-plugin
dependency so mutation testing is not forced to run against JUnit 6—look for the
junit.jupiter.version property and any pitest-junit5-plugin configuration in the
POM and change the property to a 5.x value (or remove/upgrade the plugin) to
restore compatibility with pitest.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

No socket read timeout — connections can block indefinitely

httpParser.parse(in) reads the socket's InputStream byte-by-byte with no deadline. A client that opens a connection and then stops sending data (slow-loris style, or simply abandons the connection) will hold a virtual thread and a file descriptor open forever.

Add a read timeout via socket.setSoTimeout(millis) before delegating to the parser:

🛡️ Proposed fix

     `@Override`
     public void run() {
         try (socket) {
+            socket.setSoTimeout(30_000); // 30-second read deadline
             var in = socket.getInputStream();
             var out = socket.getOutputStream();

The timeout value should ideally be read from ConfigLoader (e.g., server.read-timeout-ms) to keep it configurable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/ConnectionHandler.java` around lines 28 - 32, The
ConnectionHandler currently calls httpParser.parse(in) without configuring a
socket read timeout, allowing connections to hang indefinitely; fix this by
retrieving a configured timeout (e.g.,
ConfigLoader.getInt("server.read-timeout-ms") or equivalent) and calling
socket.setSoTimeout(timeoutMillis) before invoking httpParser.parse(in) so the
socket's read operations will time out, then handle SocketTimeoutException
appropriately (close/return a 408 or log and close) around the call to
httpParser.parse; reference the ConnectionHandler class, the socket variable,
and httpParser.parse to locate where to add the setSoTimeout and the timeout
retrieval.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check all usages of DefaultConnectionHandlerFactory
echo "=== All references to DefaultConnectionHandlerFactory ==="
rg -n "DefaultConnectionHandlerFactory" --type=java -C3

echo -e "\n=== Check if this.pipeline is referenced in DefaultConnectionHandlerFactory ==="
rg -n "this\.pipeline" --type=java -C2

Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template

Length of output: 3172

---

🏁 Script executed:

#!/bin/bash
# Read the complete DefaultConnectionHandlerFactory.java file
echo "=== Complete DefaultConnectionHandlerFactory.java ==="
cat -n src/main/java/org/juv25d/DefaultConnectionHandlerFactory.java

echo -e "\n=== Check ConnectionHandlerFactory interface ==="
rg -A10 "interface ConnectionHandlerFactory" --type=java

Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template

Length of output: 1319

---

pipeline field is stored but never used — the create() method shadows it with its parameter.

The constructor accepts and stores a Pipeline (line 16), but create(Socket, Pipeline) receives its own pipeline parameter (line 20) and passes that to ConnectionHandler (line 21). The field this.pipeline is dead code. The ConnectionHandlerFactory interface requires the create() signature to include the Pipeline parameter, and App.java (line 42) passes a pipeline to the constructor while Server.java passes its own pipeline when invoking create(), making the factory-stored one unnecessary.

Remove pipeline from the constructor and constructor parameter to eliminate the dead code:

Suggested fix

-public class DefaultConnectionHandlerFactory implements ConnectionHandlerFactory{
-    private final HttpParser httpParser;
-    private final Logger logger;
-    private final Pipeline pipeline;
-
-    public DefaultConnectionHandlerFactory(HttpParser httpParser, Logger logger, Pipeline pipeline) {
+public class DefaultConnectionHandlerFactory implements ConnectionHandlerFactory {
+    private final HttpParser httpParser;
+    private final Logger logger;
+
+    public DefaultConnectionHandlerFactory(HttpParser httpParser, Logger logger) {
         this.httpParser = httpParser;
         this.logger = logger;
-        this.pipeline = pipeline;
     }

     `@Override`
     public Runnable create(Socket socket, Pipeline pipeline) {
         return new ConnectionHandler(socket, httpParser, logger, pipeline);
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/DefaultConnectionHandlerFactory.java` around lines 8
- 22, DefaultConnectionHandlerFactory stores a dead pipeline field and shadows
it in create(Socket, Pipeline); remove the unused Pipeline field and its
constructor parameter from DefaultConnectionHandlerFactory, update the
constructor signature to only accept (HttpParser httpParser, Logger logger),
delete the private final Pipeline pipeline field and any assignments to it, and
ensure create(Socket socket, Pipeline pipeline) continues to accept the pipeline
parameter and passes that pipeline to the ConnectionHandler constructor (new
ConnectionHandler(socket, httpParser, logger, pipeline)).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

headers() returns the internal mutable map — breaks encapsulation and risks ConcurrentModificationException.

body() (line 51-53) defensively clones, but headers() returns the raw internal map. Any caller (including HttpResponseWriter.writeHeaders which iterates it) can see or cause concurrent modifications. This inconsistency is noted in the class Javadoc's intent to allow mutation by filters, but it means two filters or a filter and the writer can interfere with each other.

If mutation by callers is intentional (filters call response.setHeader()), then the direct return is fine — but document it and don't claim encapsulation. If encapsulation is intended, return an unmodifiable view or a copy.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/http/HttpResponse.java` around lines 43 - 45,
headers() currently exposes the internal mutable Map which breaks encapsulation
and risks concurrent modification; change headers() to return an unmodifiable
view or defensive copy (e.g., Collections.unmodifiableMap(new
HashMap<>(headers)) or Collections.unmodifiableMap(headers)) to prevent callers
from mutating it directly, and update callers like
HttpResponseWriter.writeHeaders to use response.setHeader()/addHeader() (or
operate on a defensive copy) if mutation is required; if you intend to allow
external mutation, instead document headers() clearly in the class Javadoc and
provide controlled mutation APIs such as
setHeader(String,String)/removeHeader(String) to centralize mutation.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Empty handle() will cause a NullPointerException in HttpResponseWriter.

HttpResponse's no-arg constructor (used throughout the pipeline) leaves headers as null. When HttpResponseWriter.writeHeaders() is called after HelloPlugin.handle() it iterates headers.entrySet() → NPE. The status line will also render as "HTTP/1.1 0 null" since statusText is also unset.

Two changes are needed:

1. HelloPlugin.handle() must set at least a valid status code, status text, and empty headers.
2. HttpResponse() no-arg constructor should defensively initialize headers to avoid this class of bug across all plugins:

🐛 Proposed fixes

HelloPlugin.java — set a minimal valid response:

 `@Override`
 public void handle(HttpRequest req, HttpResponse res) throws IOException {
-    // placeholder response logic
+    res.setStatusCode(200);
+    res.setStatusText("OK");
+    res.setBody("Hello, World!".getBytes(StandardCharsets.UTF_8));
 }

HttpResponse.java — initialize headers in the no-arg constructor:

-public HttpResponse(){}
+public HttpResponse(){
+    this.headers = new LinkedHashMap<>();
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/plugin/HelloPlugin.java` around lines 11 - 13,
HelloPlugin.handle() is currently empty which leads to NullPointerException in
HttpResponseWriter.writeHeaders() because HttpResponse.headers is null and
statusText/statusCode are unset; update HelloPlugin.handle(HttpRequest req,
HttpResponse res) to set a minimal valid response (e.g., res.setStatusCode(200),
res.setStatusText("OK"), and initialize headers to an empty map via
res.setHeaders(...)) and also modify HttpResponse's no-arg constructor to
defensively initialize the headers field (e.g., new HashMap<>()), ensuring
HttpResponseWriter.writeHeaders() can safely iterate headers and the status line
renders correctly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

getLogLevel() can return null, causing NPE in ServerLogging.

If the logging section is absent from the YAML (or the file is empty), logLevel is never assigned and remains null. When ServerLogging.configure() calls ConfigLoader.getInstance().getLogLevel() as the fallback default, levelName.toUpperCase() (line 33 of ServerLogging.java) will throw a NullPointerException.

Apply defaults for all fields so that missing YAML sections degrade gracefully, and chain the original exception for debuggability.

Proposed fix

 private void loadConfiguration() {
     Yaml yaml = new Yaml();

     try (InputStream input = getClass().getClassLoader().getResourceAsStream("application-properties.yml")) {
         if (input == null) {
             throw new IllegalArgumentException("Did not find application-properties.yml");
         }

         Map<String, Object> config = yaml.load(input);
+        if (config == null) {
+            return; // empty YAML file – keep defaults
+        }

         // server
         Map<String, Object> serverConfig = (Map<String, Object>) config.get("server");
         if (serverConfig != null) {
             this.port = (Integer) serverConfig.getOrDefault("port", 8080);
             this.rootDirectory = (String) serverConfig.getOrDefault("root-dir", "static");
         }

         // logging
         Map<String, Object> loggingConfig = (Map<String, Object>) config.get("logging");
         if (loggingConfig != null) {
             this.logLevel = (String) loggingConfig.get("level");
         }

     } catch (Exception e) {
-        throw new RuntimeException("Failed to load application config");
+        throw new RuntimeException("Failed to load application config", e);
     }
 }

And initialise defaults on the fields:

 private static ConfigLoader instance;
-private int port;
-private String logLevel;
-private String rootDirectory;
+private int port = 8080;
+private String logLevel = "INFO";
+private String rootDirectory = "static";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/util/ConfigLoader.java` around lines 25 - 50,
loadConfiguration currently leaves fields like logLevel unset when YAML sections
are missing, causing getLogLevel() to return null and NPE in
ServerLogging.configure; update ConfigLoader to initialize sensible defaults for
all fields (e.g., port=8080, rootDirectory="static", logLevel="INFO") as class
field initializers or before parsing, and in loadConfiguration only override
them when config entries exist (use getOrDefault or null-checks); also when
catching Exception in loadConfiguration, rethrow a RuntimeException that chains
the original exception (new RuntimeException("Failed to load application
config", e)) so callers can debug root causes; verify references:
loadConfiguration(), getLogLevel(), and
ServerLogging.configure()/ConfigLoader.getInstance().getLogLevel().

[coderabbitai]

⚠️ Potential issue | 🟠 Major

popstate handler creates duplicate history entries.

navigate() calls history.pushState on line 54. When the browser fires popstate (back/forward), calling navigate() pushes a new entry on top, breaking back/forward navigation entirely.

Use a separate content-update function that skips pushState, or pass a flag:

Proposed fix

 window.addEventListener("popstate", () => {
-    navigate(window.location.pathname);
+    updateContent(window.location.pathname);
 });

Then extract the fetch-and-replace logic from navigate into a shared updateContent(href) function, and have navigate call updateContent followed by history.pushState.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/js/app.js` around lines 16 - 18, The popstate
handler calls navigate(), which itself calls history.pushState and thus creates
duplicate history entries; refactor the fetch-and-replace logic out of navigate
into a new updateContent(href) function (or add a pushState boolean parameter to
navigate) so that navigate(href) calls updateContent(href) then
history.pushState(...), while the popstate handler calls
updateContent(window.location.pathname) (or navigate(window.location.pathname,
{push: false})) to update the page without pushing a new history entry.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pom.xml (1)

32-37: ⚠️ Potential issue | 🟡 Minor

Exclude transitive JUnit 5 dependency from mockito-junit-jupiter to prevent classpath conflicts with JUnit 6.

mockito-junit-jupiter:5.21.0 declares a runtime dependency on junit-jupiter-api:5.13.4 (JUnit 5). While Mockito's JUnit 6 support is not yet officially declared (GitHub issue #3779, opened Jan 2026, remains open), mockito-junit-jupiter is likely compatible with JUnit 6 since MockitoExtension relies on core callback interfaces that were preserved in JUnit 6.

However, mixing both JUnit 5 and JUnit 6 artifacts on the classpath risks resolution conflicts. To safely use mockito-junit-jupiter with JUnit 6, exclude the transitive JUnit 5 dependency:

Suggested fix

<dependency>
    <groupId>org.mockito</groupId>
    <artifactId>mockito-junit-jupiter</artifactId>
    <version>${mockito.version}</version>
    <scope>test</scope>
    <exclusions>
        <exclusion>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-api</artifactId>
        </exclusion>
    </exclusions>
</dependency>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pom.xml` around lines 32 - 37, The mockito-junit-jupiter dependency pulls in
junit-jupiter-api (JUnit 5) transitively which can conflict with JUnit 6; update
the mockito-junit-jupiter dependency declaration (artifactId:
mockito-junit-jupiter, version: ${mockito.version}) to add an <exclusions> entry
that excludes groupId org.junit.jupiter and artifactId junit-jupiter-api so the
JUnit 5 API is not on the test classpath.

🧹 Nitpick comments (1)

pom.xml (1)

122-138: spotless-maven-plugin has no <executions> — it will never run during the standard build lifecycle.

Without binding to a phase (e.g., validate for spotless:check), spotless only runs when invoked explicitly (mvn spotless:check / mvn spotless:apply). If the intent is to enforce formatting in CI, add an execution:

♻️ Proposed fix to bind spotless:check to validate phase

 <plugin>
     <groupId>com.diffplug.spotless</groupId>
     <artifactId>spotless-maven-plugin</artifactId>
     <version>3.2.1</version>
     <configuration>
         <java>
             <includes>
                 <include>src/main/java/**/*.java</include>
                 <include>src/test/java/**/*.java</include>
             </includes>
             <removeUnusedImports />
             <trimTrailingWhitespace />
             <formatAnnotations />
         </java>
     </configuration>
+    <executions>
+        <execution>
+            <goals>
+                <goal>check</goal>
+            </goals>
+        </execution>
+    </executions>
 </plugin>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pom.xml` around lines 122 - 138, The Spotless plugin configuration for
com.diffplug.spotless:spotless-maven-plugin lacks an <executions> section so it
won't run during the standard Maven lifecycle; update the plugin block
(spotless-maven-plugin) to add an <executions> element that binds the desired
goals (e.g., spotless:check and/or spotless:apply) to an appropriate phase such
as validate (or verify) so formatting is enforced in CI, ensuring the plugin
will execute automatically during builds.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@pom.xml`:
- Around line 32-37: The mockito-junit-jupiter dependency pulls in
junit-jupiter-api (JUnit 5) transitively which can conflict with JUnit 6; update
the mockito-junit-jupiter dependency declaration (artifactId:
mockito-junit-jupiter, version: ${mockito.version}) to add an <exclusions> entry
that excludes groupId org.junit.jupiter and artifactId junit-jupiter-api so the
JUnit 5 API is not on the test classpath.

---

Duplicate comments:
In `@pom.xml`:
- Around line 139-160: The pitest-maven plugin configuration currently depends
on org.pitest:pitest-junit5-plugin version 1.2.2 which targets JUnit Platform
1.x and is incompatible with JUnit Jupiter 6.x; update the pitest-junit5-plugin
dependency under the pitest-maven <plugin> (artifactId pitest-junit5-plugin) to
a version that supports JUnit Platform 6.x (bump the <version> to a compatible
release, e.g., >=1.3.0 or the latest plugin that documents JUnit 6 support) and
run your mutation tests to confirm they discover and execute JUnit 6 tests.

---

Nitpick comments:
In `@pom.xml`:
- Around line 122-138: The Spotless plugin configuration for
com.diffplug.spotless:spotless-maven-plugin lacks an <executions> section so it
won't run during the standard Maven lifecycle; update the plugin block
(spotless-maven-plugin) to add an <executions> element that binds the desired
goals (e.g., spotless:check and/or spotless:apply) to an appropriate phase such
as validate (or verify) so formatting is enforced in CI, ensuring the plugin
will execute automatically during builds.