From e12d5d58b6cf4ed8243dd903c0867001d327493c Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 26 Apr 2022 19:04:17 +0200 Subject: [PATCH] Jetty 10 7918 root pathspec (#7920) Fix #7918 Root path spec Handle root pathspec in PathMappings.asPathSpec Introduce protected asPathSpec to allow for extensibility Signed-off-by: Greg Wilkins --- .../jetty/http/pathmap/PathMappings.java | 19 ++++------ .../jetty/http/pathmap/PathMappingsTest.java | 15 ++++++++ .../security/ConstraintSecurityHandler.java | 10 ++++- .../jetty/security/ConstraintTest.java | 38 +++++++++++++++++++ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java index 1f1b3ecd867..ba02c8a0ae7 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java @@ -18,7 +18,6 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.Iterator; import java.util.List; -import java.util.Optional; import java.util.Set; import java.util.TreeSet; import java.util.function.Predicate; @@ -199,23 +198,21 @@ public class PathMappings implements Iterable>, Dumpable public static PathSpec asPathSpec(String pathSpecString) { - if ((pathSpecString == null) || (pathSpecString.length() < 1)) - { + if (pathSpecString == null) throw new RuntimeException("Path Spec String must start with '^', '/', or '*.': got [" + pathSpecString + "]"); - } + + if (pathSpecString.length() == 0) + return new ServletPathSpec(""); + return pathSpecString.charAt(0) == '^' ? new RegexPathSpec(pathSpecString) : new ServletPathSpec(pathSpecString); } public E get(PathSpec spec) { - Optional optionalResource = _mappings.stream() + return _mappings.stream() .filter(mappedResource -> mappedResource.getPathSpec().equals(spec)) - .map(mappedResource -> mappedResource.getResource()) - .findFirst(); - if (!optionalResource.isPresent()) - return null; - - return optionalResource.get(); + .map(MappedResource::getResource) + .findFirst().orElse(null); } public boolean put(String pathSpecString, E resource) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java index 11d0568572c..00b5da8e920 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java @@ -20,6 +20,7 @@ import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -456,4 +457,18 @@ public class PathMappingsTest assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(true)); assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(false)); } + + @Test + public void testAsPathSpec() + { + assertThat(PathMappings.asPathSpec(""), instanceOf(ServletPathSpec.class)); + assertThat(PathMappings.asPathSpec("/"), instanceOf(ServletPathSpec.class)); + assertThat(PathMappings.asPathSpec("/*"), instanceOf(ServletPathSpec.class)); + assertThat(PathMappings.asPathSpec("/foo/*"), instanceOf(ServletPathSpec.class)); + assertThat(PathMappings.asPathSpec("*.jsp"), instanceOf(ServletPathSpec.class)); + + assertThat(PathMappings.asPathSpec("^$"), instanceOf(RegexPathSpec.class)); + assertThat(PathMappings.asPathSpec("^.*"), instanceOf(RegexPathSpec.class)); + assertThat(PathMappings.asPathSpec("^/"), instanceOf(RegexPathSpec.class)); + } } diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java index c395880b8ab..34c0d7fee7a 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java @@ -35,6 +35,7 @@ import javax.servlet.annotation.ServletSecurity.TransportGuarantee; 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.PathSpec; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; @@ -422,7 +423,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr */ protected void processConstraintMapping(ConstraintMapping mapping) { - Map mappings = _constraintRoles.get(PathMappings.asPathSpec(mapping.getPathSpec())); + Map mappings = _constraintRoles.get(asPathSpec(mapping)); if (mappings == null) { mappings = new HashMap<>(); @@ -467,6 +468,13 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr } } + protected PathSpec asPathSpec(ConstraintMapping mapping) + { + // As currently written, this allows regex patterns to be used. + // This may not be supported by default in future releases. + return PathMappings.asPathSpec(mapping.getPathSpec()); + } + /** * Constraints that name method omissions are dealt with differently. * We create an entry in the mappings with key "<method>.omission". This entry diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java index 72b5404fe4e..3d802d77a1d 100644 --- a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java +++ b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java @@ -1869,6 +1869,44 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 403 ")); } + @Test + public void testDefaultConstraint() throws Exception + { + _security.setAuthenticator(new BasicAuthenticator()); + + ConstraintMapping forbidDefault = new ConstraintMapping(); + forbidDefault.setPathSpec("/"); + forbidDefault.setConstraint(_forbidConstraint); + _security.addConstraintMapping(forbidDefault); + + ConstraintMapping allowRoot = new ConstraintMapping(); + allowRoot.setPathSpec(""); + allowRoot.setConstraint(_relaxConstraint); + _security.addConstraintMapping(allowRoot); + + _server.start(); + String response; + + response = _connector.getResponse("GET /ctx/ HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 200 OK")); + + response = _connector.getResponse("GET /ctx/anything HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 403 Forbidden")); + + response = _connector.getResponse("GET /ctx/noauth/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 403 Forbidden")); + + response = _connector.getResponse("GET /ctx/forbid/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 403 Forbidden")); + + response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 401 Unauthorized")); + assertThat(response, containsString("WWW-Authenticate: basic realm=\"TestRealm\"")); + + response = _connector.getResponse("GET /ctx/admin/relax/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 200 OK")); + } + private static String authBase64(String authorization) { byte[] raw = authorization.getBytes(ISO_8859_1);