From bb7e48043d52cbe4e50568635cd76a7995b6b414 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 25 Feb 2020 14:58:48 +1100 Subject: [PATCH] Issue #4598 - Add URI mapping to InetAccessHandler Signed-off-by: Lachlan Roberts --- .../jetty/server/handler/IPAccessHandler.java | 12 +- .../server/handler/InetAccessHandler.java | 140 +++++++++++++++--- .../server/handler/InetAccessHandlerTest.java | 128 +++++++++------- .../eclipse/jetty/util/IncludeExcludeSet.java | 11 +- 4 files changed, 201 insertions(+), 90 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java index 869f3636829..18f76297f6e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/IPAccessHandler.java @@ -31,6 +31,7 @@ import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.IPAddressMap; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -107,8 +108,8 @@ public class IPAccessHandler extends HandlerWrapper { private static final Logger LOG = Log.getLogger(IPAccessHandler.class); // true means nodefault match - PathMap> _white = new PathMap>(true); - PathMap> _black = new PathMap>(true); + PathMap> _white = new PathMap<>(true); + PathMap> _black = new PathMap<>(true); boolean _whiteListByPath = false; /** @@ -241,17 +242,16 @@ public class IPAccessHandler extends HandlerWrapper if (addr.endsWith(".")) deprecated = true; - if (path != null && (path.startsWith("|") || path.startsWith("/*."))) + if (path.startsWith("|") || path.startsWith("/*.")) path = path.substring(1); IPAddressMap addrMap = patternMap.get(path); if (addrMap == null) { - addrMap = new IPAddressMap(); + addrMap = new IPAddressMap<>(); patternMap.put(path, addrMap); } - if (addr != null && !"".equals(addr)) - // MUST NOT BE null + if (!StringUtil.isEmpty(addr)) addrMap.put(addr, true); if (deprecated) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java index d71fad8b559..4bc203c9d70 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java @@ -21,12 +21,17 @@ package org.eclipse.jetty.server.handler; import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.util.List; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.PathMappings; +import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.IncludeExclude; @@ -54,7 +59,8 @@ public class InetAccessHandler extends HandlerWrapper private static final Logger LOG = Log.getLogger(InetAccessHandler.class); private final IncludeExcludeSet _addrs = new IncludeExcludeSet<>(InetAddressSet.class); - private final IncludeExclude _names = new IncludeExclude<>(); + private final PathMappings> _pathMappings = new PathMappings<>(); + private final IncludeExclude _connectorNames = new IncludeExclude<>(); /** * Clears all the includes, excludes, included connector names and excluded @@ -63,18 +69,33 @@ public class InetAccessHandler extends HandlerWrapper public void clear() { _addrs.clear(); - _names.clear(); + _connectorNames.clear(); + _pathMappings.reset(); } /** - * Includes an InetAddress pattern + * Includes an InetAddress pattern with an optional URI mapping for the pattern. + * + * The InetAddress pattern is separated from the URI pattern using the "|" (pipe) + * character. URI patterns follow the servlet specification for simple * prefix and + * suffix wild cards (e.g. /, /foo, /foo/bar, /foo/bar/*, *.baz). * * @param pattern InetAddress pattern to include * @see InetAddressSet */ public void include(String pattern) { - _addrs.include(pattern); + int index = pattern.indexOf('|'); + if (index > 0) + { + String addr = pattern.substring(0, index); + String path = pattern.substring(index + 1); + ensureMapping(path).include(addr); + } + else + { + _addrs.include(pattern); + } } /** @@ -89,14 +110,28 @@ public class InetAccessHandler extends HandlerWrapper } /** - * Excludes an InetAddress pattern + * Excludes an InetAddress pattern with an optional URI mapping for the pattern. + + * The InetAddress pattern is separated from the URI pattern using the "|" (pipe) + * character. URI patterns follow the servlet specification for simple * prefix and + * suffix wild cards (e.g. /, /foo, /foo/bar, /foo/bar/*, *.baz). * * @param pattern InetAddress pattern to exclude * @see InetAddressSet */ public void exclude(String pattern) { - _addrs.exclude(pattern); + int index = pattern.indexOf('|'); + if (index > 0) + { + String addr = pattern.substring(0, index); + String path = pattern.substring(index + 1); + ensureMapping(path).exclude(addr); + } + else + { + _addrs.exclude(pattern); + } } /** @@ -117,7 +152,7 @@ public class InetAccessHandler extends HandlerWrapper */ public void includeConnector(String name) { - _names.include(name); + _connectorNames.include(name); } /** @@ -127,7 +162,7 @@ public class InetAccessHandler extends HandlerWrapper */ public void excludeConnector(String name) { - _names.exclude(name); + _connectorNames.exclude(name); } /** @@ -137,7 +172,7 @@ public class InetAccessHandler extends HandlerWrapper */ public void includeConnectors(String... names) { - _names.include(names); + _connectorNames.include(names); } /** @@ -147,7 +182,7 @@ public class InetAccessHandler extends HandlerWrapper */ public void excludeConnectors(String... names) { - _names.exclude(names); + _connectorNames.exclude(names); } /** @@ -187,17 +222,69 @@ public class InetAccessHandler extends HandlerWrapper */ protected boolean isAllowed(InetAddress addr, Request baseRequest, HttpServletRequest request) { - String name = baseRequest.getHttpChannel().getConnector().getName(); - boolean filterAppliesToConnector = _names.test(name); - boolean allowedByAddr = _addrs.test(addr); + // If this connector is not in the subset of connectors to apply this filter to, the request is simply allowed through. + Connector connector = baseRequest.getHttpChannel().getConnector(); + String connectorName = connector.getName(); + boolean filterAppliesToConnector = _connectorNames.test(connectorName); + if (!filterAppliesToConnector) + { + if (LOG.isDebugEnabled()) + LOG.debug("Handler does not apply to connector {}", connector); + return true; + } + + MappingResult allowedByPath = matchMappings(baseRequest.getPathInfo(), addr); + Boolean allowedByAddr = _addrs.isIncludedAndNotExcluded(addr); if (LOG.isDebugEnabled()) { - LOG.debug("name = {}/{} addr={}/{} appliesToConnector={} allowedByAddr={}", - name, _names, addr, _addrs, filterAppliesToConnector, allowedByAddr); + LOG.debug("connectorName = {}/{} addr={}/{} allowedByAddr={} allowedByPath={}", + connectorName, _connectorNames, addr, _addrs, allowedByAddr, allowedByPath); } - if (!filterAppliesToConnector) - return true; - return allowedByAddr; + + // Not allowed if it was specifically excluded anywhere. + if (Boolean.FALSE.equals(allowedByAddr) || MappingResult.FALSE.equals(allowedByPath)) + return false; + + // If either set has any includes, then we must be included by one of them. + if (_addrs.hasIncludes() || allowedByPath.hasIncludes()) + return Boolean.TRUE.equals(allowedByAddr) || MappingResult.TRUE.equals(allowedByPath); + + return true; + } + + private MappingResult matchMappings(String path, InetAddress addr) + { + List>> matches = _pathMappings.getMatches(path); + + boolean hasIncludes = false; + boolean isIncluded = false; + for (MappedResource> resource : matches) + { + IncludeExcludeSet set = resource.getResource(); + if (set.hasIncludes()) + hasIncludes = true; + + Boolean b = set.isIncludedAndNotExcluded(addr); + if (Boolean.FALSE.equals(b)) + return MappingResult.FALSE; + + if (Boolean.TRUE.equals(b)) + isIncluded = true; + } + + return isIncluded ? MappingResult.TRUE : (hasIncludes ? MappingResult.HAS_INCLUDES : MappingResult.NO_INCLUDES); + } + + private IncludeExcludeSet ensureMapping(String path) + { + ServletPathSpec pathSpec = new ServletPathSpec(path); + IncludeExcludeSet names = _pathMappings.get(pathSpec); + if (names == null) + { + names = new IncludeExcludeSet<>(InetAddressSet.class); + _pathMappings.put(pathSpec, names); + } + return names; } @Override @@ -206,7 +293,20 @@ public class InetAccessHandler extends HandlerWrapper dumpObjects(out, indent, new DumpableCollection("included", _addrs.getIncluded()), new DumpableCollection("excluded", _addrs.getExcluded()), - new DumpableCollection("includedConnector", _names.getIncluded()), - new DumpableCollection("excludedConnector", _names.getExcluded())); + new DumpableCollection("includedConnector", _connectorNames.getIncluded()), + new DumpableCollection("excludedConnector", _connectorNames.getExcluded())); + } + + private enum MappingResult + { + TRUE, + FALSE, + NO_INCLUDES, + HAS_INCLUDES; + + boolean hasIncludes() + { + return TRUE.equals(this) || HAS_INCLUDES.equals(this); + } } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java index 6b6da0a2104..d6b28fef601 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java @@ -25,7 +25,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.stream.Stream; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -36,6 +35,7 @@ import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.params.ParameterizedTest; @@ -67,7 +67,6 @@ public class InetAccessHandlerTest { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) - throws IOException, ServletException { baseRequest.setHandled(true); response.setStatus(HttpStatus.OK_200); @@ -85,7 +84,7 @@ public class InetAccessHandlerTest @ParameterizedTest @MethodSource("data") - public void testHandler(String include, String exclude, String includeConnectors, String excludeConnectors, String code) + public void testHandler(String path, String include, String exclude, String includeConnectors, String excludeConnectors, String code) throws Exception { _handler.clear(); @@ -127,11 +126,11 @@ public class InetAccessHandlerTest } } - testConnector(_connector1.getLocalPort(), include, exclude, includeConnectors, excludeConnectors, codePerConnector.get(0)); - testConnector(_connector2.getLocalPort(), include, exclude, includeConnectors, excludeConnectors, codePerConnector.get(1)); + testConnector(_connector1.getLocalPort(), path, include, exclude, includeConnectors, excludeConnectors, codePerConnector.get(0)); + testConnector(_connector2.getLocalPort(), path, include, exclude, includeConnectors, excludeConnectors, codePerConnector.get(1)); } - private void testConnector(int port, String include, String exclude, String includeConnectors, String excludeConnectors, String code) throws IOException + private void testConnector(int port, String path, String include, String exclude, String includeConnectors, String excludeConnectors, String code) throws IOException { try (Socket socket = new Socket("127.0.0.1", port);) { @@ -139,7 +138,7 @@ public class InetAccessHandlerTest HttpTester.Request request = HttpTester.newRequest(); request.setMethod("GET"); - request.setURI("/path"); + request.setURI(StringUtil.isEmpty(path) ? "/" : path); request.setHeader("Host", "127.0.0.1"); request.setVersion(HttpVersion.HTTP_1_0); @@ -164,62 +163,79 @@ public class InetAccessHandlerTest public static Stream data() { Object[][] data = new Object[][] - { - // Empty lists 1 - {"", "", "", "", "200;200"}, + { + // Empty lists 1 + {"", "", "", "", "", "200;200"}, - // test simple filters - {"127.0.0.1", "", "", "", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "", "", "200;200"}, - {"192.0.0.1", "", "", "", "403;403"}, - {"192.0.0.1-192.0.0.254", "", "", "", "403;403"}, + // test simple filters + {"", "127.0.0.1", "", "", "", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "", "", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "", "", "200;200"}, + {"", "192.0.0.1", "", "", "", "403;403"}, + {"", "192.0.0.1-192.0.0.254", "", "", "", "403;403"}, - // test includeConnector - {"127.0.0.1", "", "http_connector1", "", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "http_connector1", "", "200;200"}, - {"192.0.0.1", "", "http_connector1", "", "403;200"}, - {"192.0.0.1-192.0.0.254", "", "http_connector1", "", "403;200"}, - {"192.0.0.1", "", "http_connector2", "", "200;403"}, - {"192.0.0.1-192.0.0.254", "", "http_connector2", "", "200;403"}, + // test includeConnector + {"", "127.0.0.1", "", "http_connector1", "", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "http_connector1", "", "200;200"}, + {"", "192.0.0.1", "", "http_connector1", "", "403;200"}, + {"", "192.0.0.1-192.0.0.254", "", "http_connector1", "", "403;200"}, + {"", "192.0.0.1", "", "http_connector2", "", "200;403"}, + {"", "192.0.0.1-192.0.0.254", "", "http_connector2", "", "200;403"}, - // test includeConnector names where none of them match - {"127.0.0.1", "", "nothttp", "", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "nothttp", "", "200;200"}, - {"192.0.0.1", "", "nothttp", "", "200;200"}, - {"192.0.0.1-192.0.0.254", "", "nothttp", "", "200;200"}, + // test includeConnector names where none of them match + {"", "127.0.0.1", "", "nothttp", "", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "nothttp", "", "200;200"}, + {"", "192.0.0.1", "", "nothttp", "", "200;200"}, + {"", "192.0.0.1-192.0.0.254", "", "nothttp", "", "200;200"}, - // text excludeConnector - {"127.0.0.1", "", "", "http_connector1", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "", "http_connector1", "200;200"}, - {"192.0.0.1", "", "", "http_connector1", "200;403"}, - {"192.0.0.1-192.0.0.254", "", "", "http_connector1", "200;403"}, - {"192.0.0.1", "", "", "http_connector2", "403;200"}, - {"192.0.0.1-192.0.0.254", "", "", "http_connector2", "403;200"}, + // text excludeConnector + {"", "127.0.0.1", "", "", "http_connector1", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "", "http_connector1", "200;200"}, + {"", "192.0.0.1", "", "", "http_connector1", "200;403"}, + {"", "192.0.0.1-192.0.0.254", "", "", "http_connector1", "200;403"}, + {"", "192.0.0.1", "", "", "http_connector2", "403;200"}, + {"", "192.0.0.1-192.0.0.254", "", "", "http_connector2", "403;200"}, - // test excludeConnector where none of them match. - {"127.0.0.1", "", "", "nothttp", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "", "nothttp", "200;200"}, - {"192.0.0.1", "", "", "nothttp", "403;403"}, - {"192.0.0.1-192.0.0.254", "", "", "nothttp", "403;403"}, + // test excludeConnector where none of them match. + {"", "127.0.0.1", "", "", "nothttp", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "", "nothttp", "200;200"}, + {"", "192.0.0.1", "", "", "nothttp", "403;403"}, + {"", "192.0.0.1-192.0.0.254", "", "", "nothttp", "403;403"}, - // both connectors are excluded - {"127.0.0.1", "", "", "http_connector1;http_connector2", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "", "http_connector1;http_connector2", "200;200"}, - {"192.0.0.1", "", "", "http_connector1;http_connector2", "200;200"}, - {"192.0.0.1-192.0.0.254", "", "", "http_connector1;http_connector2", "200;200"}, + // both connectors are excluded + {"", "127.0.0.1", "", "", "http_connector1;http_connector2", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "", "http_connector1;http_connector2", "200;200"}, + {"", "192.0.0.1", "", "", "http_connector1;http_connector2", "200;200"}, + {"", "192.0.0.1-192.0.0.254", "", "", "http_connector1;http_connector2", "200;200"}, - // both connectors are included - {"127.0.0.1", "", "http_connector1;http_connector2", "", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "http_connector1;http_connector2", "", "200;200"}, - {"192.0.0.1", "", "http_connector1;http_connector2", "", "403;403"}, - {"192.0.0.1-192.0.0.254", "", "http_connector1;http_connector2", "", "403;403"}, + // both connectors are included + {"", "127.0.0.1", "", "http_connector1;http_connector2", "", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "http_connector1;http_connector2", "", "200;200"}, + {"", "192.0.0.1", "", "http_connector1;http_connector2", "", "403;403"}, + {"", "192.0.0.1-192.0.0.254", "", "http_connector1;http_connector2", "", "403;403"}, - // exclude takes precedence over include - {"127.0.0.1", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, - {"127.0.0.1-127.0.0.254", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, - {"192.0.0.1", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, - {"192.0.0.1-192.0.0.254", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"} - }; - return Arrays.asList(data).stream().map(Arguments::of); + // exclude takes precedence over include + {"", "127.0.0.1", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, + {"", "127.0.0.1-127.0.0.254", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, + {"", "192.0.0.1", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, + {"", "192.0.0.1-192.0.0.254", "", "http_connector1;http_connector2", "http_connector1;http_connector2", "200;200"}, + + // Test path specific include/exclude. + {"/testPath", "", "", "http_connector1", "", "200;200"}, + {"/", "127.0.0.1", "127.0.0.1|/testPath", "http_connector1", "", "200;200"}, + {"/testPath", "127.0.0.1", "127.0.0.1|/testPath", "http_connector1", "", "403;200"}, + {"/notTestPath", "127.0.1.11|/testPath", "", "http_connector1", "", "200;200"}, + {"/testPath", "127.0.1.11|/testPath", "", "http_connector1", "", "403;200"}, + {"/testPath", "127.0.0.13|/testPath;127.0.0.1|/testPath", "", "http_connector1", "", "200;200"}, + {"/testPath", "127.0.0.1", "127.0.0.1|/testPath", "http_connector1", "", "403;200"}, + {"/a/b", "", "127.0.0.1|/a/*", "http_connector1", "", "403;200"}, + {"/b/a", "", "127.0.0.1|/a/*", "http_connector1", "", "200;200"}, + {"/org/eclipse/jetty/test.html", "127.0.0.1|*.html", "127.0.0.1|*.xml", "", "http_connector2", "200;200"}, + {"/org/eclipse/jetty/test.xml", "127.0.0.1|*.html", "127.0.0.1|*.xml", "", "http_connector2", "403;200"}, + {"/a/test.html", "", "127.0.0.1|*.html;127.0.0.10|/a/*", "http_connector1", "", "403;200"}, + {"/foo/bar/test.xml", "127.0.0.1|/foo/*", "127.0.0.0-127.0.0.2|*.html", "http_connector1", "", "200;200"}, + {"/foo/bar/test.html", "127.0.0.1|/foo/*", "127.0.0.0-127.0.0.2|*.html", "http_connector1", "", "403;200"}, + }; + return Arrays.stream(data).map(Arguments::of); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java b/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java index d88a9e661bf..85768af3d7c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.util; +import java.util.Arrays; import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -145,10 +146,7 @@ public class IncludeExcludeSet implements Predicate

public void include(T... element) { - for (T e : element) - { - _includes.add(e); - } + _includes.addAll(Arrays.asList(element)); } public void exclude(T element) @@ -158,10 +156,7 @@ public class IncludeExcludeSet implements Predicate

public void exclude(T... element) { - for (T e : element) - { - _excludes.add(e); - } + _excludes.addAll(Arrays.asList(element)); } @Deprecated