Fixes #9910 - Inconsistent handling of welcome files

* Added DefaultServlet combinations tests.
* Removed pathInfoOnly handling present in Jetty 11, because it is always true for mapping to "/", and always false for other mappings.

Signed-off-by: Ludovic Orban <lorban@bitronix.be>

jetty-core/jetty-deploy/src/test/resources/etc/webdefault.xml

@@ -104,8 +104,6 @@
  *                    servlet context root. Useful for only serving static content out
  *                    of only specific subdirectories.
  *
- *  pathInfoOnly      If true, only the path info will be applied to the baseResource
- *
  *  stylesheet        Set with the location of an optional stylesheet that will be used
  *                    to decorate the directory listing html.
  *

jetty-ee10/jetty-ee10-demos/jetty-ee10-demo-embedded/src/main/resources/demo/webdefault-ee10.xml

@@ -104,8 +104,6 @@
  *                    servlet context root. Useful for only serving static content out
  *                    of only specific subdirectories.
  *
- *  pathInfoOnly      If true, only the path info will be applied to the baseResource
- *
  *  stylesheet        Set with the location of an optional stylesheet that will be used
  *                    to decorate the directory listing html.
  *                    

jetty-ee10/jetty-ee10-osgi/test-jetty-ee10-osgi/src/test/config/etc/webdefault-ee10.xml

@@ -104,8 +104,6 @@
  *                    servlet context root. Useful for only serving static content out
  *                    of only specific subdirectories.
  *
- *  pathInfoOnly      If true, only the path info will be applied to the baseResource
- *
  *  stylesheet        Set with the location of an optional stylesheet that will be used
  *                    to decorate the directory listing html.
  *

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java

@@ -44,6 +44,7 @@ import jakarta.servlet.http.HttpServlet;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import jakarta.servlet.http.HttpServletResponseWrapper;
+import jakarta.servlet.http.MappingMatch;
 import org.eclipse.jetty.http.CompressedContentFormat;
 import org.eclipse.jetty.http.HttpException;
 import org.eclipse.jetty.http.HttpField;
@@ -140,12 +141,6 @@ import org.slf4j.LoggerFactory;
  *     gzipped.
  *     Defaults to {@code .svgz}.
  *   </dd>
- *   <dt>pathInfoOnly</dt>
- *   <dd>
- *     Use {@code true} to use only the request {@code pathInfo} to look for
- *     static resources.
- *     Defaults to {@code false}.
- *   </dd>
  *   <dt>precompressed</dt>
  *   <dd>
  *     Omitted by default, so that no pre-compressed content will be served.
@@ -190,7 +185,6 @@ public class DefaultServlet extends HttpServlet
     private ServletResourceService _resourceService;
     private WelcomeServletMode _welcomeServletMode;
     private Resource _baseResource;
-    private boolean _isPathInfoOnly;
 
     public ResourceService getResourceService()
     {
@@ -289,8 +283,6 @@ public class DefaultServlet extends HttpServlet
         _resourceService.setPrecompressedFormats(precompressedFormats);
         _resourceService.setEtags(getInitBoolean("etags", _resourceService.isEtags()));
 
-        _isPathInfoOnly = getInitBoolean("pathInfoOnly", _isPathInfoOnly);
-
         _welcomeServletMode = WelcomeServletMode.NONE;
         String welcomeServlets = getInitParameter("welcomeServlets");
         if (welcomeServlets != null)
@@ -335,7 +327,6 @@ public class DefaultServlet extends HttpServlet
         {
             LOG.debug("  .baseResource = {}", _baseResource);
             LOG.debug("  .resourceService = {}", _resourceService);
-            LOG.debug("  .isPathInfoOnly = {}", _isPathInfoOnly);
             LOG.debug("  .welcomeServletMode = {}", _welcomeServletMode);
         }
     }
@@ -442,25 +433,12 @@ public class DefaultServlet extends HttpServlet
             servletContext.getClass().getName() + " is not " + ContextHandler.ScopedContext.class.getName());
     }
 
-    protected boolean isPathInfoOnly()
-    {
-        return _isPathInfoOnly;
-    }
-
     @Override
     protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
     {
         String includedServletPath = (String)req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH);
+        String encodedPathInContext = getEncodedPathInContext(req, includedServletPath);
         boolean included = includedServletPath != null;
