From 3bbc8a4c9c946d97cc2fa6b91115350b0c2dd7aa Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Tue, 24 Feb 2026 16:11:47 +0100 Subject: [PATCH 1/7] get/add methods, support for subnets and proxies Signed-off-by: Jesper Larsson --- pom.xml | 6 + src/main/java/org/juv25d/filter/IpFilter.java | 160 ++++++++++++++++-- 2 files changed, 148 insertions(+), 18 deletions(-) diff --git a/pom.xml b/pom.xml index 0fdd4e15..0e41300c 100644 --- a/pom.xml +++ b/pom.xml @@ -54,6 +54,12 @@ 8.16.1 compile + + commons-net + commons-net + 3.12.0 + compile + org.testcontainers testcontainers diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java index b020a6ac..562d55d4 100644 --- a/src/main/java/org/juv25d/filter/IpFilter.java +++ b/src/main/java/org/juv25d/filter/IpFilter.java @@ -1,62 +1,166 @@ package org.juv25d.filter; +import org.apache.commons.net.util.SubnetUtils; import org.juv25d.config.IpFilterConfig; import org.juv25d.filter.annotation.Global; import org.juv25d.http.HttpRequest; import org.juv25d.http.HttpResponse; +import org.juv25d.logging.ServerLogging; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.HashSet; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Logger; + @Global(order = 2) public class IpFilter implements Filter { - private final Set whitelist = new HashSet<>(); - private final Set blacklist = new HashSet<>(); + private static final Logger logger = ServerLogging.getLogger(); + + private final Set whitelist = ConcurrentHashMap.newKeySet(); + private final Set blacklist = ConcurrentHashMap.newKeySet(); + + private final Map whitelistSubnets = new ConcurrentHashMap<>(); + private final Map blacklistSubnets = new ConcurrentHashMap<>(); private final boolean allowByDefault; public IpFilter(Set whitelist, Set blacklist, boolean allowByDefault) { if (whitelist != null) { - this.whitelist.addAll(whitelist); + for (String entry : whitelist) { + addToWhitelist(entry); + } } if (blacklist != null) { - this.blacklist.addAll(blacklist); + for (String entry : blacklist) { + addToBlacklist(entry); + } } this.allowByDefault = allowByDefault; } public IpFilter() { IpFilterConfig config = new IpFilterConfig(); - this.whitelist.addAll(config.whitelist()); - this.blacklist.addAll(config.blacklist()); + for (String entry : config.whitelist()) { + addToWhitelist(entry); + } + for (String entry : config.blacklist()) { + addToBlacklist(entry); + } this.allowByDefault = config.allowByDefault(); } - @Override - public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throws IOException { - String clientIp = getClientIp(req); + public void addToWhitelist(String ipOrCidr) { + if (ipOrCidr == null || ipOrCidr.isBlank()) return; + + if (ipOrCidr.contains("/")) { + try { + SubnetUtils subnet = new SubnetUtils(ipOrCidr); + subnet.setInclusiveHostCount(true); + whitelistSubnets.put(ipOrCidr, subnet); + } catch (IllegalArgumentException e) { + logger.warning("Invalid CIDR format for whitelist: " + ipOrCidr + ": " + e.getMessage()); + } + } else { + whitelist.add(ipOrCidr); + } + } - if (isAllowed(clientIp)) { - chain.doFilter(req, res); + public void addToBlacklist(String ipOrCidr) { + if (ipOrCidr == null || ipOrCidr.isBlank()) return; + + if (ipOrCidr.contains("/")) { + try { + SubnetUtils subnet = new SubnetUtils(ipOrCidr); + subnet.setInclusiveHostCount(true); + blacklistSubnets.put(ipOrCidr, subnet); + } catch (IllegalArgumentException e) { + logger.warning("Invalid CIDR format for blacklist: " + ipOrCidr + ": " + e.getMessage()); + } } else { - forbidden(res, clientIp); + blacklist.add(ipOrCidr); } } - public boolean isAllowed(String ip) { + public void removeFromWhitelist(String ipOrCidr) { + if (ipOrCidr == null || ipOrCidr.isBlank()) return; + + if (ipOrCidr.contains("/")) { + whitelistSubnets.remove(ipOrCidr); + } else { + whitelist.remove(ipOrCidr); + } + } + + public void removeFromBlacklist(String ipOrCidr) { + if (ipOrCidr == null || ipOrCidr.isBlank()) return; + + if (ipOrCidr.contains("/")) { + blacklistSubnets.remove(ipOrCidr); + } else { + blacklist.remove(ipOrCidr); + } + } + + @Override + public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throws IOException { + try { + String clientIp = getClientIp(req); + + if (isAllowed(clientIp)) { + chain.doFilter(req, res); + } else { + logger.fine("IP blocked: " + clientIp); + forbidden(res, clientIp); + } + } catch (Exception e) { + logger.severe("Error in IP filter: " + e.getMessage()); + forbidden(res, "error"); + } + } - if (whitelist.contains(ip) && blacklist.contains(ip)) return allowByDefault; + public boolean isAllowed(String ip) { + if (ip == null || ip.isBlank()) { + logger.finer("Null or blank IP address, denying access"); + return false; + } - if (whitelist.contains(ip)) return true; + // Whitelist prio + if (whitelist.contains(ip) || isInSubnets(ip, whitelistSubnets)) return true; - if (blacklist.contains(ip)) return false; + if (blacklist.contains(ip) || isInSubnets(ip, blacklistSubnets)) return false; return allowByDefault; } - private String getClientIp(HttpRequest req){ + private boolean isInSubnets(String ip, Map subnets) { + for (SubnetUtils subnet : subnets.values()) { + try { + if (subnet.getInfo().isInRange(ip)) { + return true; + } + } catch (IllegalArgumentException e) { + logger.finer("Invalid IP format for range check: " + ip); + } + } + return false; + } + + private String getClientIp(HttpRequest req) { + Map headers = req.headers(); + + String ip = headers.get("X-Forwarded-For"); + if (ip != null && !ip.isBlank()) { + return ip.split(",")[0].trim(); + } + + ip = headers.get("X-Real-IP"); + if (ip != null && !ip.isBlank()) { + return ip.trim(); + } + return req.remoteIp(); } @@ -70,4 +174,24 @@ private void forbidden(HttpResponse res, String ip) { res.setHeader("Content-Length", String.valueOf(body.length)); res.setBody(body); } + + public Set getWhitelistIps() { + return Set.copyOf(whitelist); + } + + public Set getBlacklistIps() { + return Set.copyOf(blacklist); + } + + public Set getWhitelistSubnets() { + return Set.copyOf(whitelistSubnets.keySet()); + } + + public Set getBlacklistSubnets() { + return Set.copyOf(blacklistSubnets.keySet()); + } + + public boolean getAllowByDefault() { + return allowByDefault; + } } From 21ef1dca12b4051f608318bb52334c7002941f43 Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Wed, 25 Feb 2026 15:22:03 +0100 Subject: [PATCH 2/7] extended IpFilterTest Signed-off-by: Jesper Larsson --- .../java/org/juv25d/filter/IpFilterTest.java | 215 ++++++++++++++++-- 1 file changed, 202 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/juv25d/filter/IpFilterTest.java b/src/test/java/org/juv25d/filter/IpFilterTest.java index bef44170..fb5f9693 100644 --- a/src/test/java/org/juv25d/filter/IpFilterTest.java +++ b/src/test/java/org/juv25d/filter/IpFilterTest.java @@ -1,24 +1,48 @@ package org.juv25d.filter; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; import org.juv25d.http.HttpRequest; import org.juv25d.http.HttpResponse; import java.io.IOException; +import java.util.Map; import java.util.Set; -import static org.junit.jupiter.api.Assertions.assertEquals; + +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; class IpFilterTest { - HttpRequest req = mock(HttpRequest.class); - HttpResponse res = new HttpResponse(); - FilterChain chain = mock(FilterChain.class); + HttpRequest req; + HttpResponse res; + FilterChain chain; + + @BeforeEach + void setUp() { + req = mock(HttpRequest.class); + when(req.headers()).thenReturn(Map.of()); + when(req.remoteIp()).thenReturn("127.0.0.1"); + + res = new HttpResponse(); + chain = mock(FilterChain.class); + } @Test void whitelist_allowsIp() throws IOException { IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false); - when(req.remoteIp()).thenReturn("127.0.0.1"); + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); + assertEquals(200, res.statusCode()); + } + + @Test + void whitelist_allowsIpInRange() throws IOException { + IpFilter filter = new IpFilter(Set.of("127.0.0.0/24"), null, false); filter.doFilter(req, res, chain); @@ -30,38 +54,203 @@ void whitelist_allowsIp() throws IOException { void blacklist_blocksIp() throws IOException { IpFilter filter = new IpFilter(null, Set.of("127.0.0.1"), true); - when(req.remoteIp()).thenReturn("127.0.0.1"); + filter.doFilter(req, res, chain); + verify(chain, never()).doFilter(req, res); + + assertEquals(403, res.statusCode()); + assertEquals("Forbidden", res.statusText()); + } + + @Test + void blacklist_blocksIpInRange() throws IOException { + IpFilter filter = new IpFilter(null, Set.of("127.0.0.0/24"), true); filter.doFilter(req, res, chain); verify(chain, never()).doFilter(req, res); assertEquals(403, res.statusCode()); assertEquals("Forbidden", res.statusText()); + } + + @Test + void whitelist_overrides_blacklist() throws IOException { + IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.0/24"), false); + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); + assertEquals(200, res.statusCode()); } @Test - void allowsIP_inBothList_defaultTrue() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.1"), true); + void mixedIpAndSubnet_bothWork() throws IOException { + IpFilter filter = new IpFilter(Set.of("127.0.0.1", "192.168.0.0/16"), null, false); - when(req.remoteIp()).thenReturn("127.0.0.1"); + filter.doFilter(req, res, chain); + verify(chain).doFilter(req, res); + + when(req.remoteIp()).thenReturn("192.168.5.100"); + filter.doFilter(req, res, chain); + verify(chain, times(2)).doFilter(req, res); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException { + IpFilter filter = new IpFilter(null, null, allowByDefault); + + filter.doFilter(req, res, chain); + + if(allowByDefault) { + verify(chain).doFilter(req, res); + assertEquals(200, res.statusCode()); + } + else { + verify(chain, never()).doFilter(req, res); + assertEquals(403, res.statusCode()); + } + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + void addIpOrRange_whitelist(String ipOrCidr) throws IOException { + IpFilter filter = new IpFilter(null, null, false); + filter.addToWhitelist(ipOrCidr); filter.doFilter(req, res, chain); verify(chain).doFilter(req, res); assertEquals(200, res.statusCode()); + + assertTrue((filter.getWhitelistIps().contains(ipOrCidr)) || filter.getWhitelistSubnets().contains(ipOrCidr)); } - @Test - void blocksIP_inNeitherList_defaultFalse() throws IOException { + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + void addIpOrRange_blacklist(String ipOrCidr) throws IOException { IpFilter filter = new IpFilter(null, null, false); + filter.addToBlacklist(ipOrCidr); - when(req.remoteIp()).thenReturn("127.0.0.1"); + filter.doFilter(req, res, chain); + + verify(chain, never()).doFilter(req, res); + assertEquals(403, res.statusCode()); + + assertTrue((filter.getBlacklistIps().contains(ipOrCidr)) || filter.getBlacklistSubnets().contains(ipOrCidr)); + } + + @Test + void doesNotAddDuplicates() { + IpFilter filter = new IpFilter(Set.of("127.0.0.1", "127.0.0.0/24"), null, false); + + filter.addToWhitelist("127.0.0.1"); + filter.addToWhitelist("127.0.0.0/24"); // Add same CIDR twice + + assertEquals(1, filter.getWhitelistIps().size()); + assertEquals(1, filter.getWhitelistSubnets().size()); + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { + IpFilter filter = new IpFilter(Set.of(ipOrCidr), null, false); + filter.removeFromWhitelist(ipOrCidr); filter.doFilter(req, res, chain); + verify(chain, never()).doFilter(req, res); + assertEquals(403, res.statusCode()); + + assertFalse((filter.getWhitelistIps().contains(ipOrCidr)) || filter.getWhitelistSubnets().contains(ipOrCidr)); + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { + IpFilter filter = new IpFilter(null, Set.of(ipOrCidr), true); + filter.removeFromBlacklist(ipOrCidr); + + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); + assertEquals(200, res.statusCode()); + + assertFalse((filter.getBlacklistIps().contains(ipOrCidr)) || filter.getBlacklistSubnets().contains(ipOrCidr)); + } + + @ParameterizedTest + @NullAndEmptySource + void nullOrBlankIp_blocked(String ip) throws IOException { + IpFilter filter = new IpFilter(null, null, true); + when(req.remoteIp()).thenReturn(ip); + + filter.doFilter(req, res, chain); + + verify(chain, never()).doFilter(req, res); assertEquals(403, res.statusCode()); - assertEquals("Forbidden", res.statusText()); + } + + @Test + void invalidCidr_loggedAndIgnored() { + IpFilter filter = new IpFilter(null, null, false); + + filter.addToWhitelist("not-a-cidr/99"); + + assertEquals(0, filter.getWhitelistSubnets().size()); + } + + @Test + void get_returnsImmutableCopy() { + IpFilter filter = new IpFilter(null, null, false); + + assertThrows(UnsupportedOperationException.class, () -> + filter.getWhitelistIps().add("test")); + assertThrows(UnsupportedOperationException.class, () -> + filter.getWhitelistSubnets().add("test")); + assertThrows(UnsupportedOperationException.class, () -> + filter.getBlacklistIps().add("test")); + assertThrows(UnsupportedOperationException.class, () -> + filter.getBlacklistSubnets().add("test")); + } + + @Test + void xForwardedFor_takesFirstIp() throws IOException { + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); + + when(req.headers()).thenReturn(Map.of("X-Forwarded-For", "1.2.3.4, 5.6.7.8")); + when(req.remoteIp()).thenReturn("5.6.7.8"); + + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); + assertEquals(200, res.statusCode()); + } + + @Test + void xRealIp_overridesRemoteIp() throws IOException { + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); + + when(req.headers()).thenReturn(Map.of("X-Real-IP", "1.2.3.4")); + when(req.remoteIp()).thenReturn("5.6.7.8"); + + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); + } + + @Test + void xForwardedFor_priorityOverXRealIp() throws IOException { + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); + + when(req.headers()).thenReturn(Map.of( + "X-Forwarded-For", "1.2.3.4", + "X-Real-IP", "5.6.7.8" + )); + when(req.remoteIp()).thenReturn("9.9.9.9"); + + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); } } From 7f30e55341e7e567f78f87ea554d4a785dfd0441 Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Thu, 26 Feb 2026 12:30:36 +0100 Subject: [PATCH 3/7] javadocs and test displaynames Signed-off-by: Jesper Larsson --- src/main/java/org/juv25d/filter/IpFilter.java | 122 ++++++++++++++++++ .../java/org/juv25d/filter/IpFilterTest.java | 32 +++-- 2 files changed, 141 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java index 562d55d4..2dc83935 100644 --- a/src/main/java/org/juv25d/filter/IpFilter.java +++ b/src/main/java/org/juv25d/filter/IpFilter.java @@ -14,6 +14,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; +/** + * IP-based access control filter that allows or denies HTTP requests based on client IP addresses. + */ @Global(order = 2) public class IpFilter implements Filter { @@ -27,6 +30,15 @@ public class IpFilter implements Filter { private final boolean allowByDefault; + /** + * Constructs an IP filter with specified whitelist, blacklist, and default policy. + * + * @param whitelist set of IPs/CIDR ranges to always allow (can be null) + * @param blacklist set of IPs/CIDR ranges to always block (can be null) + * @param allowByDefault whether to allow IPs not in either list + * + * @throws IllegalArgumentException if any CIDR notation is invalid + * */ public IpFilter(Set whitelist, Set blacklist, boolean allowByDefault) { if (whitelist != null) { for (String entry : whitelist) { @@ -41,6 +53,9 @@ public IpFilter(Set whitelist, Set blacklist, boolean allowByDe this.allowByDefault = allowByDefault; } + /** + * Constructs an IP filter using configuration from {@link IpFilterConfig}. + */ public IpFilter() { IpFilterConfig config = new IpFilterConfig(); for (String entry : config.whitelist()) { @@ -52,6 +67,11 @@ public IpFilter() { this.allowByDefault = config.allowByDefault(); } + /** + * Adds an IP address or CIDR range to the whitelist. + * + * @param ipOrCidr IP address (e.g., "192.168.1.1") or CIDR range (e.g., "10.0.0.0/8") + */ public void addToWhitelist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; @@ -68,6 +88,11 @@ public void addToWhitelist(String ipOrCidr) { } } + /** + * Adds an IP address or CIDR range to the blacklist. + * + * @param ipOrCidr IP address (e.g., "192.168.1.1") or CIDR range (e.g., "10.0.0.0/8") + */ public void addToBlacklist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; @@ -84,6 +109,18 @@ public void addToBlacklist(String ipOrCidr) { } } + /** + * Removes an IP address or CIDR range from the whitelist. + *

+ * The entry must match exactly (same format and value) to be removed. + * Removing a CIDR range does not affect individual IPs within that range that were + * added separately. + * + * @param ipOrCidr IP address or CIDR range to remove (must match exactly) + *

+ * {@link #getWhitelistIps()} + * {@link #getWhitelistSubnets()} + */ public void removeFromWhitelist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; @@ -94,6 +131,18 @@ public void removeFromWhitelist(String ipOrCidr) { } } + /** + * Removes an IP address or CIDR range from the blacklist. + *

+ * The entry must match exactly (same format and value) to be removed. + * Removing a CIDR range does not affect individual IPs within that range that were + * added separately. + * + * @param ipOrCidr IP address or CIDR range to remove (must match exactly) + *

+ * {@link #getBlacklistIps()} + * {@link #getBlacklistSubnets()} + */ public void removeFromBlacklist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; @@ -104,6 +153,21 @@ public void removeFromBlacklist(String ipOrCidr) { } } + /** + * Filters an HTTP request based on the client's IP address. + *

+ * Decision Logic: + *

    + *
  1. If IP is in whitelist → Allow (even if also in blacklist)
    2. + *
  2. If IP is in blacklist → Block
    3. + *
  3. Otherwise → Use default policy (allowByDefault)
    4. + *
+ * + * @param req the HTTP request + * @param res the HTTP response + * @param chain the filter chain to continue if allowed + * @throws IOException if an I/O error occurs during filtering + */ @Override public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throws IOException { try { @@ -121,6 +185,20 @@ public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throw } } + /** + * Checks if an IP address should be allowed. + *

+ * Decision Logic: + *

    + *
  1. Null or blank IPs → Deny
    2. + *
  2. Whitelist (exact or subnet match) → Allow
    3. + *
  3. Blacklist (exact or subnet match) → Deny
    4. + *
  4. Not in either list → Use default policy
    5. + *
+ * + * @param ip the IP address to check + * @return true if the IP should be allowed, false otherwise + */ public boolean isAllowed(String ip) { if (ip == null || ip.isBlank()) { logger.finer("Null or blank IP address, denying access"); @@ -135,6 +213,13 @@ public boolean isAllowed(String ip) { return allowByDefault; } + /** + * Checks if an IP address falls within any of the given subnets. + * + * @param ip the IP address to check + * @param subnets map of CIDR notations to SubnetUtils instances + * @return true if the IP is within any subnet, false otherwise + */ private boolean isInSubnets(String ip, Map subnets) { for (SubnetUtils subnet : subnets.values()) { try { @@ -148,6 +233,12 @@ private boolean isInSubnets(String ip, Map subnets) { return false; } + /** + * Extracts the client's IP address from the request, considering proxy headers. + * + * @param req the HTTP request + * @return the client's IP address + */ private String getClientIp(HttpRequest req) { Map headers = req.headers(); @@ -164,6 +255,12 @@ private String getClientIp(HttpRequest req) { return req.remoteIp(); } + /** + * Sends a 403 Forbidden response to the client. + * + * @param res the HTTP response + * @param ip the blocked IP address + */ private void forbidden(HttpResponse res, String ip) { byte[] body = ("403 Forbidden: IP not allowed (" + ip + ")\n") .getBytes(StandardCharsets.UTF_8); @@ -175,22 +272,47 @@ private void forbidden(HttpResponse res, String ip) { res.setBody(body); } + /** + * Returns an immutable copy of the exact IP whitelist (excludes subnets). + * + * @return immutable set of whitelisted IP addresses + */ public Set getWhitelistIps() { return Set.copyOf(whitelist); } + /** + * Returns an immutable copy of the exact IP blacklist (excludes subnets). + * + * @return immutable set of blacklisted IP addresses + */ public Set getBlacklistIps() { return Set.copyOf(blacklist); } + /** + * Returns an immutable copy of the whitelisted CIDR ranges. + * + * @return immutable set of whitelisted CIDR notations (e.g., "10.0.0.0/8") + */ public Set getWhitelistSubnets() { return Set.copyOf(whitelistSubnets.keySet()); } + /** + * Returns an immutable copy of the blacklisted CIDR ranges. + * + * @return immutable set of blacklisted CIDR notations (e.g., "192.168.0.0/16") + */ public Set getBlacklistSubnets() { return Set.copyOf(blacklistSubnets.keySet()); } + /** + * Returns the default policy for IPs not in either list. + * + * @return true if unknown IPs are allowed, false if denied + */ public boolean getAllowByDefault() { return allowByDefault; } diff --git a/src/test/java/org/juv25d/filter/IpFilterTest.java b/src/test/java/org/juv25d/filter/IpFilterTest.java index fb5f9693..f5942b9f 100644 --- a/src/test/java/org/juv25d/filter/IpFilterTest.java +++ b/src/test/java/org/juv25d/filter/IpFilterTest.java @@ -1,5 +1,6 @@ package org.juv25d.filter; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.NullAndEmptySource; @@ -31,6 +32,7 @@ void setUp() { } @Test + @DisplayName("Allow ip only in whitelist") void whitelist_allowsIp() throws IOException { IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false); @@ -41,6 +43,7 @@ void whitelist_allowsIp() throws IOException { } @Test + @DisplayName("Allow ip from CIDR range only in whitelist") void whitelist_allowsIpInRange() throws IOException { IpFilter filter = new IpFilter(Set.of("127.0.0.0/24"), null, false); @@ -51,6 +54,7 @@ void whitelist_allowsIpInRange() throws IOException { } @Test + @DisplayName("Block ip only in blacklist") void blacklist_blocksIp() throws IOException { IpFilter filter = new IpFilter(null, Set.of("127.0.0.1"), true); @@ -62,6 +66,7 @@ void blacklist_blocksIp() throws IOException { } @Test + @DisplayName("Block ip from CIDR range only in blacklist") void blacklist_blocksIpInRange() throws IOException { IpFilter filter = new IpFilter(null, Set.of("127.0.0.0/24"), true); @@ -73,6 +78,7 @@ void blacklist_blocksIpInRange() throws IOException { } @Test + @DisplayName("Allow ip in both list (whitelist prio)") void whitelist_overrides_blacklist() throws IOException { IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.0/24"), false); @@ -82,20 +88,9 @@ void whitelist_overrides_blacklist() throws IOException { assertEquals(200, res.statusCode()); } - @Test - void mixedIpAndSubnet_bothWork() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1", "192.168.0.0/16"), null, false); - - filter.doFilter(req, res, chain); - verify(chain).doFilter(req, res); - - when(req.remoteIp()).thenReturn("192.168.5.100"); - filter.doFilter(req, res, chain); - verify(chain, times(2)).doFilter(req, res); - } - @ParameterizedTest @ValueSource(booleans = {true, false}) + @DisplayName("Follow default when ip in neither list") void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException { IpFilter filter = new IpFilter(null, null, allowByDefault); @@ -113,6 +108,7 @@ void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + @DisplayName("Allow IP or CIDR range added in existing filter") void addIpOrRange_whitelist(String ipOrCidr) throws IOException { IpFilter filter = new IpFilter(null, null, false); filter.addToWhitelist(ipOrCidr); @@ -127,6 +123,7 @@ void addIpOrRange_whitelist(String ipOrCidr) throws IOException { @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + @DisplayName("Block IP or CIDR range added in existing filter") void addIpOrRange_blacklist(String ipOrCidr) throws IOException { IpFilter filter = new IpFilter(null, null, false); filter.addToBlacklist(ipOrCidr); @@ -140,11 +137,12 @@ void addIpOrRange_blacklist(String ipOrCidr) throws IOException { } @Test + @DisplayName("Adding IP or CIDR range already in filter doesn't create duplicates") void doesNotAddDuplicates() { IpFilter filter = new IpFilter(Set.of("127.0.0.1", "127.0.0.0/24"), null, false); filter.addToWhitelist("127.0.0.1"); - filter.addToWhitelist("127.0.0.0/24"); // Add same CIDR twice + filter.addToWhitelist("127.0.0.0/24"); assertEquals(1, filter.getWhitelistIps().size()); assertEquals(1, filter.getWhitelistSubnets().size()); @@ -152,6 +150,7 @@ void doesNotAddDuplicates() { @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + @DisplayName("Fall back on blacklist/default after removing IP or CIDR range from whitelist") void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { IpFilter filter = new IpFilter(Set.of(ipOrCidr), null, false); filter.removeFromWhitelist(ipOrCidr); @@ -166,6 +165,7 @@ void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) + @DisplayName("Fall back on whitelist/default after removing IP or CIDR range from whitelist") void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { IpFilter filter = new IpFilter(null, Set.of(ipOrCidr), true); filter.removeFromBlacklist(ipOrCidr); @@ -180,6 +180,7 @@ void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { @ParameterizedTest @NullAndEmptySource + @DisplayName("Block null or empty IP") void nullOrBlankIp_blocked(String ip) throws IOException { IpFilter filter = new IpFilter(null, null, true); @@ -192,6 +193,7 @@ void nullOrBlankIp_blocked(String ip) throws IOException { } @Test + @DisplayName("Ignore incorrectly formatted CIDR range") void invalidCidr_loggedAndIgnored() { IpFilter filter = new IpFilter(null, null, false); @@ -201,6 +203,7 @@ void invalidCidr_loggedAndIgnored() { } @Test + @DisplayName("Get methods return immutable copies") void get_returnsImmutableCopy() { IpFilter filter = new IpFilter(null, null, false); @@ -215,6 +218,7 @@ void get_returnsImmutableCopy() { } @Test + @DisplayName("Evaluates original client IP from X-Forwarded-For when proxied") void xForwardedFor_takesFirstIp() throws IOException { IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); @@ -228,6 +232,7 @@ void xForwardedFor_takesFirstIp() throws IOException { } @Test + @DisplayName("Uses X-Real-IP header when X-Forwarded-For is absent") void xRealIp_overridesRemoteIp() throws IOException { IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); @@ -240,6 +245,7 @@ void xRealIp_overridesRemoteIp() throws IOException { } @Test + @DisplayName("Prioritizes X-Forwarded-For over X-Real-IP when both present") void xForwardedFor_priorityOverXRealIp() throws IOException { IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); From 568c021b5dfff4e4e039843076b5369e4c21bf6e Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Thu, 26 Feb 2026 14:40:05 +0100 Subject: [PATCH 4/7] rabbit feedback Signed-off-by: Jesper Larsson --- .../org/juv25d/config/IpFilterConfig.java | 4 ++ src/main/java/org/juv25d/filter/IpFilter.java | 58 ++++++++++++++----- .../java/org/juv25d/filter/IpFilterTest.java | 49 ++++++++++------ 3 files changed, 77 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/juv25d/config/IpFilterConfig.java b/src/main/java/org/juv25d/config/IpFilterConfig.java index e8ee93f8..ec970cdd 100644 --- a/src/main/java/org/juv25d/config/IpFilterConfig.java +++ b/src/main/java/org/juv25d/config/IpFilterConfig.java @@ -14,7 +14,11 @@ public Set blacklist() { private final boolean allowByDefault = true; + private final boolean trustProxyHeaders = false; + public boolean allowByDefault() { return allowByDefault; } + + public boolean trustProxyHeaders() {return trustProxyHeaders;} } diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java index e4e8f774..c1a7280b 100644 --- a/src/main/java/org/juv25d/filter/IpFilter.java +++ b/src/main/java/org/juv25d/filter/IpFilter.java @@ -31,6 +31,7 @@ public class IpFilter implements Filter { private final Map blacklistSubnets = new ConcurrentHashMap<>(); private final boolean allowByDefault; + private final boolean trustProxyHeaders; /** * Constructs an IP filter with specified whitelist, blacklist, and default policy. @@ -38,10 +39,9 @@ public class IpFilter implements Filter { * @param whitelist set of IPs/CIDR ranges to always allow (can be null) * @param blacklist set of IPs/CIDR ranges to always block (can be null) * @param allowByDefault whether to allow IPs not in either list - * - * @throws IllegalArgumentException if any CIDR notation is invalid + * @param trustProxyHeaders whether to trust proxy headers when extracting IP from request * */ - public IpFilter(@Nullable Set whitelist, @Nullable Set blacklist, boolean allowByDefault) { + public IpFilter(@Nullable Set whitelist, @Nullable Set blacklist, boolean allowByDefault, boolean trustProxyHeaders) { if (whitelist != null) { for (String entry : whitelist) { addToWhitelist(entry); @@ -53,6 +53,7 @@ public IpFilter(@Nullable Set whitelist, @Nullable Set blacklist } } this.allowByDefault = allowByDefault; + this.trustProxyHeaders = trustProxyHeaders; } /** @@ -67,6 +68,7 @@ public IpFilter() { addToBlacklist(entry); } this.allowByDefault = config.allowByDefault(); + this.trustProxyHeaders = config.trustProxyHeaders(); } /** @@ -76,6 +78,7 @@ public IpFilter() { */ public void addToWhitelist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; + ipOrCidr = ipOrCidr.trim(); if (ipOrCidr.contains("/")) { try { @@ -97,6 +100,7 @@ public void addToWhitelist(String ipOrCidr) { */ public void addToBlacklist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; + ipOrCidr = ipOrCidr.trim(); if (ipOrCidr.contains("/")) { try { @@ -125,6 +129,7 @@ public void addToBlacklist(String ipOrCidr) { */ public void removeFromWhitelist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; + ipOrCidr = ipOrCidr.trim(); if (ipOrCidr.contains("/")) { whitelistSubnets.remove(ipOrCidr); @@ -147,6 +152,7 @@ public void removeFromWhitelist(String ipOrCidr) { */ public void removeFromBlacklist(String ipOrCidr) { if (ipOrCidr == null || ipOrCidr.isBlank()) return; + ipOrCidr = ipOrCidr.trim(); if (ipOrCidr.contains("/")) { blacklistSubnets.remove(ipOrCidr); @@ -172,18 +178,13 @@ public void removeFromBlacklist(String ipOrCidr) { */ @Override public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throws IOException { - try { - String clientIp = getClientIp(req); - - if (isAllowed(clientIp)) { - chain.doFilter(req, res); - } else { - logger.fine("IP blocked: " + clientIp); - forbidden(res, clientIp); - } - } catch (Exception e) { - logger.severe("Error in IP filter: " + e.getMessage()); - forbidden(res, "error"); + String clientIp = getClientIp(req); + + if (isAllowed(clientIp)) { + chain.doFilter(req, res); + } else { + logger.fine("IP blocked: " + clientIp); + forbidden(res, clientIp); } } @@ -236,12 +237,28 @@ private boolean isInSubnets(String ip, Map subnets) { } /** - * Extracts the client's IP address from the request, considering proxy headers. + * Extracts the client's IP address from the request. + *

+ * Security Note: Proxy headers (X-Forwarded-For, X-Real-IP) can be + * spoofed by clients. Only enable {@code trustProxyHeaders} when deployed behind a + * trusted reverse proxy or load balancer that strips/overwrites these headers. + *

+ * When {@code trustProxyHeaders = true}, checks headers in this order: + *

    + *
  1. {@code X-Forwarded-For} - takes first IP in comma-separated list
    2. + *
  2. {@code X-Real-IP} - single IP value
    3. + *
  3. Direct connection IP from {@code req.remoteIp()}
    4. + *
+ * When {@code trustProxyHeaders = false}, always uses {@code req.remoteIp()}. * * @param req the HTTP request * @return the client's IP address */ private String getClientIp(HttpRequest req) { + if (!trustProxyHeaders) { + return req.remoteIp(); + } + Map headers = req.headers(); String ip = headers.get("X-Forwarded-For"); @@ -318,4 +335,13 @@ public Set getBlacklistSubnets() { public boolean getAllowByDefault() { return allowByDefault; } + + /** + * Returns the default policy for trusting proxy headers. + * + * @return true if proxy headers are trusted, false if not + */ + public boolean getTrustProxyHeaders() { + return trustProxyHeaders; + } } diff --git a/src/test/java/org/juv25d/filter/IpFilterTest.java b/src/test/java/org/juv25d/filter/IpFilterTest.java index f5942b9f..345c7bb0 100644 --- a/src/test/java/org/juv25d/filter/IpFilterTest.java +++ b/src/test/java/org/juv25d/filter/IpFilterTest.java @@ -34,7 +34,7 @@ void setUp() { @Test @DisplayName("Allow ip only in whitelist") void whitelist_allowsIp() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false, false); filter.doFilter(req, res, chain); @@ -45,7 +45,7 @@ void whitelist_allowsIp() throws IOException { @Test @DisplayName("Allow ip from CIDR range only in whitelist") void whitelist_allowsIpInRange() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.0/24"), null, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.0/24"), null, false, false); filter.doFilter(req, res, chain); @@ -56,7 +56,7 @@ void whitelist_allowsIpInRange() throws IOException { @Test @DisplayName("Block ip only in blacklist") void blacklist_blocksIp() throws IOException { - IpFilter filter = new IpFilter(null, Set.of("127.0.0.1"), true); + IpFilter filter = new IpFilter(null, Set.of("127.0.0.1"), true, false); filter.doFilter(req, res, chain); verify(chain, never()).doFilter(req, res); @@ -68,7 +68,7 @@ void blacklist_blocksIp() throws IOException { @Test @DisplayName("Block ip from CIDR range only in blacklist") void blacklist_blocksIpInRange() throws IOException { - IpFilter filter = new IpFilter(null, Set.of("127.0.0.0/24"), true); + IpFilter filter = new IpFilter(null, Set.of("127.0.0.0/24"), true, false); filter.doFilter(req, res, chain); verify(chain, never()).doFilter(req, res); @@ -80,7 +80,7 @@ void blacklist_blocksIpInRange() throws IOException { @Test @DisplayName("Allow ip in both list (whitelist prio)") void whitelist_overrides_blacklist() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.0/24"), false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.0/24"), false, false); filter.doFilter(req, res, chain); @@ -92,7 +92,7 @@ void whitelist_overrides_blacklist() throws IOException { @ValueSource(booleans = {true, false}) @DisplayName("Follow default when ip in neither list") void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException { - IpFilter filter = new IpFilter(null, null, allowByDefault); + IpFilter filter = new IpFilter(null, null, allowByDefault, false); filter.doFilter(req, res, chain); @@ -110,7 +110,7 @@ void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Allow IP or CIDR range added in existing filter") void addIpOrRange_whitelist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(null, null, false); + IpFilter filter = new IpFilter(null, null, false, false); filter.addToWhitelist(ipOrCidr); filter.doFilter(req, res, chain); @@ -125,7 +125,7 @@ void addIpOrRange_whitelist(String ipOrCidr) throws IOException { @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Block IP or CIDR range added in existing filter") void addIpOrRange_blacklist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(null, null, false); + IpFilter filter = new IpFilter(null, null, false, false); filter.addToBlacklist(ipOrCidr); filter.doFilter(req, res, chain); @@ -139,7 +139,7 @@ void addIpOrRange_blacklist(String ipOrCidr) throws IOException { @Test @DisplayName("Adding IP or CIDR range already in filter doesn't create duplicates") void doesNotAddDuplicates() { - IpFilter filter = new IpFilter(Set.of("127.0.0.1", "127.0.0.0/24"), null, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1", "127.0.0.0/24"), null, false, false); filter.addToWhitelist("127.0.0.1"); filter.addToWhitelist("127.0.0.0/24"); @@ -152,7 +152,7 @@ void doesNotAddDuplicates() { @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Fall back on blacklist/default after removing IP or CIDR range from whitelist") void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(Set.of(ipOrCidr), null, false); + IpFilter filter = new IpFilter(Set.of(ipOrCidr), null, false, false); filter.removeFromWhitelist(ipOrCidr); filter.doFilter(req, res, chain); @@ -165,9 +165,9 @@ void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) - @DisplayName("Fall back on whitelist/default after removing IP or CIDR range from whitelist") + @DisplayName("Fall back on whitelist/default after removing IP or CIDR range from blacklist") void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(null, Set.of(ipOrCidr), true); + IpFilter filter = new IpFilter(null, Set.of(ipOrCidr), true, false); filter.removeFromBlacklist(ipOrCidr); filter.doFilter(req, res, chain); @@ -182,7 +182,7 @@ void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { @NullAndEmptySource @DisplayName("Block null or empty IP") void nullOrBlankIp_blocked(String ip) throws IOException { - IpFilter filter = new IpFilter(null, null, true); + IpFilter filter = new IpFilter(null, null, true, false); when(req.remoteIp()).thenReturn(ip); @@ -195,7 +195,7 @@ void nullOrBlankIp_blocked(String ip) throws IOException { @Test @DisplayName("Ignore incorrectly formatted CIDR range") void invalidCidr_loggedAndIgnored() { - IpFilter filter = new IpFilter(null, null, false); + IpFilter filter = new IpFilter(null, null, false, false); filter.addToWhitelist("not-a-cidr/99"); @@ -205,7 +205,7 @@ void invalidCidr_loggedAndIgnored() { @Test @DisplayName("Get methods return immutable copies") void get_returnsImmutableCopy() { - IpFilter filter = new IpFilter(null, null, false); + IpFilter filter = new IpFilter(null, null, false, false); assertThrows(UnsupportedOperationException.class, () -> filter.getWhitelistIps().add("test")); @@ -217,10 +217,23 @@ void get_returnsImmutableCopy() { filter.getBlacklistSubnets().add("test")); } + @Test + @DisplayName("Use clientIp when not trusting proxies") + void ignoreXForwarded_whenTrustProxyHeadersFalse() throws IOException { + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false, false); + + when(req.headers()).thenReturn(Map.of("X-Forwarded-For", "1.2.3.4")); + + filter.doFilter(req, res, chain); + + verify(chain, never()).doFilter(req, res); + assertEquals(403, res.statusCode()); + } + @Test @DisplayName("Evaluates original client IP from X-Forwarded-For when proxied") void xForwardedFor_takesFirstIp() throws IOException { - IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false, true); when(req.headers()).thenReturn(Map.of("X-Forwarded-For", "1.2.3.4, 5.6.7.8")); when(req.remoteIp()).thenReturn("5.6.7.8"); @@ -234,7 +247,7 @@ void xForwardedFor_takesFirstIp() throws IOException { @Test @DisplayName("Uses X-Real-IP header when X-Forwarded-For is absent") void xRealIp_overridesRemoteIp() throws IOException { - IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false, true); when(req.headers()).thenReturn(Map.of("X-Real-IP", "1.2.3.4")); when(req.remoteIp()).thenReturn("5.6.7.8"); @@ -247,7 +260,7 @@ void xRealIp_overridesRemoteIp() throws IOException { @Test @DisplayName("Prioritizes X-Forwarded-For over X-Real-IP when both present") void xForwardedFor_priorityOverXRealIp() throws IOException { - IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false); + IpFilter filter = new IpFilter(Set.of("1.2.3.4"), null, false, true); when(req.headers()).thenReturn(Map.of( "X-Forwarded-For", "1.2.3.4", From d73617aafd9307fbd633192201218ac0d0fc52be Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Thu, 26 Feb 2026 15:07:56 +0100 Subject: [PATCH 5/7] 3 and 4 arg constructor, case insensitive header getting Signed-off-by: Jesper Larsson --- src/main/java/org/juv25d/filter/IpFilter.java | 45 ++++++++++++++++++- .../java/org/juv25d/filter/IpFilterTest.java | 28 ++++++------ 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java index c1a7280b..b621f429 100644 --- a/src/main/java/org/juv25d/filter/IpFilter.java +++ b/src/main/java/org/juv25d/filter/IpFilter.java @@ -33,6 +33,31 @@ public class IpFilter implements Filter { private final boolean allowByDefault; private final boolean trustProxyHeaders; + /** + * Constructs an IP filter with specified whitelist, blacklist, and default policy. + * This constructor sets {@code trustProxyHeaders = true} + *

+ * To specify proxy trusting use {@link #IpFilter(Set, Set, boolean, boolean)} + * + * @param whitelist set of IPs/CIDR ranges to always allow (can be null) + * @param blacklist set of IPs/CIDR ranges to always block (can be null) + * @param allowByDefault whether to allow IPs not in either list + * */ + public IpFilter(@Nullable Set whitelist, @Nullable Set blacklist, boolean allowByDefault) { + if (whitelist != null) { + for (String entry : whitelist) { + addToWhitelist(entry); + } + } + if (blacklist != null) { + for (String entry : blacklist) { + addToBlacklist(entry); + } + } + this.allowByDefault = allowByDefault; + this.trustProxyHeaders = false; + } + /** * Constructs an IP filter with specified whitelist, blacklist, and default policy. * @@ -261,12 +286,12 @@ private String getClientIp(HttpRequest req) { Map headers = req.headers(); - String ip = headers.get("X-Forwarded-For"); + String ip = getHeaderIgnoreCase(headers, "X-Forwarded-For"); if (ip != null && !ip.isBlank()) { return ip.split(",")[0].trim(); } - ip = headers.get("X-Real-IP"); + ip = getHeaderIgnoreCase(headers, "X-Real-IP"); if (ip != null && !ip.isBlank()) { return ip.trim(); } @@ -274,6 +299,22 @@ private String getClientIp(HttpRequest req) { return req.remoteIp(); } + /** + * Retrieves a header value using case-insensitive name matching. + * + * @param headers the header map to search + * @param name the header name + * @return the header value, or {@code null} if not found + */ + private @Nullable String getHeaderIgnoreCase(Map headers, String name) { + for (Map.Entry entry : headers.entrySet()) { + if (entry.getKey() != null && entry.getKey().equalsIgnoreCase(name)) { + return entry.getValue(); + } + } + return null; + } + /** * Sends a 403 Forbidden response to the client. * diff --git a/src/test/java/org/juv25d/filter/IpFilterTest.java b/src/test/java/org/juv25d/filter/IpFilterTest.java index 345c7bb0..f160515b 100644 --- a/src/test/java/org/juv25d/filter/IpFilterTest.java +++ b/src/test/java/org/juv25d/filter/IpFilterTest.java @@ -34,7 +34,7 @@ void setUp() { @Test @DisplayName("Allow ip only in whitelist") void whitelist_allowsIp() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false); filter.doFilter(req, res, chain); @@ -45,7 +45,7 @@ void whitelist_allowsIp() throws IOException { @Test @DisplayName("Allow ip from CIDR range only in whitelist") void whitelist_allowsIpInRange() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.0/24"), null, false, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.0/24"), null, false); filter.doFilter(req, res, chain); @@ -56,7 +56,7 @@ void whitelist_allowsIpInRange() throws IOException { @Test @DisplayName("Block ip only in blacklist") void blacklist_blocksIp() throws IOException { - IpFilter filter = new IpFilter(null, Set.of("127.0.0.1"), true, false); + IpFilter filter = new IpFilter(null, Set.of("127.0.0.1"), true); filter.doFilter(req, res, chain); verify(chain, never()).doFilter(req, res); @@ -68,7 +68,7 @@ void blacklist_blocksIp() throws IOException { @Test @DisplayName("Block ip from CIDR range only in blacklist") void blacklist_blocksIpInRange() throws IOException { - IpFilter filter = new IpFilter(null, Set.of("127.0.0.0/24"), true, false); + IpFilter filter = new IpFilter(null, Set.of("127.0.0.0/24"), true); filter.doFilter(req, res, chain); verify(chain, never()).doFilter(req, res); @@ -80,7 +80,7 @@ void blacklist_blocksIpInRange() throws IOException { @Test @DisplayName("Allow ip in both list (whitelist prio)") void whitelist_overrides_blacklist() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.0/24"), false, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1"), Set.of("127.0.0.0/24"), false); filter.doFilter(req, res, chain); @@ -92,7 +92,7 @@ void whitelist_overrides_blacklist() throws IOException { @ValueSource(booleans = {true, false}) @DisplayName("Follow default when ip in neither list") void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException { - IpFilter filter = new IpFilter(null, null, allowByDefault, false); + IpFilter filter = new IpFilter(null, null, allowByDefault); filter.doFilter(req, res, chain); @@ -110,7 +110,7 @@ void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Allow IP or CIDR range added in existing filter") void addIpOrRange_whitelist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(null, null, false, false); + IpFilter filter = new IpFilter(null, null, false); filter.addToWhitelist(ipOrCidr); filter.doFilter(req, res, chain); @@ -125,7 +125,7 @@ void addIpOrRange_whitelist(String ipOrCidr) throws IOException { @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Block IP or CIDR range added in existing filter") void addIpOrRange_blacklist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(null, null, false, false); + IpFilter filter = new IpFilter(null, null, false); filter.addToBlacklist(ipOrCidr); filter.doFilter(req, res, chain); @@ -139,7 +139,7 @@ void addIpOrRange_blacklist(String ipOrCidr) throws IOException { @Test @DisplayName("Adding IP or CIDR range already in filter doesn't create duplicates") void doesNotAddDuplicates() { - IpFilter filter = new IpFilter(Set.of("127.0.0.1", "127.0.0.0/24"), null, false, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1", "127.0.0.0/24"), null, false); filter.addToWhitelist("127.0.0.1"); filter.addToWhitelist("127.0.0.0/24"); @@ -152,7 +152,7 @@ void doesNotAddDuplicates() { @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Fall back on blacklist/default after removing IP or CIDR range from whitelist") void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(Set.of(ipOrCidr), null, false, false); + IpFilter filter = new IpFilter(Set.of(ipOrCidr), null, false); filter.removeFromWhitelist(ipOrCidr); filter.doFilter(req, res, chain); @@ -167,7 +167,7 @@ void removeIpOrRange_whitelist(String ipOrCidr) throws IOException { @ValueSource(strings = {"127.0.0.1", "127.0.0.0/24"}) @DisplayName("Fall back on whitelist/default after removing IP or CIDR range from blacklist") void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { - IpFilter filter = new IpFilter(null, Set.of(ipOrCidr), true, false); + IpFilter filter = new IpFilter(null, Set.of(ipOrCidr), true); filter.removeFromBlacklist(ipOrCidr); filter.doFilter(req, res, chain); @@ -182,7 +182,7 @@ void removeIpOrRange_blacklist(String ipOrCidr) throws IOException { @NullAndEmptySource @DisplayName("Block null or empty IP") void nullOrBlankIp_blocked(String ip) throws IOException { - IpFilter filter = new IpFilter(null, null, true, false); + IpFilter filter = new IpFilter(null, null, true); when(req.remoteIp()).thenReturn(ip); @@ -195,7 +195,7 @@ void nullOrBlankIp_blocked(String ip) throws IOException { @Test @DisplayName("Ignore incorrectly formatted CIDR range") void invalidCidr_loggedAndIgnored() { - IpFilter filter = new IpFilter(null, null, false, false); + IpFilter filter = new IpFilter(null, null, false); filter.addToWhitelist("not-a-cidr/99"); @@ -205,7 +205,7 @@ void invalidCidr_loggedAndIgnored() { @Test @DisplayName("Get methods return immutable copies") void get_returnsImmutableCopy() { - IpFilter filter = new IpFilter(null, null, false, false); + IpFilter filter = new IpFilter(null, null, false); assertThrows(UnsupportedOperationException.class, () -> filter.getWhitelistIps().add("test")); From 06707975c11a7b4ce17e3284e84f44991d3de36e Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Sat, 28 Feb 2026 00:08:12 +0100 Subject: [PATCH 6/7] Fix error in javadoc Signed-off-by: Jesper Larsson --- src/main/java/org/juv25d/filter/IpFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java index b621f429..0a46a734 100644 --- a/src/main/java/org/juv25d/filter/IpFilter.java +++ b/src/main/java/org/juv25d/filter/IpFilter.java @@ -35,7 +35,7 @@ public class IpFilter implements Filter { /** * Constructs an IP filter with specified whitelist, blacklist, and default policy. - * This constructor sets {@code trustProxyHeaders = true} + * This constructor sets {@code trustProxyHeaders = false} *

* To specify proxy trusting use {@link #IpFilter(Set, Set, boolean, boolean)} * From f6f040ac817e5956dfc970492599b493ffa396b4 Mon Sep 17 00:00:00 2001 From: Jesper Larsson Date: Sat, 28 Feb 2026 00:15:27 +0100 Subject: [PATCH 7/7] rabbit nitpick Signed-off-by: Jesper Larsson --- src/main/java/org/juv25d/config/IpFilterConfig.java | 4 +++- src/main/java/org/juv25d/filter/IpFilter.java | 13 +------------ src/test/java/org/juv25d/filter/IpFilterTest.java | 4 ++-- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/juv25d/config/IpFilterConfig.java b/src/main/java/org/juv25d/config/IpFilterConfig.java index ec970cdd..bc69d936 100644 --- a/src/main/java/org/juv25d/config/IpFilterConfig.java +++ b/src/main/java/org/juv25d/config/IpFilterConfig.java @@ -20,5 +20,7 @@ public boolean allowByDefault() { return allowByDefault; } - public boolean trustProxyHeaders() {return trustProxyHeaders;} + public boolean trustProxyHeaders() { + return trustProxyHeaders; + } } diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java index 0a46a734..9c0ff43d 100644 --- a/src/main/java/org/juv25d/filter/IpFilter.java +++ b/src/main/java/org/juv25d/filter/IpFilter.java @@ -44,18 +44,7 @@ public class IpFilter implements Filter { * @param allowByDefault whether to allow IPs not in either list * */ public IpFilter(@Nullable Set whitelist, @Nullable Set blacklist, boolean allowByDefault) { - if (whitelist != null) { - for (String entry : whitelist) { - addToWhitelist(entry); - } - } - if (blacklist != null) { - for (String entry : blacklist) { - addToBlacklist(entry); - } - } - this.allowByDefault = allowByDefault; - this.trustProxyHeaders = false; + this(whitelist, blacklist, allowByDefault, false); } /** diff --git a/src/test/java/org/juv25d/filter/IpFilterTest.java b/src/test/java/org/juv25d/filter/IpFilterTest.java index 87bb7b52..628f50db 100644 --- a/src/test/java/org/juv25d/filter/IpFilterTest.java +++ b/src/test/java/org/juv25d/filter/IpFilterTest.java @@ -34,7 +34,7 @@ void setUp() { @Test @DisplayName("Allow ip only in whitelist") void whitelist_allowsIp() throws IOException { - IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false, false); + IpFilter filter = new IpFilter(Set.of("127.0.0.1"), null, false); filter.doFilter(req, res, chain); @@ -91,7 +91,7 @@ void whitelist_overrides_blacklist() throws IOException { @ParameterizedTest @ValueSource(booleans = {true, false}) @DisplayName("Follow default when ip in neither list") - void Ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException { + void ip_inNeitherList_followsDefault(boolean allowByDefault) throws IOException { IpFilter filter = new IpFilter(null, null, allowByDefault); filter.doFilter(req, res, chain);