UNDERTOW-1844: Fix getHttpServletMapping when both prefix and extension match

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<String, ServletHandler> 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<DispatcherType, List<ManagedFilter>> noExtension = new EnumMap<>(DispatcherType.class);
             final Map<String, Map<DispatcherType, List<ManagedFilter>>> 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, List<ManagedFilter>>(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

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());

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();

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());
     }
 
 }

servlet/src/test/java/io/undertow/servlet/test/path/MappingTestCase.java

@@ -63,39 +63,39 @@ 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);
             Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
             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);
             Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
             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);
             Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
             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);
             Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
             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();

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 <a href="https://issues.redhat.com/browse/UNDERTOW-1844">UNDERTOW-1844</a>.
+ *
+ * @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();
+        }
+    }
+}

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
+ * <a href="https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf">Servlet 4.0 specification</a>.
+ *
+ * @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();
+        }
+    }
+}