-        String encodedPathInContext;
-        if (included)
-            encodedPathInContext = URIUtil.encodePath(getIncludedPathInContext(req, includedServletPath, isPathInfoOnly()));
-        else if (isPathInfoOnly())
-            encodedPathInContext = URIUtil.encodePath(req.getPathInfo());
-        else if (req instanceof ServletApiRequest apiRequest)
-            encodedPathInContext = Context.getPathInContext(req.getContextPath(), apiRequest.getRequest().getHttpURI().getCanonicalPath());
-        else
-            encodedPathInContext = Context.getPathInContext(req.getContextPath(), URIUtil.canonicalPath(req.getRequestURI()));
 
         if (LOG.isDebugEnabled())
             LOG.debug("doGet(req={}, resp={}) pathInContext={}, included={}", req, resp, encodedPathInContext, included);
@@ -536,6 +514,18 @@ public class DefaultServlet extends HttpServlet
         }
     }
 
+    protected String getEncodedPathInContext(HttpServletRequest req, String includedServletPath)
+    {
+        if (includedServletPath != null)
+            return URIUtil.encodePath(getIncludedPathInContext(req, includedServletPath, !isDefaultMapping(req)));
+        else if (!isDefaultMapping(req))
+            return URIUtil.encodePath(req.getPathInfo());
+        else if (req instanceof ServletApiRequest apiRequest)
+            return Context.getPathInContext(req.getContextPath(), apiRequest.getRequest().getHttpURI().getCanonicalPath());
+        else
+            return Context.getPathInContext(req.getContextPath(), URIUtil.canonicalPath(req.getRequestURI()));
+    }
+
     @Override
     protected void doHead(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
     {
@@ -1064,7 +1054,7 @@ public class DefaultServlet extends HttpServlet
                     // Check whether a Servlet may serve the welcome resource.
                     if (_welcomeServletMode != WelcomeServletMode.NONE && welcomeTarget == null)
                     {
-                        if (isPathInfoOnly() && !isIncluded(getServletRequest(coreRequest)))
+                        if (!isDefaultMapping(getServletRequest(coreRequest)) && !isIncluded(getServletRequest(coreRequest)))
                             welcomeTarget = URIUtil.addPaths(getServletRequest(coreRequest).getPathInfo(), welcome);
 
                         ServletHandler.MappedServlet entry = _servletContextHandler.getServletHandler().getMappedServlet(welcomeInContext);
@@ -1084,24 +1074,6 @@ public class DefaultServlet extends HttpServlet
             return welcomeTarget;
         }
 
-        @Override
-        protected void redirectWelcome(Request request, Response response, Callback callback, String welcomeTarget) throws IOException
-        {
-            HttpServletRequest servletRequest = getServletRequest(request);
-            HttpServletResponse servletResponse = getServletResponse(response);
-
-            boolean included = isIncluded(servletRequest);
-
-            String servletPath = included ? (String)servletRequest.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH)
-                : servletRequest.getServletPath();
-
-            if (isPathInfoOnly())
-                welcomeTarget = URIUtil.addPaths(servletPath, welcomeTarget);
-
-            servletResponse.setContentLength(0);
-            Response.sendRedirect(request, response, callback, welcomeTarget);
-        }
-
         @Override
         protected void serveWelcome(Request request, Response response, Callback callback, String welcomeTarget) throws IOException
         {
@@ -1227,6 +1199,11 @@ public class DefaultServlet extends HttpServlet
         return request.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI) != null;
     }
 
+    private static boolean isDefaultMapping(HttpServletRequest req)
+    {
+        return req.getHttpServletMapping().getMappingMatch() == MappingMatch.DEFAULT;
+    }
+
     /**
      * Wrap an existing HttpContent with one that takes has an unknown/unspecified length.
      */

jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletCombinationsTest.java

