Add IP filter to request pipeline

[HerrKanin]

Closes #43

This PR adds an IP filter to the request pipeline.

• Introduces an IP filter that can allow or block requests based on client IP
• Extends HttpRequest with client IP information
• Client IP is populated from the socket in ConnectionHandler
• Filter is currently configured with open access to allow group development
  (whitelist/blacklist can be enabled when needed)

The implementation was tested by blocking localhost and verifying HTTP 403 responses.

Summary by CodeRabbit

• New Features

  • IP-based access control filter added (optional whitelist/blacklist).
  • Requests now carry client IP information throughout request handling.

• Behavior

  • When no lists are configured, traffic is allowed by default.
  • Blocked clients receive a 403 Forbidden with a plain-text message.

• Tests

  • Tests updated to accommodate the adjusted request shape.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds IP-based access control and propagates client IP through requests: new IpFilter plus pipeline insertion; HttpRequest gains remoteIp; HttpParser and ConnectionHandler populate it; tests updated to supply the extra constructor argument.

Changes

Cohort / File(s)	Summary
IP Filter Implementation src/main/java/org/juv25d/filter/IpFilter.java	New IpFilter implementing Filter; enforces optional whitelist/blacklist, responds 403 for blocked IPs, delegates when allowed.
Pipeline wiring src/main/java/org/juv25d/App.java	Registers and inserts IpFilter (empty sets) into filter pipeline before LoggingFilter; minor ordering/formatting tweaks.
Request model & parsing src/main/java/org/juv25d/http/HttpRequest.java, src/main/java/org/juv25d/http/HttpParser.java	HttpRequest record gains remoteIp field; HttpParser updated to call new constructor (uses "UNKNOWN" when IP not set).
Connection extraction src/main/java/org/juv25d/ConnectionHandler.java	Extracts remote IP from socket, rebuilds HttpRequest including remoteIp, and forwards it into the filter chain.
Tests updated src/test/java/org/juv25d/filter/FilterChainImplTest.java, src/test/java/org/juv25d/handler/StaticFileHandlerTest.java, src/test/java/org/juv25d/plugin/NotFoundPluginTest.java	Updated HttpRequest constructor calls to include the new trailing String (e.g., "UNKNOWN").

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant Conn as ConnectionHandler
    participant Parser as HttpParser
    participant IpF as IpFilter
    participant Chain as FilterChain
    participant Res as HttpResponse

    Client->>Conn: send TCP request
    Conn->>Conn: extract remoteIp from socket
    Conn->>Parser: parse raw request bytes
    Parser-->>Conn: HttpRequest(..., "UNKNOWN") (if parser has no IP)
    Conn->>IpF: pass HttpRequest(with remoteIp)
    alt IP allowed
        IpF->>Chain: doFilter -> continue pipeline
        Chain->>Res: handlers produce response
    else IP blocked
        IpF->>Res: set 403 Forbidden response
    end
    Res->>Client: send response

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly Related PRs

