diff --git a/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatches.java b/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatches.java index 247544e53..fad74b8d4 100644 --- a/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatches.java +++ b/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatches.java @@ -230,7 +230,7 @@ private ServletPathMatchesData setupServletChains() { //now loop through all servlets. for (Map.Entry entry : servlets.getServletHandlers().entrySet()) { final ServletHandler handler = entry.getValue(); - //add the servlet to the approprite path maps + //add the servlet to the appropriate path maps for (String path : handler.getManagedServlet().getServletInfo().getMappings()) { if (path.equals("/")) { //the default servlet @@ -282,7 +282,7 @@ private ServletPathMatchesData setupServletChains() { final Map> noExtension = new EnumMap<>(DispatcherType.class); final Map>> extension = new HashMap<>(); - //initalize the extension map. This contains all the filers in the noExtension map, plus + //initialize the extension map. This contains all the filers in the noExtension map, plus //any filters that match the extension key for (String ext : extensionMatches) { extension.put(ext, new EnumMap>(DispatcherType.class)); @@ -332,16 +332,36 @@ private ServletPathMatchesData setupServletChains() { ServletHandler pathServlet = targetServletMatch.handler; String pathMatch = targetServletMatch.matchedPath; - boolean defaultServletMatch = targetServletMatch.defaultServlet; - if (defaultServletMatch && extensionServlets.containsKey(entry.getKey())) { + final boolean defaultServletMatch; + final String servletMatchPattern; + final MappingMatch mappingMatch; + if (targetServletMatch.defaultServlet) { + // Path matches always take precedence over extension matches, however the default servlet is matched + // at a lower priority, after extension matches. The "/*" pattern is applied implicitly onto the + // default servlet. If there's an extension match in addition to a non-default servlet path match, + // the servlet path match is higher priority. However if the path match is the default servlets + // default catch-all path, the extension match is a higher priority. + ServletHandler extensionServletHandler = extensionServlets.get(entry.getKey()); + if (extensionServletHandler != null) { + defaultServletMatch = false; + pathServlet = extensionServletHandler; + servletMatchPattern = "*." + entry.getKey(); + mappingMatch = MappingMatch.EXTENSION; + } else { + defaultServletMatch = true; + servletMatchPattern = "/"; + mappingMatch = MappingMatch.DEFAULT; + } + } else { defaultServletMatch = false; - pathServlet = extensionServlets.get(entry.getKey()); + servletMatchPattern = path; + mappingMatch = MappingMatch.PATH; } HttpHandler handler = pathServlet; if (!entry.getValue().isEmpty()) { handler = new FilterHandler(entry.getValue(), deploymentInfo.isAllowNonStandardWrappers(), handler); } - builder.addExtensionMatch(prefix, entry.getKey(), servletChain(handler, pathServlet.getManagedServlet(), entry.getValue(), pathMatch, deploymentInfo, defaultServletMatch, defaultServletMatch ? MappingMatch.DEFAULT : MappingMatch.EXTENSION, defaultServletMatch ? "/" : "*." + entry.getKey())); + builder.addExtensionMatch(prefix, entry.getKey(), servletChain(handler, pathServlet.getManagedServlet(), entry.getValue(), pathMatch, deploymentInfo, defaultServletMatch, mappingMatch, servletMatchPattern)); } } else if (path.isEmpty()) { //the context root match diff --git a/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatchesData.java b/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatchesData.java index 7e4a8aeec..86fd229d1 100644 --- a/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatchesData.java +++ b/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatchesData.java @@ -79,19 +79,15 @@ public ServletPathMatch getServletHandlerByPath(final String path) { } } //this should never happen - //as the default servlet is aways registered under /* + //as the default servlet is always registered under /* throw UndertowMessages.MESSAGES.servletPathMatchFailed(); } private ServletPathMatch handleMatch(final String path, final PathMatch match, final int extensionPos) { - if (match.extensionMatches.isEmpty()) { + if (extensionPos == -1 || match.extensionMatches.isEmpty()) { return new ServletPathMatch(match.defaultHandler, path, match.requireWelcomeFileMatch); } - if (extensionPos == -1) { - return new ServletPathMatch(match.defaultHandler, path, match.requireWelcomeFileMatch); - } - final String ext; - ext = path.substring(extensionPos + 1, path.length()); + final String ext = path.substring(extensionPos + 1); ServletChain handler = match.extensionMatches.get(ext); if (handler != null) { return new ServletPathMatch(handler, path, handler.getManagedServlet().getServletInfo().isRequireWelcomeFileMapping()); diff --git a/servlet/src/main/java/io/undertow/servlet/spec/HttpServletRequestImpl.java b/servlet/src/main/java/io/undertow/servlet/spec/HttpServletRequestImpl.java index fab89f09c..2b71b89f7 100644 --- a/servlet/src/main/java/io/undertow/servlet/spec/HttpServletRequestImpl.java +++ b/servlet/src/main/java/io/undertow/servlet/spec/HttpServletRequestImpl.java @@ -229,15 +229,18 @@ public HttpServletMapping getHttpServletMapping() { break; case PATH: matchValue = match.getRemaining(); - if (matchValue.startsWith("/")) { + if (matchValue == null) { + matchValue = ""; + } else if (matchValue.startsWith("/")) { matchValue = matchValue.substring(1); } break; case EXTENSION: - matchValue = match.getMatched().substring(0, match.getMatched().length() - match.getMatchString().length() + 1); - if (matchValue.startsWith("/")) { - matchValue = matchValue.substring(1); - } + String matched = match.getMatched(); + String matchString = match.getMatchString(); + int startIndex = matched.startsWith("/") ? 1 : 0; + int endIndex = matched.length() - matchString.length() + 1; + matchValue = matched.substring(startIndex, endIndex); break; default: matchValue = match.getRemaining(); diff --git a/servlet/src/test/java/io/undertow/servlet/test/path/GetMappingServlet.java b/servlet/src/test/java/io/undertow/servlet/test/path/GetMappingServlet.java index 99c92e1f8..979fc3604 100644 --- a/servlet/src/test/java/io/undertow/servlet/test/path/GetMappingServlet.java +++ b/servlet/src/test/java/io/undertow/servlet/test/path/GetMappingServlet.java @@ -35,12 +35,12 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t response.getWriter() .append("Mapping match:") .append(mapping.getMappingMatch().name()) - .append("\n") - .append("Match value:") + .append("\nMatch value:") .append(mapping.getMatchValue()) - .append("\n") - .append("Pattern:") - .append(mapping.getPattern()); + .append("\nPattern:") + .append(mapping.getPattern()) + .append("\nServlet:") + .append(mapping.getServletName()); } } diff --git a/servlet/src/test/java/io/undertow/servlet/test/path/MappingTestCase.java b/servlet/src/test/java/io/undertow/servlet/test/path/MappingTestCase.java index aff035a34..de3dee4e5 100644 --- a/servlet/src/test/java/io/undertow/servlet/test/path/MappingTestCase.java +++ b/servlet/src/test/java/io/undertow/servlet/test/path/MappingTestCase.java @@ -63,7 +63,7 @@ public void testGetMapping() throws IOException { String response = HttpClientUtils.readResponse(result); Assert.assertEquals("Mapping match:PATH\n" + "Match value:foo\n" + - "Pattern:/path/*", response); + "Pattern:/path/*\nServlet:path", response); get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/foo.ext"); result = client.execute(get); @@ -71,7 +71,7 @@ public void testGetMapping() throws IOException { response = HttpClientUtils.readResponse(result); Assert.assertEquals("Mapping match:EXTENSION\n" + "Match value:foo\n" + - "Pattern:*.ext", response); + "Pattern:*.ext\nServlet:path", response); get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/"); result = client.execute(get); @@ -79,7 +79,7 @@ public void testGetMapping() throws IOException { response = HttpClientUtils.readResponse(result); Assert.assertEquals("Mapping match:CONTEXT_ROOT\n" + "Match value:\n" + - "Pattern:", response); + "Pattern:\nServlet:path", response); get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/doesnotexist"); result = client.execute(get); @@ -87,7 +87,7 @@ public void testGetMapping() throws IOException { response = HttpClientUtils.readResponse(result); Assert.assertEquals("Mapping match:DEFAULT\n" + "Match value:\n" + - "Pattern:/", response); + "Pattern:/\nServlet:path", response); get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/exact"); result = client.execute(get); @@ -95,7 +95,7 @@ public void testGetMapping() throws IOException { response = HttpClientUtils.readResponse(result); Assert.assertEquals("Mapping match:EXACT\n" + "Match value:exact\n" + - "Pattern:/exact", response); + "Pattern:/exact\nServlet:path", response); } finally { client.getConnectionManager().shutdown(); diff --git a/servlet/src/test/java/io/undertow/servlet/test/path/MultipleMatchingMappingTestCase.java b/servlet/src/test/java/io/undertow/servlet/test/path/MultipleMatchingMappingTestCase.java new file mode 100644 index 000000000..c825f96a1 --- /dev/null +++ b/servlet/src/test/java/io/undertow/servlet/test/path/MultipleMatchingMappingTestCase.java @@ -0,0 +1,94 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2021 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.undertow.servlet.test.path; + +import io.undertow.servlet.api.ServletInfo; +import io.undertow.servlet.test.util.DeploymentUtils; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.HttpClientUtils; +import io.undertow.testutils.TestHttpClient; +import io.undertow.httpcore.StatusCodes; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.servlet.ServletException; +import javax.servlet.http.MappingMatch; +import java.io.IOException; + +/** + * Test cases for UNDERTOW-1844. + * + * @author Carter Kozak + */ +@RunWith(DefaultServer.class) +public class MultipleMatchingMappingTestCase { + @BeforeClass + public static void setup() throws ServletException { + DeploymentUtils.setupServlet( + new ServletInfo("path", GetMappingServlet.class) + .addMapping("/path/*") + .addMapping("/*") + // This extension prefix is impossible to reach due to the '/*' path match. + .addMapping("*.ext")); + + } + + @Test + public void testMatchesPathAndExtension1() { + doTest("/foo.ext", MappingMatch.PATH, "foo.ext", "/*", "path"); + } + + @Test + public void testMatchesPathAndExtension2() { + doTest("/other/foo.ext", MappingMatch.PATH, "other/foo.ext", "/*", "path"); + } + + @Test + public void testMatchesPathAndExtension3() { + doTest("/path/foo.ext", MappingMatch.PATH, "foo.ext", "/path/*", "path"); + } + + private static void doTest( + // Input request path excluding the servlet context path + String path, + // Expected HttpServletMapping result values + MappingMatch mappingMatch, + String matchValue, + String pattern, + String servletName) { + TestHttpClient client = new TestHttpClient(); + try { + HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext" + path); + HttpResponse result = client.execute(get); + Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); + String response = HttpClientUtils.readResponse(result); + String expected = String.format("Mapping match:%s\nMatch value:%s\nPattern:%s\nServlet:%s", + mappingMatch.name(), matchValue, pattern, servletName); + Assert.assertEquals(expected, response); + } catch (IOException e) { + throw new AssertionError(e); + } finally { + client.getConnectionManager().shutdown(); + } + } +} diff --git a/servlet/src/test/java/io/undertow/servlet/test/path/ServletSpecExampleTestCase.java b/servlet/src/test/java/io/undertow/servlet/test/path/ServletSpecExampleTestCase.java new file mode 100644 index 000000000..914f43b63 --- /dev/null +++ b/servlet/src/test/java/io/undertow/servlet/test/path/ServletSpecExampleTestCase.java @@ -0,0 +1,108 @@ +package io.undertow.servlet.test.path; + +import io.undertow.servlet.api.ServletInfo; +import io.undertow.servlet.test.util.DeploymentUtils; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.HttpClientUtils; +import io.undertow.testutils.TestHttpClient; +import io.undertow.httpcore.StatusCodes; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.servlet.ServletException; +import javax.servlet.http.MappingMatch; +import java.io.IOException; + +/** + * Test cases for the servlet mapping examples in section 12.2.2 of the + * Servlet 4.0 specification. + * + * @author Carter Kozak + */ +@RunWith(DefaultServer.class) +public class ServletSpecExampleTestCase { + + @BeforeClass + public static void setup() throws ServletException { + // Servlet 4.0 table 12-1 Example Set of Maps + DeploymentUtils.setupServlet( + new ServletInfo("servlet1", GetMappingServlet.class) + .addMapping("/foo/bar/*"), + new ServletInfo("servlet2", GetMappingServlet.class) + .addMapping("/baz/*"), + new ServletInfo("servlet3", GetMappingServlet.class) + .addMapping("/catalog"), + new ServletInfo("servlet4", GetMappingServlet.class) + .addMapping("*.bop"), + new ServletInfo("default", GetMappingServlet.class)); + + } + + @Test + public void testOne() { + doTest("/foo/bar/index.html", MappingMatch.PATH, "index.html", "/foo/bar/*", "servlet1"); + } + + @Test + public void testTwo() { + doTest("/foo/bar/index.bop", MappingMatch.PATH, "index.bop", "/foo/bar/*", "servlet1"); + } + + @Test + public void testThree() { + doTest("/baz", MappingMatch.PATH, "", "/baz/*", "servlet2"); + } + + @Test + public void testFour() { + doTest("/baz/index.html", MappingMatch.PATH, "index.html", "/baz/*", "servlet2"); + } + + @Test + public void testFive() { + doTest("/catalog", MappingMatch.EXACT, "catalog", "/catalog", "servlet3"); + } + + @Test + public void testSix() { + doTest("/catalog/index.html", MappingMatch.DEFAULT, "", "/", "default"); + } + + @Test + public void testSeven() { + doTest("/catalog/racecar.bop", MappingMatch.EXTENSION, "catalog/racecar", "*.bop", "servlet4"); + } + + @Test + public void testEight() { + doTest("/index.bop", MappingMatch.EXTENSION, "index", "*.bop", "servlet4"); + } + + private static void doTest( + // Input request path excluding the servlet context path + String path, + // Expected HttpServletMapping result values + MappingMatch mappingMatch, + String matchValue, + String pattern, + String servletName) { + TestHttpClient client = new TestHttpClient(); + try { + HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext" + path); + HttpResponse result = client.execute(get); + Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); + String response = HttpClientUtils.readResponse(result); + String expected = String.format("Mapping match:%s\nMatch value:%s\nPattern:%s\nServlet:%s", + mappingMatch.name(), matchValue, pattern, servletName); + Assert.assertEquals(expected, response); + } catch (IOException e) { + throw new AssertionError(e); + } finally { + client.getConnectionManager().shutdown(); + } + } +}