@@ -0,0 +1,285 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.ee10.servlet;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.server.HttpConfiguration;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.ResourceService;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
+import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.eclipse.jetty.server.ResourceService.WelcomeMode.REDIRECT;
+import static org.eclipse.jetty.server.ResourceService.WelcomeMode.REHANDLE;
+import static org.eclipse.jetty.server.ResourceService.WelcomeMode.SERVE;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+@ExtendWith(WorkDirExtension.class)
+public class DefaultServletCombinationsTest
+{
+    public WorkDir workDir;
+
+    public Path docRoot;
+    private Server server;
+    private LocalConnector connector;
+    private ServletContextHandler context;
+
+    @BeforeEach
+    public void init() throws Exception
+    {
+        docRoot = workDir.getEmptyPathDir().resolve("docroot");
+        FS.ensureDirExists(docRoot);
+        Files.writeString(docRoot.resolve("index.html"), "Static index.html at root", UTF_8);
+        Files.writeString(docRoot.resolve("foo.welcome"), "Static foo.welcome at root", UTF_8);
+
+        Path subdirPath = docRoot.resolve("subdir");
+        FS.ensureDirExists(subdirPath);
+        Files.writeString(subdirPath.resolve("index.html"), "Static index.html at root subdir", UTF_8);
+        Files.writeString(subdirPath.resolve("foo.welcome"), "Static foo.welcome at root subdir", UTF_8);
+
+        Path emptyPath = docRoot.resolve("empty");
+        FS.ensureDirExists(emptyPath);
+
+        Path staticPath = docRoot.resolve("static");
+        FS.ensureDirExists(staticPath);
+        Files.writeString(staticPath.resolve("index.html"), "Static index.html at static", UTF_8);
+        Files.writeString(staticPath.resolve("foo.welcome"), "Static foo.welcome at static", UTF_8);
+
+        Path staticsubdirPath = staticPath.resolve("subdir");
+        FS.ensureDirExists(staticsubdirPath);
+        Files.writeString(staticsubdirPath.resolve("index.html"), "Static index.html at static subdir", UTF_8);
+        Files.writeString(staticsubdirPath.resolve("foo.welcome"), "Static foo.welcome at static subdir", UTF_8);
+
+        Path subdirHtmlPath = docRoot.resolve("subdirHtml");
+        FS.ensureDirExists(subdirHtmlPath);
+        Files.writeString(subdirHtmlPath.resolve("index.html"), "Static index.html at root subdirHtml", UTF_8);
+        Path staticSubdirHtmlPath = staticPath.resolve("subdirHtml");
+        FS.ensureDirExists(staticSubdirHtmlPath);
+        Files.writeString(staticSubdirHtmlPath.resolve("index.html"), "Static index.html at static subdirHtml", UTF_8);
+
+        Path subdirWelcomePath = docRoot.resolve("subdirWelcome");
+        FS.ensureDirExists(subdirWelcomePath);
+        Files.writeString(subdirWelcomePath.resolve("foo.welcome"), "Static foo.welcome at root subdirWelcome", UTF_8);
+
+        Path staticSubdirWelcomePath = staticPath.resolve("subdirWelcome");
+        FS.ensureDirExists(staticSubdirWelcomePath);
+        Files.writeString(staticSubdirWelcomePath.resolve("foo.welcome"), "Static foo.welcome at static subdirWelcome", UTF_8);
+
+        Path subdirEmptyPath = staticPath.resolve("empty");
+        FS.ensureDirExists(subdirEmptyPath);
+
+        server = new Server();
+
+        connector = new LocalConnector(server);
+        connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setSendServerVersion(false);
+
+        context = new ServletContextHandler();
+        context.setBaseResourceAsPath(docRoot);
+        context.setContextPath("/ctx");
+        context.setWelcomeFiles(new String[]{"index.html", "index.welcome"});
+
+        ServletHolder welcomeExtHolder = context.addServlet(WelcomeServlet.class, "*.welcome");
+        welcomeExtHolder.setInitParameter("mapping", "welcome extension");
+    }
+
+    private void startServer(boolean pathInfoOnly, ResourceService.WelcomeMode welcomeMode) throws Exception
+    {
+        ServletHolder defaultHolder;
+        if (pathInfoOnly)
+        {
+            defaultHolder = context.addServlet(DefaultServlet.class, "/static/*");
+            context.addServlet(TeapotServlet.class,  "/");
+        }
+        else
+        {
+            defaultHolder = context.addServlet(DefaultServlet.class, "/");
+        }
+        defaultHolder.setInitParameter("dirAllowed", "false");
+
+        server.setHandler(context);
+        server.addConnector(connector);
+
+        server.start();
+        // Must happen after start.
+        ((DefaultServlet)defaultHolder.getServlet()).getResourceService().setWelcomeMode(welcomeMode);
+    }
+
+    @AfterEach
+    public void destroy() throws Exception
+    {
+        LifeCycle.stop(server);
+    }
+
+    public static class WelcomeServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            resp.setStatus(HttpStatus.OK_200);
+            String mapping = getInitParameter("mapping");
+            resp.getWriter().print("Servlet at " + mapping);
+        }
+    }
+
+    public static class TeapotServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            resp.setStatus(HttpStatus.IM_A_TEAPOT_418);
+        }
+    }
+
+    record Data(boolean pathInfoOnly, ResourceService.WelcomeMode welcomeMode, String requestPath, int expectedStatus, String expected)
+    {
+    }
+
+    public static Stream<Data> data()
+    {
+        List<Data> datas = new ArrayList<>();
+        for (String requestPath : List.of("/", "/foo.welcome", "/subdirHtml/",
+            "/subdirWelcome/", "/empty/", "/nothing/index.welcome", "/nothing/"))
+        {
+            for (ResourceService.WelcomeMode welcomeMode : List.of(SERVE, REDIRECT, REHANDLE))
+            {
+                for (boolean pathInfoOnly : List.of(false, true))
+                {
+                    int expectedStatus;
+                    String expected;
+
+                    switch (requestPath)
+                    {
+                        case "/" ->
+                        {
+                            switch (welcomeMode)
+                            {
+                                case SERVE ->
+                                {
+                                    expectedStatus = HttpStatus.OK_200;
+                                    expected = "Static index.html at root";
+                                }
+                                case REDIRECT ->
+                                {
+                                    expectedStatus = HttpStatus.FOUND_302;
+                                    expected = pathInfoOnly ? "http://local/ctx/static/index.html" : "http://local/ctx/index.html";
+                                }
+                                case REHANDLE ->
+                                {
+                                    expectedStatus = pathInfoOnly ? HttpStatus.IM_A_TEAPOT_418 : HttpStatus.NOT_FOUND_404;
+                                    expected = null;
+                                }
+                                default -> throw new AssertionError();
+                            }
+                        }
+                        case "/foo.welcome" ->
+                        {
+                            expectedStatus = HttpStatus.OK_200;
+                            expected = pathInfoOnly ? "Static foo.welcome at root" : "Servlet at welcome extension";
+                        }
+                        case "/subdirHtml/" ->
+                        {
+                            switch (welcomeMode)
+                            {
+                                case SERVE ->
+                                {
+                                    expectedStatus = HttpStatus.OK_200;
+                                    expected = "Static index.html at root subdirHtml";
+                                }
+                                case REDIRECT ->
+                                {
+                                    expectedStatus = HttpStatus.FOUND_302;
+                                    expected = pathInfoOnly ? "http://local/ctx/static/subdirHtml/index.html" : "http://local/ctx/subdirHtml/index.html";
+                                }
+                                case REHANDLE ->
+                                {
+                                    expectedStatus = pathInfoOnly ? HttpStatus.IM_A_TEAPOT_418 : HttpStatus.NOT_FOUND_404;
+                                    expected = null;
+                                }
+                                default -> throw new AssertionError();
+                            }
+                        }
+                        case "/subdirWelcome/" ->
+                        {
+                            expectedStatus = HttpStatus.FORBIDDEN_403;
+                            expected = null;
+                        }
+                        case "/empty/" ->
+                        {
+                            expectedStatus = HttpStatus.FORBIDDEN_403;
+                            expected = null;
+                        }
+                        case "/nothing/index.welcome" ->
+                        {
+                            expectedStatus = pathInfoOnly ? HttpStatus.NOT_FOUND_404 : HttpStatus.OK_200;
+                            expected = pathInfoOnly ? null : "Servlet at welcome extension";
+                        }
+                        case "/nothing/" ->
+                        {
+                            expectedStatus = HttpStatus.NOT_FOUND_404;
+                            expected = null;
+                        }
+                        default -> throw new AssertionError();
+                    }
+
+                    datas.add(new Data(pathInfoOnly, welcomeMode, requestPath, expectedStatus, expected));
+                }
+            }
+        }
+        return datas.stream();
+    }
+
+    @ParameterizedTest
+    @MethodSource("data")
+    public void testDefaultPathCombinations(Data data) throws Exception
+    {
+        startServer(data.pathInfoOnly(), data.welcomeMode());
+        String requestPath = context.getContextPath() + (data.pathInfoOnly() ? "/static" : "") + data.requestPath();
+        String rawResponse = connector.getResponse(String.format("""
+            GET %s HTTP/1.1\r
+            Host: local\r
+            Connection: close\r
+            \r
+            """, requestPath));
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        int status = response.getStatus();
+        assertThat(response.toString(), status, is(data.expectedStatus()));
+        if (status > 299 && status < 400)
+            assertThat(response.get(HttpHeader.LOCATION), is(data.expected));
+        else if (data.expected != null)
+            assertThat(response.getContent(), is(data.expected));
+    }
+}

jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java

@@ -531,7 +531,6 @@ public class DefaultServletTest
 
         ServletHolder defholder = context.addServlet(DefaultServlet.class, "/extra/*");
         defholder.setInitParameter("resourceBase", extraResourceBaseString);
-        defholder.setInitParameter("pathInfoOnly", "true");
         defholder.setInitParameter("dirAllowed", "true");
         defholder.setInitParameter("redirectWelcome", "false");
         defholder.setInitParameter("gzip", "false");
@@ -1056,7 +1055,6 @@ public class DefaultServletTest
 
         ServletHolder altholder = context.addServlet(DefaultServlet.class, "/alt/*");
         altholder.setInitParameter("resourceBase", altRoot.toUri().toASCIIString());
-        altholder.setInitParameter("pathInfoOnly", "true");
         altholder.setInitParameter("dirAllowed", "false");
         altholder.setInitParameter("redirectWelcome", "false");
         altholder.setInitParameter("welcomeServlets", "false");
@@ -1064,7 +1062,6 @@ public class DefaultServletTest
 
         ServletHolder otherholder = context.addServlet(DefaultServlet.class, "/other/*");
         otherholder.setInitParameter("resourceBase", altRoot.toUri().toASCIIString());
-        otherholder.setInitParameter("pathInfoOnly", "true");
         otherholder.setInitParameter("dirAllowed", "true");
         otherholder.setInitParameter("redirectWelcome", "false");
         otherholder.setInitParameter("welcomeServlets", "false");
