Skip to content

Commit

Permalink
Fixes #9910 - Inconsistent handling of welcome files
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@lorban
lorban authored Jul 14, 2023
1 parent a317a18 commit 248354f
Show file tree
Hide file tree
Showing 8 changed files with 305 additions and 59 deletions.
2 changes: 0 additions & 2 deletions jetty-core/jetty-deploy/src/test/resources/etc/webdefault.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
2 changes: 0 additions & 2 deletions ...e10/jetty-ee10-demos/jetty-ee10-demo-embedded/src/main/resources/demo/webdefault-ee10.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
2 changes: 0 additions & 2 deletions jetty-ee10/jetty-ee10-osgi/test-jetty-ee10-osgi/src/test/config/etc/webdefault-ee10.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
63 changes: 20 additions & 43 deletions ...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
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;
Expand Down Expand Up @@ -140,12 +141,6 @@
* 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.
Expand Down Expand Up @@ -190,7 +185,6 @@ public class DefaultServlet extends HttpServlet
private ServletResourceService _resourceService;
private WelcomeServletMode _welcomeServletMode;
private Resource _baseResource;
private boolean _isPathInfoOnly;

public ResourceService getResourceService()
{
Expand Down Expand Up @@ -289,8 +283,6 @@ public void init() throws ServletException
_resourceService.setPrecompressedFormats(precompressedFormats);
_resourceService.setEtags(getInitBoolean("etags", _resourceService.isEtags()));

_isPathInfoOnly = getInitBoolean("pathInfoOnly", _isPathInfoOnly);

_welcomeServletMode = WelcomeServletMode.NONE;
String welcomeServlets = getInitParameter("welcomeServlets");
if (welcomeServlets != null)
Expand Down Expand Up @@ -335,7 +327,6 @@ public void init() throws ServletException
{
LOG.debug(" .baseResource = {}", _baseResource);
LOG.debug(" .resourceService = {}", _resourceService);
LOG.debug(" .isPathInfoOnly = {}", _isPathInfoOnly);
LOG.debug(" .welcomeServletMode = {}", _welcomeServletMode);
}
}
Expand Down Expand Up @@ -442,25 +433,12 @@ protected ServletContextHandler initContextHandler(ServletContext servletContext
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);
Expand Down Expand Up @@ -536,6 +514,18 @@ else if (req instanceof ServletApiRequest apiRequest)
}
}

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
{
Expand Down Expand Up @@ -1064,7 +1054,7 @@ public String getWelcomeTarget(HttpContent content, Request coreRequest)
// 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);
Expand All @@ -1084,24 +1074,6 @@ public String getWelcomeTarget(HttpContent content, Request coreRequest)
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
{
Expand Down Expand Up @@ -1227,6 +1199,11 @@ private static boolean isIncluded(HttpServletRequest request)
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.
*/
Expand Down
Loading

0 comments on commit 248354f

Please sign in to comment.