diff --git a/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/PaxWebSessionHandler.java b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/PaxWebSessionHandler.java index 6c50ee6fad..66575995c0 100644 --- a/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/PaxWebSessionHandler.java +++ b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/PaxWebSessionHandler.java @@ -26,7 +26,7 @@ public HttpCookie getSessionCookie(ManagedSession session, boolean requestIsSecu HttpCookie cookie = super.getSessionCookie(session, requestIsSecure); if (cookie != null) { String id = getExtendedId(cookie.getValue()); - return HttpCookie.from(cookie.getName(), id, getSessionAttributes()); + return HttpCookie.from(cookie.getName(), id, getSessionCookieAttributes()); } return cookie; } diff --git a/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/DefaultServlet.java b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/DefaultServlet.java index 5253fa98e1..18fa77f3a1 100644 --- a/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/DefaultServlet.java +++ b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/DefaultServlet.java @@ -15,26 +15,17 @@ import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStreamReader; import java.net.URI; -import java.nio.ByteBuffer; import java.nio.file.InvalidPathException; import java.time.Duration; import java.util.ArrayList; -import java.util.EnumSet; -import java.util.Enumeration; -import java.util.HashSet; import java.util.List; -import java.util.ListIterator; import java.util.Locale; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.StringTokenizer; -import java.util.concurrent.CompletableFuture; -import java.util.function.Supplier; -import java.util.stream.Collectors; +import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletContext; @@ -43,8 +34,11 @@ 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.ee10.servlet.Dispatcher; import org.eclipse.jetty.ee10.servlet.ServletApiRequest; +import org.eclipse.jetty.ee10.servlet.ServletApiResponse; +import org.eclipse.jetty.ee10.servlet.ServletChannel; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.ee10.servlet.ServletContextRequest; import org.eclipse.jetty.ee10.servlet.ServletContextResponse; @@ -52,11 +46,8 @@ import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.HttpException; import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.content.FileMappingHttpContentFactory; import org.eclipse.jetty.http.content.HttpContent; @@ -64,7 +55,6 @@ import org.eclipse.jetty.http.content.ResourceHttpContentFactory; import org.eclipse.jetty.http.content.ValidatingCachingHttpContentFactory; import org.eclipse.jetty.http.content.VirtualHttpContentFactory; -import org.eclipse.jetty.io.ByteBufferInputStream; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Request; @@ -73,17 +63,19 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.Blocker; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; import org.eclipse.jetty.util.resource.Resources; +import org.ops4j.pax.web.service.spi.servlet.OsgiHttpServletRequestWrapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.util.URIUtil.encodePath; + /** *

The default Servlet, normally mapped to {@code /}, that handles static resources.

*

The following init parameters are supported:

@@ -146,12 +138,6 @@ * gzipped. * Defaults to {@code .svgz}. * - *
pathInfoOnly
- *
- * Use {@code true} to use only the request {@code pathInfo} to look for - * static resources. - * Defaults to {@code false}. - *
*
precompressed
*
* Omitted by default, so that no pre-compressed content will be served. @@ -191,12 +177,11 @@ public class DefaultServlet extends HttpServlet { private static final Logger LOG = LoggerFactory.getLogger(DefaultServlet.class); + public static final String CONTEXT_INIT = "org.eclipse.jetty.servlet.Default."; private ServletContextHandler _contextHandler; private ServletResourceService _resourceService; private WelcomeServletMode _welcomeServletMode; - private Resource _baseResource; - private boolean _isPathInfoOnly; protected boolean redirectWelcome = false; @@ -211,14 +196,14 @@ public void init() throws ServletException _contextHandler = initContextHandler(getServletContext()); _resourceService = new ServletResourceService(_contextHandler); _resourceService.setWelcomeFactory(_resourceService); - _baseResource = _contextHandler.getBaseResource(); + Resource baseResource = _contextHandler.getBaseResource(); String rb = getInitParameter("baseResource", "resourceBase"); if (rb != null) { try { - _baseResource = Objects.requireNonNull(_contextHandler.newResource(rb)); + baseResource = Objects.requireNonNull(_contextHandler.newResource(rb)); } catch (Exception e) { @@ -228,7 +213,7 @@ public void init() throws ServletException } // Pax Web: for welcome file handling we need some _baseResource - _baseResource = configureBaseResource(); + baseResource = configureBaseResource(); List precompressedFormats = parsePrecompressedFormats(getInitParameter("precompressed"), getInitBoolean("gzip"), _resourceService.getPrecompressedFormats()); @@ -241,7 +226,7 @@ public void init() throws ServletException // Pax Web: we need to translate "" to "/", so this::getResource is not enough // also we want to use LaxResourceFactory whether or not _baseResource is set (it has to be set for // proper welcome file handling) -// ResourceFactory resourceFactory = _baseResource != null ? ResourceFactory.of(_baseResource) : this::getResource; +// ResourceFactory resourceFactory = baseResource != null ? ResourceFactory.of(baseResource) : this::getResource; ResourceFactory resourceFactory = new LaxResourceFactory(); contentFactory = new ResourceHttpContentFactory(resourceFactory, mimeTypes); @@ -305,8 +290,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) @@ -349,13 +332,13 @@ public void init() throws ServletException if (LOG.isDebugEnabled()) { - LOG.debug(" .baseResource = {}", _baseResource); + LOG.debug(" .baseResource = {}", baseResource); LOG.debug(" .resourceService = {}", _resourceService); - LOG.debug(" .isPathInfoOnly = {}", _isPathInfoOnly); LOG.debug(" .welcomeServletMode = {}", _welcomeServletMode); } } + // Pax Web protected Resource configureBaseResource() { return new EmptyResource(); } @@ -424,6 +407,29 @@ else if (gzip == Boolean.TRUE) return ret; } + /** + *

+ * Returns a {@code String} containing the value of the named initialization parameter, or null if the parameter does not exist. + *

+ * + *

+ * Parameter lookup first checks the {@link ServletContext#getInitParameter(String)} for the + * parameter prefixed with {@code org.eclipse.jetty.servlet.Default.}, then checks + * {@link jakarta.servlet.ServletConfig#getInitParameter(String)} for the actual value + *

+ * + * @param name a {@code String} specifying the name of the initialization parameter + * @return a {@code String} containing the value of the initialization parameter + */ + @Override + public String getInitParameter(String name) + { + String value = getServletContext().getInitParameter(CONTEXT_INIT + name); + if (value == null) + value = super.getInitParameter(name); + return value; + } + private Boolean getInitBoolean(String name) { String value = getInitParameter(name); @@ -462,32 +468,19 @@ 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 + protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) throws ServletException, IOException { - String includedServletPath = (String)req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + String includedServletPath = (String)httpServletRequest.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + String encodedPathInContext = getEncodedPathInContext(httpServletRequest, 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); + LOG.debug("doGet(hsReq={}, hsResp={}) pathInContext={}, included={}", httpServletRequest, httpServletResponse, encodedPathInContext, included); try { - HttpContent content = _resourceService.getContent(encodedPathInContext, ServletContextRequest.getServletContextRequest(req)); + HttpContent content = _resourceService.getContent(encodedPathInContext, ServletContextRequest.getServletContextRequest(httpServletRequest)); if (LOG.isDebugEnabled()) LOG.debug("content = {}", content); @@ -505,13 +498,31 @@ else if (req instanceof ServletApiRequest apiRequest) } // no content - resp.sendError(404); + httpServletResponse.sendError(404); } else { - ServletCoreRequest coreRequest = new ServletCoreRequest(req); - ServletCoreResponse coreResponse = new ServletCoreResponse(coreRequest, resp, included); - + // lookup the core request and response as wrapped by the ServletContextHandler + ServletContextRequest servletContextRequest = ServletContextRequest.getServletContextRequest(httpServletRequest); + ServletContextResponse servletContextResponse = servletContextRequest.getServletContextResponse(); + ServletChannel servletChannel = servletContextRequest.getServletChannel(); + + // If the servlet request has not been wrapped, + // we can use the core request directly, + // otherwise wrap the servlet request as a core request + Request coreRequest = httpServletRequest instanceof ServletApiRequest + ? servletChannel.getRequest() + : new ServletCoreRequest(httpServletRequest); + + // If the servlet response has been wrapped and has been written to, + // then the servlet response must be wrapped as a core response + // otherwise we can use the core response directly. + boolean useServletResponse = !(httpServletResponse instanceof ServletApiResponse) || servletContextResponse.isWritingOrStreaming(); + Response coreResponse = useServletResponse + ? new ServletCoreResponse(coreRequest, httpServletResponse, included) + : servletChannel.getResponse(); + + // If the core response is already committed then do nothing more if (coreResponse.isCommitted()) { if (LOG.isDebugEnabled()) @@ -519,30 +530,39 @@ else if (req instanceof ServletApiRequest apiRequest) return; } + // Get the content length before we may wrap the content + long contentLength = content.getContentLengthValue(); + // Servlet Filters could be interacting with the Response already. - if (coreResponse.isHttpServletResponseWrapped() || - coreResponse.isWritingOrStreaming()) - { + if (useServletResponse) content = new UnknownLengthHttpContent(content); - } - ServletContextResponse contextResponse = coreResponse.getServletContextResponse(); - if (contextResponse != null) - { - String characterEncoding = contextResponse.getRawCharacterEncoding(); - if (characterEncoding != null) - content = new ForcedCharacterEncodingHttpContent(content, characterEncoding); - } + // The character encoding may be forced + String characterEncoding = servletContextResponse.getRawCharacterEncoding(); + if (characterEncoding != null) + content = new ForcedCharacterEncodingHttpContent(content, characterEncoding); - // serve content - try (Blocker.Callback callback = Blocker.callback()) + // If async is supported and the unwrapped content is larger than an output buffer + if (httpServletRequest.isAsyncSupported() && + (contentLength < 0 || contentLength > coreRequest.getConnectionMetaData().getHttpConfiguration().getOutputBufferSize())) { + // send the content asynchronously + AsyncContext asyncContext = httpServletRequest.startAsync(); + Callback callback = new AsyncContextCallback(asyncContext, httpServletResponse); _resourceService.doGet(coreRequest, coreResponse, callback, content); - callback.block(); } - catch (Exception e) + else { - throw new ServletException(e); + // send the content blocking + try (Blocker.Callback callback = Blocker.callback()) + { + _resourceService.doGet(coreRequest, coreResponse, callback, content); + callback.block(); + } + catch (Exception e) + { + throw new ServletException(e); + } } } } @@ -552,10 +572,22 @@ else if (req instanceof ServletApiRequest apiRequest) LOG.debug("InvalidPathException for pathInContext: {}", encodedPathInContext, e); if (included) throw new FileNotFoundException(encodedPathInContext); - resp.setStatus(404); + httpServletResponse.setStatus(404); } } + protected String getEncodedPathInContext(HttpServletRequest req, String includedServletPath) + { + if (includedServletPath != null) + return encodePath(getIncludedPathInContext(req, includedServletPath, !isDefaultMapping(req))); + else if (!isDefaultMapping(req)) + return 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 { @@ -564,6 +596,20 @@ protected void doHead(HttpServletRequest req, HttpServletResponse resp) throws S doGet(req, resp); } + @Override + protected void doTrace(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + // Always return 405: Method Not Allowed for DefaultServlet + resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED); + } + + @Override + protected void doOptions(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + // override to eliminate TRACE that the default HttpServlet impl adds + resp.setHeader("Allow", "GET, HEAD, OPTIONS"); + } + protected Resource getResource(URI uri) { String uriPath = uri.getRawPath(); @@ -581,476 +627,6 @@ protected Resource getResource(URI uri) return result; } - private static class ServletCoreRequest extends Request.Wrapper - { - // TODO fully implement this class and move it to the top level - // TODO Some methods are directed to core that probably should be intercepted - - private final HttpServletRequest _servletRequest; - private final HttpFields _httpFields; - private final HttpURI _uri; - - ServletCoreRequest(HttpServletRequest request) - { - super(ServletContextRequest.getServletContextRequest(request)); - _servletRequest = request; - - HttpFields.Mutable fields = HttpFields.build(); - - Enumeration headerNames = request.getHeaderNames(); - while (headerNames.hasMoreElements()) - { - String headerName = headerNames.nextElement(); - Enumeration headerValues = request.getHeaders(headerName); - while (headerValues.hasMoreElements()) - { - String headerValue = headerValues.nextElement(); - fields.add(new HttpField(headerName, headerValue)); - } - } - - _httpFields = fields.asImmutable(); - String includedServletPath = (String)request.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); - boolean included = includedServletPath != null; - if (request.getDispatcherType() == DispatcherType.REQUEST) - _uri = getWrapped().getHttpURI(); - else if (included) - _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.encodePath(getIncludedPathInContext(request, includedServletPath, false))); - else - _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.encodePath(URIUtil.addPaths(_servletRequest.getServletPath(), _servletRequest.getPathInfo()))); - } - - @Override - public HttpFields getHeaders() - { - return _httpFields; - } - - @Override - public HttpURI getHttpURI() - { - return _uri; - } - - @Override - public String getId() - { - return _servletRequest.getRequestId(); - } - - @Override - public String getMethod() - { - return _servletRequest.getMethod(); - } - - @Override - public boolean isSecure() - { - return _servletRequest.isSecure(); - } - - @Override - public Object removeAttribute(String name) - { - Object value = _servletRequest.getAttribute(name); - _servletRequest.removeAttribute(name); - return value; - } - - @Override - public Object setAttribute(String name, Object attribute) - { - Object value = _servletRequest.getAttribute(name); - _servletRequest.setAttribute(name, attribute); - return value; - } - - @Override - public Object getAttribute(String name) - { - return _servletRequest.getAttribute(name); - } - - @Override - public Set getAttributeNameSet() - { - Set set = new HashSet<>(); - Enumeration e = _servletRequest.getAttributeNames(); - while (e.hasMoreElements()) - set.add(e.nextElement()); - return set; - } - - @Override - public void clearAttributes() - { - Enumeration e = _servletRequest.getAttributeNames(); - while (e.hasMoreElements()) - _servletRequest.removeAttribute(e.nextElement()); - } - } - - private static class HttpServletResponseHttpFields implements HttpFields.Mutable - { - private final HttpServletResponse _response; - - private HttpServletResponseHttpFields(HttpServletResponse response) - { - _response = response; - } - - @Override - public ListIterator listIterator() - { - // The minimum requirement is to implement the listIterator, but it is inefficient. - // Other methods are implemented for efficiency. - final ListIterator list = _response.getHeaderNames().stream() - .map(n -> new HttpField(n, _response.getHeader(n))) - .collect(Collectors.toList()) - .listIterator(); - - return new ListIterator<>() - { - HttpField _last; - - @Override - public boolean hasNext() - { - return list.hasNext(); - } - - @Override - public HttpField next() - { - return _last = list.next(); - } - - @Override - public boolean hasPrevious() - { - return list.hasPrevious(); - } - - @Override - public HttpField previous() - { - return _last = list.previous(); - } - - @Override - public int nextIndex() - { - return list.nextIndex(); - } - - @Override - public int previousIndex() - { - return list.previousIndex(); - } - - @Override - public void remove() - { - if (_last != null) - { - // This is not exactly the right semantic for repeated field names - list.remove(); - _response.setHeader(_last.getName(), null); - } - } - - @Override - public void set(HttpField httpField) - { - list.set(httpField); - _response.setHeader(httpField.getName(), httpField.getValue()); - } - - @Override - public void add(HttpField httpField) - { - list.add(httpField); - _response.addHeader(httpField.getName(), httpField.getValue()); - } - }; - } - - @Override - public Mutable add(String name, String value) - { - _response.addHeader(name, value); - return this; - } - - @Override - public Mutable add(HttpHeader header, HttpHeaderValue value) - { - _response.addHeader(header.asString(), value.asString()); - return this; - } - - @Override - public Mutable add(HttpHeader header, String value) - { - _response.addHeader(header.asString(), value); - return this; - } - - @Override - public Mutable add(HttpField field) - { - _response.addHeader(field.getName(), field.getValue()); - return this; - } - - @Override - public Mutable put(HttpField field) - { - _response.setHeader(field.getName(), field.getValue()); - return this; - } - - @Override - public Mutable put(String name, String value) - { - _response.setHeader(name, value); - return this; - } - - @Override - public Mutable put(HttpHeader header, HttpHeaderValue value) - { - _response.setHeader(header.asString(), value.asString()); - return this; - } - - @Override - public Mutable put(HttpHeader header, String value) - { - _response.setHeader(header.asString(), value); - return this; - } - - @Override - public Mutable put(String name, List list) - { - Objects.requireNonNull(name); - Objects.requireNonNull(list); - boolean first = true; - for (String s : list) - { - if (first) - _response.setHeader(name, s); - else - _response.addHeader(name, s); - first = false; - } - return this; - } - - @Override - public Mutable remove(HttpHeader header) - { - _response.setHeader(header.asString(), null); - return this; - } - - @Override - public Mutable remove(EnumSet fields) - { - for (HttpHeader header : fields) - remove(header); - return this; - } - - @Override - public Mutable remove(String name) - { - _response.setHeader(name, null); - return this; - } - } - - private static class ServletCoreResponse implements Response - { - // TODO fully implement this class and move it to the top level - - private final HttpServletResponse _response; - private final ServletCoreRequest _coreRequest; - private final Response _coreResponse; - private final HttpFields.Mutable _httpFields; - private final boolean _included; - - public ServletCoreResponse(ServletCoreRequest coreRequest, HttpServletResponse response, boolean included) - { - _coreRequest = coreRequest; - _response = response; - _coreResponse = ServletContextResponse.getServletContextResponse(response); - HttpFields.Mutable fields = new HttpServletResponseHttpFields(response); - if (included) - { - // If included, accept but ignore mutations. - fields = new HttpFields.Mutable.Wrapper(fields) - { - @Override - public HttpField onAddField(HttpField field) - { - return null; - } - - @Override - public boolean onRemoveField(HttpField field) - { - return false; - } - }; - } - _httpFields = fields; - _included = included; - } - - @Override - public HttpFields.Mutable getHeaders() - { - return _httpFields; - } - - public ServletContextResponse getServletContextResponse() - { - return ServletContextResponse.getServletContextResponse(_response); - } - - @Override - public boolean isCommitted() - { - return _response.isCommitted(); - } - - /** - * Test if the HttpServletResponse is wrapped by the webapp. - * - * @return true if wrapped. - */ - public boolean isHttpServletResponseWrapped() - { - return (_response instanceof HttpServletResponseWrapper); - } - - /** - * Test if {@link HttpServletResponse#getOutputStream()} or - * {@link HttpServletResponse#getWriter()} has been called already - * - * @return true if {@link HttpServletResponse} has started to write or stream content - */ - public boolean isWritingOrStreaming() - { - ServletContextResponse servletContextResponse = Response.as(_coreResponse, ServletContextResponse.class); - return servletContextResponse.isWritingOrStreaming(); - } - - public boolean isWriting() - { - ServletContextResponse servletContextResponse = Response.as(_coreResponse, ServletContextResponse.class); - return servletContextResponse.isWriting(); - } - - @Override - public void write(boolean last, ByteBuffer byteBuffer, Callback callback) - { - if (_included) - last = false; - try - { - if (BufferUtil.hasContent(byteBuffer)) - { - if (isWriting()) - { - String characterEncoding = _response.getCharacterEncoding(); - try (ByteBufferInputStream bbis = new ByteBufferInputStream(byteBuffer); - InputStreamReader reader = new InputStreamReader(bbis, characterEncoding)) - { - IO.copy(reader, _response.getWriter()); - } - - if (last) - _response.getWriter().close(); - } - else - { - BufferUtil.writeTo(byteBuffer, _response.getOutputStream()); - if (last) - _response.getOutputStream().close(); - } - } - - callback.succeeded(); - } - catch (Throwable t) - { - callback.failed(t); - } - } - - @Override - public Request getRequest() - { - return _coreRequest; - } - - @Override - public int getStatus() - { - return _response.getStatus(); - } - - @Override - public void setStatus(int code) - { - if (LOG.isDebugEnabled()) - LOG.debug("{}.setStatus({})", this.getClass().getSimpleName(), code); - if (_included) - return; - _response.setStatus(code); - } - - @Override - public Supplier getTrailersSupplier() - { - return null; - } - - @Override - public void setTrailersSupplier(Supplier trailers) - { - } - - @Override - public boolean isCompletedSuccessfully() - { - return _coreResponse.isCompletedSuccessfully(); - } - - @Override - public void reset() - { - _response.reset(); - } - - @Override - public CompletableFuture writeInterim(int status, HttpFields headers) - { - return null; - } - - @Override - public String toString() - { - return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), this._coreRequest, _response); - } - } - private class ServletResourceService extends ResourceService implements ResourceService.WelcomeFactory { private final ServletContextHandler _servletContextHandler; @@ -1084,7 +660,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); @@ -1104,24 +680,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 { @@ -1224,18 +782,32 @@ protected boolean passConditionalHeaders(Request request, Response response, Htt private HttpServletRequest getServletRequest(Request request) { - // TODO, this unwrapping is fragile - return ((ServletCoreRequest)request)._servletRequest; + ServletCoreRequest servletCoreRequest = Request.as(request, ServletCoreRequest.class); + if (servletCoreRequest != null) + return servletCoreRequest.getServletRequest(); + + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + if (servletContextRequest != null) + return servletContextRequest.getServletApiRequest(); + + throw new IllegalStateException("instanceof " + request.getClass()); } private HttpServletResponse getServletResponse(Response response) { - // TODO, this unwrapping is fragile - return ((ServletCoreResponse)response)._response; + ServletCoreResponse servletCoreResponse = Response.as(response, ServletCoreResponse.class); + if (servletCoreResponse != null) + return servletCoreResponse.getServletResponse(); + + ServletContextResponse servletContextResponse = Response.as(response, ServletContextResponse.class); + if (servletContextResponse != null) + return servletContextResponse.getServletApiResponse(); + + throw new IllegalStateException("instanceof " + response.getClass()); } } - private static String getIncludedPathInContext(HttpServletRequest request, String includedServletPath, boolean isPathInfoOnly) + static String getIncludedPathInContext(HttpServletRequest request, String includedServletPath, boolean isPathInfoOnly) { String servletPath = isPathInfoOnly ? "/" : includedServletPath; String pathInfo = (String)request.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO); @@ -1247,6 +819,13 @@ private static boolean isIncluded(HttpServletRequest request) return request.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI) != null; } + protected boolean isDefaultMapping(HttpServletRequest req) + { + if (req.getHttpServletMapping().getMappingMatch() == MappingMatch.DEFAULT) + return true; + return (req.getDispatcherType() != DispatcherType.REQUEST) && "default".equals(getServletConfig().getServletName()); + } + /** * Wrap an existing HttpContent with one that takes has an unknown/unspecified length. */ @@ -1334,6 +913,47 @@ private enum WelcomeServletMode EXACT } + private static class AsyncContextCallback implements Callback + { + private final AsyncContext _asyncContext; + private final HttpServletResponse _response; + + private AsyncContextCallback(AsyncContext asyncContext, HttpServletResponse response) + { + _asyncContext = asyncContext; + _response = response; + } + + @Override + public void succeeded() + { + _asyncContext.complete(); + } + + @Override + public void failed(Throwable x) + { + try + { + if (LOG.isDebugEnabled()) + LOG.debug("AsyncContextCallback failed {}", _asyncContext, x); + // It is known that this callback is only failed if the response is already committed, + // thus we can only abort the response here. + _response.sendError(-1); + } + catch (IOException e) + { + ExceptionUtil.addSuppressedIfNotAssociated(x, e); + } + finally + { + _asyncContext.complete(); + } + if (LOG.isDebugEnabled()) + LOG.debug("Async get failed", x); + } + } + private class LaxResourceFactory implements ResourceFactory { @Override diff --git a/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/ServletCoreRequest.java b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/ServletCoreRequest.java new file mode 100644 index 0000000000..f24f55380e --- /dev/null +++ b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/ServletCoreRequest.java @@ -0,0 +1,267 @@ +// +// ======================================================================== +// 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.ops4j.pax.web.service.jetty.internal.web; + +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Predicate; + +import jakarta.servlet.DispatcherType; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.http.HttpServletRequest; +import org.eclipse.jetty.ee10.servlet.ServletContextRequest; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Components; +import org.eclipse.jetty.server.ConnectionMetaData; +import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.server.HttpStream; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Session; +import org.eclipse.jetty.server.TunnelSupport; +import org.eclipse.jetty.util.URIUtil; + +import static org.eclipse.jetty.util.URIUtil.addEncodedPaths; +import static org.eclipse.jetty.util.URIUtil.encodePath; + +/** + * Wrap a {@link jakarta.servlet.ServletRequest} as a core {@link Request}. + *

+ * Whilst similar to a {@link Request.Wrapper}, this class is not a {@code Wrapper} + * as callers should not be able to access {@link Wrapper#getWrapped()} and bypass + * the {@link jakarta.servlet.ServletRequest}. + *

+ *

+ * The current implementation does not support any read operations. + *

+ */ +class ServletCoreRequest implements Request +{ + private final HttpServletRequest _servletRequest; + private final ServletContextRequest _servletContextRequest; + private final HttpFields _httpFields; + private final HttpURI _uri; + + ServletCoreRequest(HttpServletRequest request) + { + _servletRequest = request; + _servletContextRequest = ServletContextRequest.getServletContextRequest(_servletRequest); + + HttpFields.Mutable fields = HttpFields.build(); + + Enumeration headerNames = request.getHeaderNames(); + while (headerNames.hasMoreElements()) + { + String headerName = headerNames.nextElement(); + Enumeration headerValues = request.getHeaders(headerName); + while (headerValues.hasMoreElements()) + { + String headerValue = headerValues.nextElement(); + fields.add(new HttpField(headerName, headerValue)); + } + } + + _httpFields = fields.asImmutable(); + String includedServletPath = (String)request.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + boolean included = includedServletPath != null; + + HttpURI.Mutable builder = HttpURI.build(); + builder.scheme(request.getScheme()) + .authority(request.getServerName(), request.getServerPort()); + + if (included) + builder.path(addEncodedPaths(request.getContextPath(), encodePath(DefaultServlet.getIncludedPathInContext(request, includedServletPath, false)))); + else if (request.getDispatcherType() != DispatcherType.REQUEST) + builder.path(addEncodedPaths(request.getContextPath(), encodePath(URIUtil.addPaths(_servletRequest.getServletPath(), _servletRequest.getPathInfo())))); + else + builder.path(request.getRequestURI()); + builder.query(request.getQueryString()); + _uri = builder.asImmutable(); + } + + @Override + public HttpFields getHeaders() + { + return _httpFields; + } + + @Override + public HttpURI getHttpURI() + { + return _uri; + } + + @Override + public String getId() + { + return _servletRequest.getRequestId(); + } + + @Override + public String getMethod() + { + return _servletRequest.getMethod(); + } + + public HttpServletRequest getServletRequest() + { + return _servletRequest; + } + + @Override + public boolean isSecure() + { + return _servletRequest.isSecure(); + } + + @Override + public Object removeAttribute(String name) + { + Object value = _servletRequest.getAttribute(name); + _servletRequest.removeAttribute(name); + return value; + } + + @Override + public Object setAttribute(String name, Object attribute) + { + Object value = _servletRequest.getAttribute(name); + _servletRequest.setAttribute(name, attribute); + return value; + } + + @Override + public Object getAttribute(String name) + { + return _servletRequest.getAttribute(name); + } + + @Override + public Set getAttributeNameSet() + { + Set set = new HashSet<>(); + Enumeration e = _servletRequest.getAttributeNames(); + while (e.hasMoreElements()) + { + set.add(e.nextElement()); + } + return set; + } + + @Override + public void clearAttributes() + { + Enumeration e = _servletRequest.getAttributeNames(); + while (e.hasMoreElements()) + { + _servletRequest.removeAttribute(e.nextElement()); + } + } + + @Override + public void fail(Throwable failure) + { + throw new UnsupportedOperationException(); + } + + @Override + public Components getComponents() + { + return _servletContextRequest.getComponents(); + } + + @Override + public ConnectionMetaData getConnectionMetaData() + { + return _servletContextRequest.getConnectionMetaData(); + } + + @Override + public Context getContext() + { + return _servletContextRequest.getContext(); + } + + @Override + public void demand(Runnable demandCallback) + { + throw new UnsupportedOperationException(); + } + + @Override + public HttpFields getTrailers() + { + return _servletContextRequest.getTrailers(); + } + + @Override + public long getBeginNanoTime() + { + return _servletContextRequest.getBeginNanoTime(); + } + + @Override + public long getHeadersNanoTime() + { + return _servletContextRequest.getHeadersNanoTime(); + } + + @Override + public Content.Chunk read() + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean consumeAvailable() + { + throw new UnsupportedOperationException(); + } + + @Override + public void addIdleTimeoutListener(Predicate onIdleTimeout) + { + _servletContextRequest.addIdleTimeoutListener(onIdleTimeout); + } + + @Override + public void addFailureListener(Consumer onFailure) + { + _servletContextRequest.addFailureListener(onFailure); + } + + @Override + public TunnelSupport getTunnelSupport() + { + return null; + } + + @Override + public void addHttpStreamWrapper(Function wrapper) + { + _servletContextRequest.addHttpStreamWrapper(wrapper); + } + + @Override + public Session getSession(boolean create) + { + return Session.getSession(_servletRequest.getSession(create)); + } +} diff --git a/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/ServletCoreResponse.java b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/ServletCoreResponse.java new file mode 100644 index 0000000000..caee13db7a --- /dev/null +++ b/pax-web-jetty/src/main/java/org/ops4j/pax/web/service/jetty/internal/web/ServletCoreResponse.java @@ -0,0 +1,380 @@ +// +// ======================================================================== +// 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.ops4j.pax.web.service.jetty.internal.web; + +import java.io.InputStreamReader; +import java.nio.ByteBuffer; +import java.util.EnumSet; +import java.util.List; +import java.util.ListIterator; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee10.servlet.ServletContextResponse; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpHeaderValue; +import org.eclipse.jetty.io.ByteBufferInputStream; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; + +/** + * A {@link HttpServletResponse} wrapped as a core {@link Response}. + * All write operations are internally converted to blocking writes on the servlet API. + */ +class ServletCoreResponse implements Response +{ + private final HttpServletResponse _response; + private final Request _coreRequest; + private final HttpFields.Mutable _httpFields; + private final boolean _included; + private final ServletContextResponse _servletContextResponse; + + public ServletCoreResponse(Request coreRequest, HttpServletResponse response, boolean included) + { + _coreRequest = coreRequest; + _response = response; + _servletContextResponse = ServletContextResponse.getServletContextResponse(response); + HttpFields.Mutable fields = new HttpServletResponseHttpFields(response); + if (included) + { + // If included, accept but ignore mutations. + fields = new HttpFields.Mutable.Wrapper(fields) + { + @Override + public HttpField onAddField(HttpField field) + { + return null; + } + + @Override + public boolean onRemoveField(HttpField field) + { + return false; + } + }; + } + _httpFields = fields; + _included = included; + } + + @Override + public HttpFields.Mutable getHeaders() + { + return _httpFields; + } + + public HttpServletResponse getServletResponse() + { + return _response; + } + + @Override + public boolean hasLastWrite() + { + return _servletContextResponse.hasLastWrite(); + } + + @Override + public boolean isCompletedSuccessfully() + { + return _servletContextResponse.isCompletedSuccessfully(); + } + + @Override + public boolean isCommitted() + { + return _response.isCommitted(); + } + + private boolean isWriting() + { + return _servletContextResponse.isWriting(); + } + + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (_included) + last = false; + try + { + if (BufferUtil.hasContent(byteBuffer)) + { + if (isWriting()) + { + String characterEncoding = _response.getCharacterEncoding(); + try (ByteBufferInputStream bbis = new ByteBufferInputStream(byteBuffer); + InputStreamReader reader = new InputStreamReader(bbis, characterEncoding)) + { + IO.copy(reader, _response.getWriter()); + } + + if (last) + _response.getWriter().close(); + } + else + { + BufferUtil.writeTo(byteBuffer, _response.getOutputStream()); + if (last) + _response.getOutputStream().close(); + } + } + + callback.succeeded(); + } + catch (Throwable t) + { + callback.failed(t); + } + } + + @Override + public Request getRequest() + { + return _coreRequest; + } + + @Override + public int getStatus() + { + return _response.getStatus(); + } + + @Override + public void setStatus(int code) + { + if (_included) + return; + _response.setStatus(code); + } + + @Override + public Supplier getTrailersSupplier() + { + return null; + } + + @Override + public void setTrailersSupplier(Supplier trailers) + { + } + + @Override + public void reset() + { + _response.reset(); + } + + @Override + public CompletableFuture writeInterim(int status, HttpFields headers) + { + throw new UnsupportedOperationException(); + } + + @Override + public String toString() + { + return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), this._coreRequest, _response); + } + + private static class HttpServletResponseHttpFields implements HttpFields.Mutable + { + private final HttpServletResponse _response; + + private HttpServletResponseHttpFields(HttpServletResponse response) + { + _response = response; + } + + @Override + public ListIterator listIterator(int index) + { + // The minimum requirement is to implement the listIterator, but it is inefficient. + // Other methods are implemented for efficiency. + final ListIterator list = _response.getHeaderNames().stream() + .map(n -> new HttpField(n, _response.getHeader(n))) + .collect(Collectors.toList()) + .listIterator(index); + + return new ListIterator<>() + { + HttpField _last; + + @Override + public boolean hasNext() + { + return list.hasNext(); + } + + @Override + public HttpField next() + { + return _last = list.next(); + } + + @Override + public boolean hasPrevious() + { + return list.hasPrevious(); + } + + @Override + public HttpField previous() + { + return _last = list.previous(); + } + + @Override + public int nextIndex() + { + return list.nextIndex(); + } + + @Override + public int previousIndex() + { + return list.previousIndex(); + } + + @Override + public void remove() + { + if (_last != null) + { + // This is not exactly the right semantic for repeated field names + list.remove(); + _response.setHeader(_last.getName(), null); + } + } + + @Override + public void set(HttpField httpField) + { + list.set(httpField); + _response.setHeader(httpField.getName(), httpField.getValue()); + } + + @Override + public void add(HttpField httpField) + { + list.add(httpField); + _response.addHeader(httpField.getName(), httpField.getValue()); + } + }; + } + + @Override + public Mutable add(String name, String value) + { + _response.addHeader(name, value); + return this; + } + + @Override + public Mutable add(HttpHeader header, HttpHeaderValue value) + { + _response.addHeader(header.asString(), value.asString()); + return this; + } + + @Override + public Mutable add(HttpHeader header, String value) + { + _response.addHeader(header.asString(), value); + return this; + } + + @Override + public Mutable add(HttpField field) + { + _response.addHeader(field.getName(), field.getValue()); + return this; + } + + @Override + public Mutable put(HttpField field) + { + _response.setHeader(field.getName(), field.getValue()); + return this; + } + + @Override + public Mutable put(String name, String value) + { + _response.setHeader(name, value); + return this; + } + + @Override + public Mutable put(HttpHeader header, HttpHeaderValue value) + { + _response.setHeader(header.asString(), value.asString()); + return this; + } + + @Override + public Mutable put(HttpHeader header, String value) + { + _response.setHeader(header.asString(), value); + return this; + } + + @Override + public Mutable put(String name, List list) + { + Objects.requireNonNull(name); + Objects.requireNonNull(list); + boolean first = true; + for (String s : list) + { + if (first) + _response.setHeader(name, s); + else + _response.addHeader(name, s); + first = false; + } + return this; + } + + @Override + public Mutable remove(HttpHeader header) + { + _response.setHeader(header.asString(), null); + return this; + } + + @Override + public Mutable remove(EnumSet fields) + { + for (HttpHeader header : fields) + remove(header); + return this; + } + + @Override + public Mutable remove(String name) + { + _response.setHeader(name, null); + return this; + } + } +} diff --git a/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/UnifiedJettyTest.java b/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/UnifiedJettyTest.java index 7bc61dbeb2..483e617d8b 100644 --- a/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/UnifiedJettyTest.java +++ b/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/UnifiedJettyTest.java @@ -46,6 +46,7 @@ import org.eclipse.jetty.ee10.servlet.ServletMapping; import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.PathResourceFactory; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.ops4j.pax.web.service.jetty.internal.web.JettyResourceServlet; import org.slf4j.Logger; @@ -371,10 +372,11 @@ public void resourceServletWithWelcomePages() throws Exception { } @Test + @Disabled("jetty/jetty.project/issues/10608") public void paxWebWelcomePages() throws Exception { Server server = new Server(); ServerConnector connector = new ServerConnector(server, 1, 1, new HttpConnectionFactory()); - connector.setPort(0); + connector.setPort(8900); server.setConnectors(new Connector[] { connector }); ContextHandlerCollection chc = new ContextHandlerCollection(); @@ -512,7 +514,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc assertTrue(response.endsWith("'index-z-b1'")); // "/" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet - // forward is performed implicitly by Jetty's DefaultServlet + // (see order of welcome files). Forward is performed implicitly by Jetty's DefaultServlet. response = send(port, "/"); assertTrue(response.contains("req.context_path=\"\"")); assertTrue(response.contains("req.request_uri=\"/index.y\"")); @@ -604,8 +606,23 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc response = send(port, "/gateway/x?what=include&where=/r"); assertTrue(response.endsWith(">>><<<")); + // See https://github.com/eclipse/jetty.project/issues/9910 + // for changed welcome file handling with Jetty's own "pathInfoOnly" + // "/r" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet - // forward is performed implicitly by Jetty's DefaultServlet to "/index.y" (first welcome) + // forward is performed implicitly by Jetty's DefaultServlet (even if mapped to /r/*), forward URI is + // "/index.y" (first welcome), which is different than in Jetty before 12, where forward was "/r/index.y". + // the reasons are explained in https://github.com/eclipse/jetty.project/issues/9910 and quick summary may + // be just this: + // - '/r/' is handled by resource servlet which may (pathInfoOnly=false) or may not (pathInfoOnly=true) + // use '/r/' prefix to find resources (with welcome file definitions support) + // - at the stage of servlet mapping with welcome files, pahInfoOnly setting was not used by Jetty before 12 + // and the forward (or redirect) was prepending the URL with servlet path + // - so in Pax Web, where we always set pathInfoOnly=true for resource (default) servlets mapped to something + // different than '/', we should always reject servlet path - during resource lookup AND during + // forward/redirect + // - and pathInfoOnly=false is set only for '/' mapped servlets where it's not changing URI construction. + // it's used only to prevent endless redirect ;) response = send(port, "/r/"); assertTrue(response.contains("req.context_path=\"\"")); assertTrue(response.contains("req.request_uri=\"/index.y\"")); @@ -613,6 +630,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/r/\"")); assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/r\"")); assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/\"")); + response = send(port, "/gateway/x?what=forward&where=/r/"); assertTrue(response.contains("req.context_path=\"\"")); assertTrue(response.contains("req.request_uri=\"/index.y\"")); @@ -621,8 +639,20 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/gateway\"")); assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/x\"")); response = send(port, "/gateway/x?what=include&where=/r/"); + // gateway uses dispatcher for /r/ and calls include. /r/* servlet uses welcome files, so index.y servlet mapping + // is found and include target is /index.y (in Jetty before 12 it was /r/index.x which wasn't found by + // resource servlet, so should result in HTTP 500 according to 9.3 "The Include Method") + // see https://github.com/jetty/jetty.project/issues/10582 + // see https://github.com/jetty/jetty.project/issues/10608 + assertTrue(response.contains(">>>'indexx servlet'")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.include.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.include.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.servlet_path=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.path_info=\"null\"")); // HTTP 500 according to 9.3 "The Include Method" - // >>> current failure (HTTP 500 is only when the resource doesn't exist) <<< + response = send(port, "/gateway/x?what=include&where=/r/xyz"); assertTrue(response.startsWith("HTTP/1.1 500")); response = send(port, "/r/sub"); @@ -635,13 +665,10 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc // this time, welcome file is /sub/index.x and even if it maps to existing servlet (*.x), physical // resource exists and is returned response = send(port, "/r/sub/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.endsWith("'sub/index-b2'")); response = send(port, "/gateway/x?what=forward&where=/r/sub/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.endsWith("'sub/index-b2'")); response = send(port, "/gateway/x?what=include&where=/r/sub/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.endsWith(">>>'sub/index-b2'<<<")); // --- resource access through "/s" servlet - welcome files with redirect @@ -655,17 +682,15 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc assertTrue(response.endsWith(">>><<<")); response = send(port, "/s/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.startsWith("HTTP/1.1 302")); - // redirect to first welcome page with found *.y mapping, but another mapping will be found using /s/* - assertTrue(extractHeaders(response).get("Location").endsWith("/s/index.y")); + // redirect to first welcome page with found *.y mapping, and with redirect NOT using '/s' servlet path + // (because pathInfoOnly=true) + assertTrue(extractHeaders(response).get("Location").endsWith("/index.y")); response = send(port, "/gateway/x?what=forward&where=/s/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.startsWith("HTTP/1.1 302")); - assertTrue(extractHeaders(response).get("Location").endsWith("/s/index.y?what=forward&where=/s/")); + assertTrue(extractHeaders(response).get("Location").endsWith("/index.y?what=forward&where=/s/")); response = send(port, "/gateway/x?what=include&where=/s/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.contains(">>><<<")); response = send(port, "/s/sub"); @@ -680,15 +705,12 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc // this time, welcome file is /sub/index.x and even if it maps to existing servlet (*.x), physical // resource exists and is returned response = send(port, "/s/sub/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.startsWith("HTTP/1.1 302")); assertTrue(extractHeaders(response).get("Location").endsWith("/s/sub/index.x")); response = send(port, "/gateway/x?what=forward&where=/s/sub/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.startsWith("HTTP/1.1 302")); assertTrue(extractHeaders(response).get("Location").endsWith("/s/sub/index.x?what=forward&where=/s/sub/")); response = send(port, "/gateway/x?what=include&where=/s/sub/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.contains(">>><<<")); server.stop(); @@ -696,6 +718,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc } @Test + @Disabled("jetty/jetty.project/issues/10608") public void paxWebWelcomePagesWithDifferentContext() throws Exception { Server server = new Server(); ServerConnector connector = new ServerConnector(server, 1, 1, new HttpConnectionFactory()); @@ -851,8 +874,8 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se // "/" (but through gateway) - similar forward, but performed explicitly by gateway servlet // 9.4 The Forward Method: - // The path elements of the request object exposed to the target servlet must reflect the - // path used to obtain the RequestDispatcher. + // The path elements of the request object exposed to the target servlet must reflect the + // path used to obtain the RequestDispatcher. // so "gateway" forwards to "/", "/" is handled by "default" which forwards to "/index.y" response = send(port, "/c/gateway/x?what=forward&where=/"); assertTrue(response.contains("req.context_path=\"/c\"")); @@ -882,14 +905,14 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se // "/sub/" + "index.x" welcome files is forwarded and mapped to indexx servlet // According to 10.10 "Welcome Files": - // The Web server must append each welcome file in the order specified in the deployment descriptor to the - // partial request and check whether a static resource in the WAR is mapped to that - // request URI. If no match is found, the Web server MUST again append each - // welcome file in the order specified in the deployment descriptor to the partial - // request and check if a servlet is mapped to that request URI. - // [...] - // The container may send the request to the welcome resource with a forward, a redirect, or a - // container specific mechanism that is indistinguishable from a direct request. + // The Web server must append each welcome file in the order specified in the deployment descriptor to the + // partial request and check whether a static resource in the WAR is mapped to that + // request URI. If no match is found, the Web server MUST again append each + // welcome file in the order specified in the deployment descriptor to the partial + // request and check if a servlet is mapped to that request URI. + // [...] + // The container may send the request to the welcome resource with a forward, a redirect, or a + // container specific mechanism that is indistinguishable from a direct request. // Jetty detects /sub/index.y (first welcome file) can be mapped to indexx servlet, but continues the // search for physical resource. /sub/index.x is actual physical resource, so forward is chosen, which // is eventually mapped to indexx again - with index.x, not index.y @@ -927,21 +950,51 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/c/gateway/x?what=include&where=/r"); assertTrue(response.endsWith(">>><<<")); + // See https://github.com/eclipse/jetty.project/issues/9910 + // for changed welcome file handling with Jetty's own "pathInfoOnly" + // "/r" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet // forward is performed implicitly by Jetty's DefaultServlet (even if mapped to /r/*), forward URI is - // "/r/index.y" (first welcome), but this time, "/r/*" is a mapping with higher priority than "*.y" - // (with "/" servlet, "*.y" had higher priority than "/"), so "resource" servlet is called, this time - // with full URI (no welcome files are checked). Such resource is not found, so we have 404 + // "/index.y" (first welcome), which is different than in Jetty before 12, where forward was "/r/index.y". + // the reasons are explained in https://github.com/eclipse/jetty.project/issues/9910 and quick summary may + // be just this: + // - '/r/' is handled by resource servlet which may (pathInfoOnly=false) or may not (pathInfoOnly=true) + // use '/r/' prefix to find resources (with welcome file definitions support) + // - at the stage of servlet mapping with welcome files, pahInfoOnly setting was not used by Jetty before 12 + // and the forward (or redirect) was prepending the URL with servlet path + // - so in Pax Web, where we always set pathInfoOnly=true for resource (default) servlets mapped to something + // different than '/', we should always reject servlet path - during resource lookup AND during + // forward/redirect + // - and pathInfoOnly=false is set only for '/' mapped servlets where it's not changing URI construction. + // it's used only to prevent endless redirect ;) response = send(port, "/c/r/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/c/r/\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/r\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/\"")); response = send(port, "/c/gateway/x?what=forward&where=/r/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/c/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/gateway\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/x\"")); response = send(port, "/c/gateway/x?what=include&where=/r/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 + // gateway uses dispatcher for /r/ and calls include. /r/* servlet uses welcome files, so index.y servlet mapping + // is found and include target is /index.y (in Jetty before 12 it was /r/index.x which wasn't found by + // resource servlet, so should result in HTTP 500 according to 9.3 "The Include Method") + assertTrue(response.contains(">>>'indexx servlet'")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.include.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.include.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.servlet_path=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.path_info=\"null\"")); // HTTP 500 according to 9.3 "The Include Method" + response = send(port, "/c/gateway/x?what=include&where=/r/xyz"); assertTrue(response.startsWith("HTTP/1.1 500")); response = send(port, "/c/r/sub"); @@ -971,17 +1024,15 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se assertTrue(response.endsWith(">>><<<")); response = send(port, "/c/s/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.startsWith("HTTP/1.1 302")); - // redirect to first welcome page with found *.y mapping, but another mapping will be found using /s/* - assertTrue(extractHeaders(response).get("Location").endsWith("/c/s/index.y")); + // redirect to first welcome page with found *.y mapping, and with redirect NOT using '/s' servlet path + // (because pathInfoOnly=true) + assertTrue(extractHeaders(response).get("Location").endsWith("/c/index.y")); response = send(port, "/c/gateway/x?what=forward&where=/s/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.startsWith("HTTP/1.1 302")); - assertTrue(extractHeaders(response).get("Location").endsWith("/c/s/index.y?what=forward&where=/s/")); + assertTrue(extractHeaders(response).get("Location").endsWith("/c/index.y?what=forward&where=/s/")); response = send(port, "/c/gateway/x?what=include&where=/s/"); - // TODO: https://github.com/eclipse/jetty.project/issues/9910 assertTrue(response.contains(">>><<<")); response = send(port, "/c/s/sub"); diff --git a/pax-web-tomcat/src/main/java/org/ops4j/pax/web/service/tomcat/internal/web/TomcatResourceServlet.java b/pax-web-tomcat/src/main/java/org/ops4j/pax/web/service/tomcat/internal/web/TomcatResourceServlet.java index 6eca147c3b..73e51ba4fa 100644 --- a/pax-web-tomcat/src/main/java/org/ops4j/pax/web/service/tomcat/internal/web/TomcatResourceServlet.java +++ b/pax-web-tomcat/src/main/java/org/ops4j/pax/web/service/tomcat/internal/web/TomcatResourceServlet.java @@ -59,6 +59,8 @@ public class TomcatResourceServlet extends DefaultServlet { private String[] welcomeFiles; private boolean redirectWelcome = false; + + // this flag has to be set if resource/default servlet is mapped to "/", as pathInfo is null in such case private boolean pathInfoOnly = true; private OsgiServletContext highestRankedContext; @@ -208,12 +210,13 @@ protected void serveResource(HttpServletRequest request, HttpServletResponse res String resolvedWelcome = null; - // 1) physical resources (but checked for pathInfo only): + // 1) physical resources: for (String welcome : welcomeFiles) { String path = relativePath + welcome; WebResource resource = resources.getResource(path); if (resource.exists()) { // redirect/include/forward has to be done with our context + servlet path + // for physical resources, we need to add servletPath back resolvedWelcome = pathInfoOnly ? servletPath + path : path; break; } @@ -223,9 +226,7 @@ protected void serveResource(HttpServletRequest request, HttpServletResponse res RequestDispatcher dispatcher = null; if (resolvedWelcome == null) { for (String welcome : welcomeFiles) { - // path uses servlet path as well - always - not only with !pathInfoOnly, but because - // pathInfoOnly=false is set ONLY for "/" servlet, it doesn't really matter - String path = servletPath + relativePath + welcome; + String path = relativePath + welcome; dispatcher = request.getRequestDispatcher(path); if (dispatcher != null) { resolvedWelcome = path; @@ -241,7 +242,7 @@ protected void serveResource(HttpServletRequest request, HttpServletResponse res return; } String queryString = request.getQueryString(); - if (queryString != null && !"".equals(queryString)) { + if (queryString != null && !queryString.isEmpty()) { resolvedWelcome = resolvedWelcome + "?" + queryString; } resolvedWelcome = request.getContextPath() + resolvedWelcome; @@ -283,7 +284,7 @@ protected void serveResource(HttpServletRequest request, HttpServletResponse res /** *

Override {@link DefaultServlet#getRelativePath(HttpServletRequest, boolean)} to use only path info. Just * as {@link org.apache.catalina.servlets.WebdavServlet} and just as Jetty does it with {@code pathInfoOnly} - * servlet init parameter.

+ * servlet init parameter (although it has changed drammatically with Jetty 12).

* *

As with Jetty, {@code pathInfoOnly} has to be {@code false} because servlet path and path info are * confusing when servlet is mapped to "/".

@@ -322,7 +323,7 @@ protected String getRelativePath(HttpServletRequest request, boolean allowEmptyP if (childPath == null) { return null; } - if (childPath.length() > 0 && !childPath.startsWith("/")) { + if (!childPath.isEmpty() && !childPath.startsWith("/")) { childPath = "/" + childPath; } diff --git a/pax-web-tomcat/src/test/java/org/ops4j/pax/web/service/tomcat/internal/UnifiedTomcatTest.java b/pax-web-tomcat/src/test/java/org/ops4j/pax/web/service/tomcat/internal/UnifiedTomcatTest.java index 5f841b9fda..6a2dba239e 100644 --- a/pax-web-tomcat/src/test/java/org/ops4j/pax/web/service/tomcat/internal/UnifiedTomcatTest.java +++ b/pax-web-tomcat/src/test/java/org/ops4j/pax/web/service/tomcat/internal/UnifiedTomcatTest.java @@ -542,13 +542,13 @@ public void paxWebWelcomePages() throws Exception { // the "/r" resource servlet StandardWrapper resourceWrapper = new StandardWrapper(); - resourceWrapper.setName("resource"); + resourceWrapper.setName("resource1"); TomcatResourceServlet servlet2 = new TomcatResourceServlet(b2, null, null); servlet2.setWelcomeFiles(new String[] { "index.y", "index.x" }); resourceWrapper.setServlet(servlet2); resourceWrapper.addInitParameter("listings", "false"); rootContext.addChild(resourceWrapper); - rootContext.addServletMappingDecoded("/r/*", "resource"); + rootContext.addServletMappingDecoded("/r/*", "resource1"); // the "/s" resource servlet - with redirected welcome files StandardWrapper resource2Wrapper = new StandardWrapper(); @@ -632,7 +632,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se assertTrue(response.endsWith("'index-z-b1'")); // "/" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet - // forward is performed implicitly by Tomcat's DefaultServlet + // (see order of welcome files). Forward is performed implicitly by Tomcat's DefaultServlet. response = send(port, "/"); assertTrue(response.contains("req.context_path=\"\"")); assertTrue(response.contains("req.request_uri=\"/index.y\"")); @@ -724,18 +724,51 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/gateway/x?what=include&where=/r"); assertTrue(response.endsWith(">>><<<")); + // See https://github.com/eclipse/jetty.project/issues/9910 + // for changed welcome file handling with Jetty's own "pathInfoOnly" + // "/r" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet // forward is performed implicitly by Tomcat's DefaultServlet (even if mapped to /r/*), forward URI is - // "/r/index.y" (first welcome), but this time, "/r/*" is a mapping with higher priority than "*.y" - // (with "/" servlet, "*.y" had higher priority than "/"), so "resource" servlet is called, this time - // with full URI (no welcome files are checked). Such resource is not found, so we have 404 + // "/index.y" (first welcome), which is different than in Jetty before 12, where forward was "/r/index.y". + // the reasons are explained in https://github.com/eclipse/jetty.project/issues/9910 and quick summary may + // be just this: + // - '/r/' is handled by resource servlet which may (pathInfoOnly=false) or may not (pathInfoOnly=true) + // use '/r/' prefix to find resources (with welcome file definitions support) + // - at the stage of servlet mapping with welcome files, pahInfoOnly setting was not used by Jetty before 12 + // and the forward (or redirect) was prepending the URL with servlet path + // - so in Pax Web, where we always set pathInfoOnly=true for resource (default) servlets mapped to something + // different than '/', we should always reject servlet path - during resource lookup AND during + // forward/redirect + // - and pathInfoOnly=false is set only for '/' mapped servlets where it's not changing URI construction. + // it's used only to prevent endless redirect ;) response = send(port, "/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/r/\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/r\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/\"")); response = send(port, "/gateway/x?what=forward&where=/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/gateway\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/x\"")); response = send(port, "/gateway/x?what=include&where=/r/"); + // gateway uses dispatcher for /r/ and calls include. /r/* servlet uses welcome files, so index.y servlet mapping + // is found and include target is /index.y (in Jetty before 12 it was /r/index.x which wasn't found by + // resource servlet, so should result in HTTP 500 according to 9.3 "The Include Method") + assertTrue(response.contains(">>>'indexx servlet'")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.include.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.include.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.servlet_path=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.path_info=\"null\"")); // HTTP 500 according to 9.3 "The Include Method" + response = send(port, "/gateway/x?what=include&where=/r/xyz"); assertTrue(response.startsWith("HTTP/1.1 500")); response = send(port, "/r/sub"); @@ -766,12 +799,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - // redirect to first welcome page with found *.y mapping, but another mapping will be found using /s/* - assertTrue(extractHeaders(response).get("Location").endsWith("/s/index.y")); + // redirect to first welcome page with found *.y mapping, and with redirect NOT using '/s' servlet path + // (because pathInfoOnly=true) + assertTrue(extractHeaders(response).get("Location").endsWith("/index.y")); response = send(port, "/gateway/x?what=forward&where=/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - assertTrue(extractHeaders(response).get("Location").endsWith("/s/index.y?what=forward&where=/s/")); + assertTrue(extractHeaders(response).get("Location").endsWith("/index.y?what=forward&where=/s/")); response = send(port, "/gateway/x?what=include&where=/s/"); assertTrue(response.contains(">>><<<")); @@ -966,7 +1000,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se assertTrue(response.endsWith("'index-z-b1'")); // "/" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet - // forward is performed implicitly by Tomcat's DefaultServlet + // (see order of welcome files). Forward is performed implicitly by Tomcat's DefaultServlet. response = send(port, "/c/"); assertTrue(response.contains("req.context_path=\"/c\"")); assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); @@ -1058,18 +1092,51 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/c/gateway/x?what=include&where=/r"); assertTrue(response.endsWith(">>><<<")); + // See https://github.com/eclipse/jetty.project/issues/9910 + // for changed welcome file handling with Jetty's own "pathInfoOnly" + // "/r" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet // forward is performed implicitly by Tomcat's DefaultServlet (even if mapped to /r/*), forward URI is - // "/r/index.y" (first welcome), but this time, "/r/*" is a mapping with higher priority than "*.y" - // (with "/" servlet, "*.y" had higher priority than "/"), so "resource" servlet is called, this time - // with full URI (no welcome files are checked). Such resource is not found, so we have 404 + // "/index.y" (first welcome), which is different than in Jetty before 12, where forward was "/r/index.y". + // the reasons are explained in https://github.com/eclipse/jetty.project/issues/9910 and quick summary may + // be just this: + // - '/r/' is handled by resource servlet which may (pathInfoOnly=false) or may not (pathInfoOnly=true) + // use '/r/' prefix to find resources (with welcome file definitions support) + // - at the stage of servlet mapping with welcome files, pahInfoOnly setting was not used by Jetty before 12 + // and the forward (or redirect) was prepending the URL with servlet path + // - so in Pax Web, where we always set pathInfoOnly=true for resource (default) servlets mapped to something + // different than '/', we should always reject servlet path - during resource lookup AND during + // forward/redirect + // - and pathInfoOnly=false is set only for '/' mapped servlets where it's not changing URI construction. + // it's used only to prevent endless redirect ;) response = send(port, "/c/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/c/r/\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/r\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/\"")); response = send(port, "/c/gateway/x?what=forward&where=/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/c/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/gateway\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/x\"")); response = send(port, "/c/gateway/x?what=include&where=/r/"); + // gateway uses dispatcher for /r/ and calls include. /r/* servlet uses welcome files, so index.y servlet mapping + // is found and include target is /index.y (in Jetty before 12 it was /r/index.x which wasn't found by + // resource servlet, so should result in HTTP 500 according to 9.3 "The Include Method") + assertTrue(response.contains(">>>'indexx servlet'")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.include.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.include.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.servlet_path=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.path_info=\"null\"")); // HTTP 500 according to 9.3 "The Include Method" + response = send(port, "/c/gateway/x?what=include&where=/r/xyz"); assertTrue(response.startsWith("HTTP/1.1 500")); response = send(port, "/c/r/sub"); @@ -1100,12 +1167,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/c/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - // redirect to first welcome page with found *.y mapping, but another mapping will be found using /s/* - assertTrue(extractHeaders(response).get("Location").endsWith("/c/s/index.y")); + // redirect to first welcome page with found *.y mapping, and with redirect NOT using '/s' servlet path + // (because pathInfoOnly=true) + assertTrue(extractHeaders(response).get("Location").endsWith("/c/index.y")); response = send(port, "/c/gateway/x?what=forward&where=/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - assertTrue(extractHeaders(response).get("Location").endsWith("/c/s/index.y?what=forward&where=/s/")); + assertTrue(extractHeaders(response).get("Location").endsWith("/c/index.y?what=forward&where=/s/")); response = send(port, "/c/gateway/x?what=include&where=/s/"); assertTrue(response.contains(">>><<<")); diff --git a/pax-web-undertow/src/main/java/org/ops4j/pax/web/service/undertow/internal/web/UndertowResourceServlet.java b/pax-web-undertow/src/main/java/org/ops4j/pax/web/service/undertow/internal/web/UndertowResourceServlet.java index 68b60d8b94..8c7f2f0c4c 100644 --- a/pax-web-undertow/src/main/java/org/ops4j/pax/web/service/undertow/internal/web/UndertowResourceServlet.java +++ b/pax-web-undertow/src/main/java/org/ops4j/pax/web/service/undertow/internal/web/UndertowResourceServlet.java @@ -74,6 +74,7 @@ public class UndertowResourceServlet extends DefaultServlet implements ResourceM private String[] welcomeFiles; private boolean redirectWelcome = false; + // this flag has to be set if resource/default servlet is mapped to "/", as pathInfo is null in such case private boolean pathInfoOnly = true; private boolean cacheConfigurable = false; @@ -261,12 +262,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se HttpServerExchange exchange = requireCurrentServletRequestContext().getOriginalRequest().getExchange(); String resolvedWelcome = null; - // 1) physical resources (but checked for pathInfo only): + // 1) physical resources: for (String welcome : welcomeFiles) { String path = relativePath + welcome; Resource resource = resourceSupplier.getResource(exchange, path); if (resource != null) { // redirect/include/forward has to be done with our context + servlet path + // for physical resources, we need to add servletPath back resolvedWelcome = pathInfoOnly ? servletPath + path : path; break; } @@ -276,8 +278,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se RequestDispatcher dispatcher = null; if (resolvedWelcome == null) { for (String welcome : welcomeFiles) { - // path uses servlet path as well - String path = servletPath + relativePath + welcome; + String path = relativePath + welcome; dispatcher = req.getRequestDispatcher(path); if (dispatcher != null) { resolvedWelcome = path; diff --git a/pax-web-undertow/src/test/java/org/ops4j/pax/web/service/undertow/internal/UnifiedUndertowTest.java b/pax-web-undertow/src/test/java/org/ops4j/pax/web/service/undertow/internal/UnifiedUndertowTest.java index 7f258dcac7..0ecd72b26c 100644 --- a/pax-web-undertow/src/test/java/org/ops4j/pax/web/service/undertow/internal/UnifiedUndertowTest.java +++ b/pax-web-undertow/src/test/java/org/ops4j/pax/web/service/undertow/internal/UnifiedUndertowTest.java @@ -545,7 +545,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se assertTrue(response.endsWith("'index-z-b1'")); // "/" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet - // forward is performed implicitly by Undertow's DefaultServlet + // (see order of welcome files). Forward is performed implicitly by Undertow's DefaultServlet. response = send(port, "/"); assertTrue(response.contains("req.context_path=\"\"")); assertTrue(response.contains("req.request_uri=\"/index.y\"")); @@ -637,18 +637,51 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/gateway/x?what=include&where=/r"); assertTrue(response.endsWith(">>><<<")); + // See https://github.com/eclipse/jetty.project/issues/9910 + // for changed welcome file handling with Jetty's own "pathInfoOnly" + // "/r" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet // forward is performed implicitly by Undertow's DefaultServlet (even if mapped to /r/*), forward URI is - // "/r/index.y" (first welcome), but this time, "/r/*" is a mapping with higher priority than "*.y" - // (with "/" servlet, "*.y" had higher priority than "/"), so "resource" servlet is called, this time - // with full URI (no welcome files are checked). Such resource is not found, so we have 404 + // "/index.y" (first welcome), which is different than in Jetty before 12, where forward was "/r/index.y". + // the reasons are explained in https://github.com/eclipse/jetty.project/issues/9910 and quick summary may + // be just this: + // - '/r/' is handled by resource servlet which may (pathInfoOnly=false) or may not (pathInfoOnly=true) + // use '/r/' prefix to find resources (with welcome file definitions support) + // - at the stage of servlet mapping with welcome files, pahInfoOnly setting was not used by Jetty before 12 + // and the forward (or redirect) was prepending the URL with servlet path + // - so in Pax Web, where we always set pathInfoOnly=true for resource (default) servlets mapped to something + // different than '/', we should always reject servlet path - during resource lookup AND during + // forward/redirect + // - and pathInfoOnly=false is set only for '/' mapped servlets where it's not changing URI construction. + // it's used only to prevent endless redirect ;) response = send(port, "/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/r/\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/r\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/\"")); response = send(port, "/gateway/x?what=forward&where=/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/gateway\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/x\"")); response = send(port, "/gateway/x?what=include&where=/r/"); + // gateway uses dispatcher for /r/ and calls include. /r/* servlet uses welcome files, so index.y servlet mapping + // is found and include target is /index.y (in Jetty before 12 it was /r/index.x which wasn't found by + // resource servlet, so should result in HTTP 500 according to 9.3 "The Include Method") + assertTrue(response.contains(">>>'indexx servlet'")); + assertTrue(response.contains("req.context_path=\"\"")); + assertTrue(response.contains("req.request_uri=\"/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.include.context_path=\"\"")); + assertTrue(response.contains("jakarta.servlet.include.request_uri=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.servlet_path=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.path_info=\"null\"")); // HTTP 500 according to 9.3 "The Include Method" + response = send(port, "/gateway/x?what=include&where=/r/xyz"); assertTrue(response.startsWith("HTTP/1.1 500")); response = send(port, "/r/sub"); @@ -679,12 +712,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - // redirect to first welcome page with found *.y mapping, but another mapping will be found using /s/* - assertTrue(extractHeaders(response).get("Location").endsWith("/s/index.y")); + // redirect to first welcome page with found *.y mapping, and with redirect NOT using '/s' servlet path + // (because pathInfoOnly=true) + assertTrue(extractHeaders(response).get("Location").endsWith("/index.y")); response = send(port, "/gateway/x?what=forward&where=/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - assertTrue(extractHeaders(response).get("Location").endsWith("/s/index.y?what=forward&where=/s/")); + assertTrue(extractHeaders(response).get("Location").endsWith("/index.y?what=forward&where=/s/")); response = send(port, "/gateway/x?what=include&where=/s/"); assertTrue(response.contains(">>><<<")); @@ -954,18 +988,51 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/c/gateway/x?what=include&where=/r"); assertTrue(response.endsWith(">>><<<")); + // See https://github.com/eclipse/jetty.project/issues/9910 + // for changed welcome file handling with Jetty's own "pathInfoOnly" + // "/r" - no "/index.x" or "/index.y" physical resource, but existing mapping for *.y to indexx servlet // forward is performed implicitly by Undertow's DefaultServlet (even if mapped to /r/*), forward URI is - // "/r/index.y" (first welcome), but this time, "/r/*" is a mapping with higher priority than "*.y" - // (with "/" servlet, "*.y" had higher priority than "/"), so "resource" servlet is called, this time - // with full URI (no welcome files are checked). Such resource is not found, so we have 404 + // "/index.y" (first welcome), which is different than in Jetty before 12, where forward was "/r/index.y". + // the reasons are explained in https://github.com/eclipse/jetty.project/issues/9910 and quick summary may + // be just this: + // - '/r/' is handled by resource servlet which may (pathInfoOnly=false) or may not (pathInfoOnly=true) + // use '/r/' prefix to find resources (with welcome file definitions support) + // - at the stage of servlet mapping with welcome files, pahInfoOnly setting was not used by Jetty before 12 + // and the forward (or redirect) was prepending the URL with servlet path + // - so in Pax Web, where we always set pathInfoOnly=true for resource (default) servlets mapped to something + // different than '/', we should always reject servlet path - during resource lookup AND during + // forward/redirect + // - and pathInfoOnly=false is set only for '/' mapped servlets where it's not changing URI construction. + // it's used only to prevent endless redirect ;) response = send(port, "/c/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/c/r/\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/r\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/\"")); response = send(port, "/c/gateway/x?what=forward&where=/r/"); - assertTrue(response.startsWith("HTTP/1.1 404")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.forward.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.forward.request_uri=\"/c/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.forward.servlet_path=\"/gateway\"")); + assertTrue(response.contains("jakarta.servlet.forward.path_info=\"/x\"")); response = send(port, "/c/gateway/x?what=include&where=/r/"); + // gateway uses dispatcher for /r/ and calls include. /r/* servlet uses welcome files, so index.y servlet mapping + // is found and include target is /index.y (in Jetty before 12 it was /r/index.x which wasn't found by + // resource servlet, so should result in HTTP 500 according to 9.3 "The Include Method") + assertTrue(response.contains(">>>'indexx servlet'")); + assertTrue(response.contains("req.context_path=\"/c\"")); + assertTrue(response.contains("req.request_uri=\"/c/gateway/x\"")); + assertTrue(response.contains("jakarta.servlet.include.context_path=\"/c\"")); + assertTrue(response.contains("jakarta.servlet.include.request_uri=\"/c/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.servlet_path=\"/index.y\"")); + assertTrue(response.contains("jakarta.servlet.include.path_info=\"null\"")); // HTTP 500 according to 9.3 "The Include Method" + response = send(port, "/c/gateway/x?what=include&where=/r/xyz"); assertTrue(response.startsWith("HTTP/1.1 500")); response = send(port, "/c/r/sub"); @@ -996,12 +1063,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se response = send(port, "/c/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - // redirect to first welcome page with found *.y mapping, but another mapping will be found using /s/* - assertTrue(extractHeaders(response).get("Location").endsWith("/c/s/index.y")); + // redirect to first welcome page with found *.y mapping, and with redirect NOT using '/s' servlet path + // (because pathInfoOnly=true) + assertTrue(extractHeaders(response).get("Location").endsWith("/c/index.y")); response = send(port, "/c/gateway/x?what=forward&where=/s/"); assertTrue(response.startsWith("HTTP/1.1 302")); - assertTrue(extractHeaders(response).get("Location").endsWith("/c/s/index.y?what=forward&where=/s/")); + assertTrue(extractHeaders(response).get("Location").endsWith("/c/index.y?what=forward&where=/s/")); response = send(port, "/c/gateway/x?what=include&where=/s/"); assertTrue(response.contains(">>><<<"));