@@ -1242,7 +1239,6 @@ public class DefaultServletTest
         defholder.setInitParameter("dirAllowed", "false");
         defholder.setInitParameter("redirectWelcome", "false");
         defholder.setInitParameter("welcomeServlets", "true");
-        defholder.setInitParameter("pathInfoOnly", "true");
 
         ServletHolder gwholder = new ServletHolder("gateway", new HttpServlet()
         {
@@ -3317,7 +3313,6 @@ public class DefaultServletTest
         ServletHolder slashHolder = new ServletHolder("default", new DefaultServlet());
         slashHolder.setInitParameter("redirectWelcome", "false");
         slashHolder.setInitParameter("welcomeServlets", "true");
-        slashHolder.setInitParameter("pathInfoOnly", "false");
         slashHolder.setInitParameter("baseResource", defaultDir.toAbsolutePath().toString());
         context.addServlet(slashHolder, "/");
 
@@ -3325,7 +3320,6 @@ public class DefaultServletTest
         ServletHolder rHolder = new ServletHolder("rdefault", new DefaultServlet());
         rHolder.setInitParameter("redirectWelcome", "false");
         rHolder.setInitParameter("welcomeServlets", "true");
-        rHolder.setInitParameter("pathInfoOnly", "true");
         rHolder.setInitParameter("baseResource", rDir.toAbsolutePath().toString());
         context.addServlet(rHolder, "/r/*");
 

jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/resources/webdefault-ee10.xml

@@ -96,8 +96,6 @@
  *                    servlet context root. Useful for only serving static content out
  *                    of only specific subdirectories.
  *
- *  pathInfoOnly      If true, only the path info will be applied to the baseResource
- *
  *  stylesheet        Set with the location of an optional stylesheet that will be used
  *                    to decorate the directory listing html.
  *

jetty-ee10/jetty-ee10-webapp/src/main/config/etc/webdefault-ee10.xml

@@ -104,8 +104,6 @@
  *                    servlet context root. Useful for only serving static content out
  *                    of only specific subdirectories.
  *
- *  pathInfoOnly      If true, only the path info will be applied to the baseResource
- *
  *  stylesheet        Set with the location of an optional stylesheet that will be used
  *                    to decorate the directory listing html.
  *