• feature/FilterPlugin #17 — Prior changes to filter/plugin pipeline and filter registration.
• Feature/global filter #51 — Overlapping updates to pipeline and ConnectionHandler request propagation.
• Implement static file handler (foundation for #18) #36 — Related modifications to App pipeline setup and global filters.

Suggested labels

enhancement

Suggested reviewers

• kristina0x7
• fmazmz

Poem

🐇 I hopped through bytes and sniffed the trail,
I learned each IP, both near and frail,
I nibble whitelist crumbs with care,
I thump when strangers linger there,
Safe burrow, guarded by my tail.

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The implementation provides IP filtering logic but does not meet the configuration requirement from issue #43, which mandates whitelist/blacklist configuration via the main config file.	Implement configuration loading from the main config file or a separate configuration file to dynamically set whitelist/blacklist instead of hardcoding empty lists in App.java.
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add IP filter to request pipeline' clearly and concisely summarizes the main change of implementing IP-based access control in the request processing pipeline.
Out of Scope Changes check	✅ Passed	All changes directly support the IP filtering feature: HttpRequest extension with remoteIp, ConnectionHandler IP extraction, IpFilter implementation, and test updates are all in scope.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/ip-filter

---

No actionable comments were generated in the recent review. 🎉

---

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: 2

🤖 Fix all issues with AI agents

In `@src/main/java/org/juv25d/ConnectionHandler.java`:
- Around line 32-43: Guard against socket.getInetAddress() returning null and
stop manually copying fields; implement a copy method on the HttpRequest record
(e.g., withRemoteIp(String remoteIp)) that returns a new HttpRequest with the
same fields but different remoteIp, then in ConnectionHandler use InetAddress
addr = socket.getInetAddress(); String remoteIp = addr != null ?
addr.getHostAddress() : socket.getRemoteSocketAddress() != null ?
socket.getRemoteSocketAddress().toString() : "unknown"; and create the request
via parsed.withRemoteIp(remoteIp) instead of reconstructing all fields.

In `@src/main/java/org/juv25d/filter/IpFilter.java`:
- Around line 44-53: The forbidden(HttpResponse res, String ip) method in
IpFilter currently echoes the client IP in the 403 response body, which risks
disclosing internal/proxy IPs; change it to return a generic 403 message (e.g.,
"403 Forbidden") or make verbosity configurable so the body does not include the
ip variable. Update the forbidden(...) implementation to remove concatenation of
ip from the response body (or gate that behavior behind a configuration flag in
IpFilter) and ensure Content-Length and body bytes reflect the new generic
message.

🧹 Nitpick comments (4)

src/main/java/org/juv25d/App.java (1)

18-21: Hardcoded empty sets — consider loading from configuration.

Both sets are empty, making the filter a no-op. Per issue #43, whitelist/blacklist should be configurable via the main config file or a separate configuration. This is fine as a development default, but consider loading these from configuration so the feature can be enabled without code changes.

Also, placing IpFilter before LoggingFilter means blocked requests won't be logged. If security auditing of denied requests is desired, consider swapping the order or adding a log statement inside IpFilter.forbidden().

src/main/java/org/juv25d/http/HttpParser.java (1)

60-60: "UNKNOWN" placeholder is fragile — consider a factory method or builder.

The parser shouldn't need to supply a transport-layer value. Since HttpRequest is a record, every construction site must pass all fields, so adding a field requires updating every call site. A static factory method on HttpRequest (e.g., HttpRequest.fromParsed(...) that defaults remoteIp to null) or a withRemoteIp(String) copy method would decouple parsing from transport concerns and reduce the maintenance burden.

src/main/java/org/juv25d/filter/IpFilter.java (2)

15-18: Consider defensive copies of the sets.

The constructor stores direct references to the passed sets. If a caller passes a mutable HashSet, it could be modified after construction, silently changing filter behavior at runtime. While the current call site uses Set.of() (immutable), the class API should be safe regardless.

Proposed fix

     public IpFilter(Set<String> whitelist, Set<String> blacklist) {
-        this.whitelist = whitelist;
-        this.blacklist = blacklist;
+        this.whitelist = whitelist != null ? Set.copyOf(whitelist) : Set.of();
+        this.blacklist = blacklist != null ? Set.copyOf(blacklist) : Set.of();
     }

This also eliminates the null checks in doFilter since the sets would always be non-null.

---

40-42: getClientIp only checks socket IP — won't work behind a reverse proxy.

If the server sits behind a load balancer or reverse proxy, remoteIp will always be the proxy's address. Standard practice is to also check headers like X-Forwarded-For or X-Real-IP (with caution, as these can be spoofed by clients when there's no trusted proxy). This is likely out of scope for this PR, but worth keeping in mind for production use.

[annikaholmqvist94]

Great addition, IP filtering is a valuable security feature.

Highlights

• Clean whitelist/blacklist implementation
• Smart default-open for development, easy to lock down later
• Good integration with existing pipeline-architecture
• Clear 403 responses for blocked clients

Suggestion
Consider adding a comment in App.java explaining the Set.of() configuration is intentionally open for development.

Solid implementation that's flexible and production-ready.

Approved

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/main/java/org/juv25d/App.java`:
- Around line 23-28: The pipeline currently adds LoggingFilter twice (once via
pipeline.addFilter(new LoggingFilter()) and again via
pipeline.addGlobalFilter(new LoggingFilter(), 0)), causing duplicate logs;
remove the duplicate by deleting the pipeline.addFilter(new LoggingFilter())
call (leaving the global filter) or vice versa—ensure only one LoggingFilter is
registered (either via addFilter or addGlobalFilter) and keep the IpFilter(...)
line intact.

🧹 Nitpick comments (1)

src/main/java/org/juv25d/App.java (1)

21-26: IP filter lists are hardcoded — issue #43 requires config-driven whitelist/blacklist.

The linked issue specifies that whitelist/blacklist should be configurable via the main config file (or a referenced file). Currently the sets are hardcoded as empty. ConfigLoader is already available on line 15 — consider loading the IP lists from it. The current no-op setup is fine as a stepping stone, but this should be tracked as a follow-up.

Would you like me to open an issue to track loading IP filter configuration from ConfigLoader?