diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java index bc8fdbd65da..baa596aedf1 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java @@ -219,8 +219,18 @@ public class Dispatcher implements RequestDispatcher _contextHandler.handle(_pathInContext, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); - if (!baseRequest.getHttpChannelState().isAsync()) - baseRequest.getResponse().softClose(); + // If we are not async and not closed already, then close via the possibly wrapped response. + if (!baseRequest.getHttpChannelState().isAsync() && !baseResponse.getHttpOutput().isClosed()) + { + try + { + response.getOutputStream().close(); + } + catch (IllegalStateException e) + { + response.getWriter().close(); + } + } } } finally diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index b1d05bd587d..8c7cf77a87b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -905,7 +905,7 @@ public class HttpChannelState throw new IllegalStateException("Response is " + _outputState); response.setStatus(code); - response.softClose(); + response.errorClose(); request.setAttribute(ErrorHandler.ERROR_CONTEXT, request.getErrorContext()); request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI()); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index af92ecd60f9..cea9f830697 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -141,12 +141,14 @@ public class Response implements HttpServletResponse public void reopen() { + // Make the response mutable and reopen output. setErrorSent(false); _out.reopen(); } - public void softClose() + public void errorClose() { + // Make the response immutable and soft close the output. setErrorSent(true); _out.softClose(); } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java index cee4e52bd40..a6202ae2fdc 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java @@ -18,7 +18,9 @@ package org.eclipse.jetty.servlet; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; @@ -32,10 +34,12 @@ import javax.servlet.RequestDispatcher; import javax.servlet.Servlet; import javax.servlet.ServletContext; import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; import javax.servlet.ServletRequest; import javax.servlet.ServletRequestWrapper; import javax.servlet.ServletResponse; import javax.servlet.ServletResponseWrapper; +import javax.servlet.WriteListener; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; @@ -114,8 +118,9 @@ public class DispatcherTest String expected = "HTTP/1.1 200 OK\r\n" + "Content-Type: text/html\r\n" + - "Content-Length: 0\r\n" + - "\r\n"; + "Content-Length: 7\r\n" + + "\r\n" + + "FORWARD"; String responses = _connector.getResponse("GET /context/ForwardServlet?do=assertforward&do=more&test=1 HTTP/1.0\n\n"); @@ -131,8 +136,9 @@ public class DispatcherTest String expected = "HTTP/1.1 200 OK\r\n" + "Content-Type: text/html\r\n" + - "Content-Length: 0\r\n" + - "\r\n"; + "Content-Length: 7\r\n" + + "\r\n" + + "FORWARD"; String responses = _connector.getResponse("GET /context/ForwardServlet?do=assertforward&foreign=%d2%e5%ec%ef%e5%f0%e0%f2%f3%f0%e0&test=1 HTTP/1.0\n\n"); assertEquals(expected, responses); @@ -201,7 +207,9 @@ public class DispatcherTest String expected = "HTTP/1.1 200 OK\r\n" + - "\r\n"; + "Content-Length: 7\r\n" + + "\r\n" + + "INCLUDE"; String responses = _connector.getResponse("GET /context/IncludeServlet?do=assertinclude&do=more&test=1 HTTP/1.0\n\n"); @@ -217,7 +225,9 @@ public class DispatcherTest String expected = "HTTP/1.1 200 OK\r\n" + - "\r\n"; + "Content-Length: 7\r\n" + + "\r\n" + + "INCLUDE"; String responses = _connector.getResponse("GET /context/ForwardServlet/forwardpath?do=include HTTP/1.0\n\n"); @@ -233,7 +243,9 @@ public class DispatcherTest String expected = "HTTP/1.1 200 OK\r\n" + - "\r\n"; + "Content-Length: 7\r\n" + + "\r\n" + + "FORWARD"; String responses = _connector.getResponse("GET /context/IncludeServlet/includepath?do=forward HTTP/1.0\n\n"); @@ -373,6 +385,88 @@ public class DispatcherTest assertThat(rechoResponse, containsString("txeTohce")); } + @Test + public void testWrappedForwardCloseIntercepted() throws Exception + { + // Add filter that wraps response, intercepts close and writes after doChain + _contextHandler.addFilter(WrappingFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)); + testForward(); + } + + public static class WrappingFilter implements Filter + { + @Override + public void init(FilterConfig filterConfig) throws ServletException + { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + { + ResponseWrapper wrapper = new ResponseWrapper((HttpServletResponse)response); + chain.doFilter(request, wrapper); + wrapper.sendResponse(response.getOutputStream()); + } + + @Override + public void destroy() + { + } + } + + public static class ResponseWrapper extends HttpServletResponseWrapper + { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + + public ResponseWrapper(HttpServletResponse response) + { + super(response); + } + + @Override + public ServletOutputStream getOutputStream() throws IOException + { + return new ServletOutputStream() + { + @Override + public boolean isReady() + { + return true; + } + + @Override + public void setWriteListener(WriteListener writeListener) + { + throw new UnsupportedOperationException(); + } + + @Override + public void write(int b) throws IOException + { + buffer.write(b); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException + { + buffer.write(b, off, len); + } + + @Override + public void close() throws IOException + { + buffer.close(); + } + }; + } + + public void sendResponse(OutputStream out) throws IOException + { + out.write(buffer.toByteArray()); + out.close(); + } + } + public static class ForwardServlet extends HttpServlet implements Servlet { @Override @@ -648,6 +742,7 @@ public class DispatcherTest response.setContentType("text/html"); response.setStatus(HttpServletResponse.SC_OK); + response.getOutputStream().print(request.getDispatcherType().toString()); } } @@ -696,6 +791,7 @@ public class DispatcherTest response.setContentType("text/html"); response.setStatus(HttpServletResponse.SC_OK); + response.getOutputStream().print(request.getDispatcherType().toString()); } } @@ -724,6 +820,7 @@ public class DispatcherTest response.setContentType("text/html"); response.setStatus(HttpServletResponse.SC_OK); + response.getOutputStream().print(request.getDispatcherType().toString()); } } @@ -761,6 +858,7 @@ public class DispatcherTest response.setContentType("text/html"); response.setStatus(HttpServletResponse.SC_OK); + response.getOutputStream().print(request.getDispatcherType().toString()); } } @@ -796,6 +894,7 @@ public class DispatcherTest response.setContentType("text/html"); response.setStatus(HttpServletResponse.SC_OK); + response.getOutputStream().print(request.getDispatcherType().toString()); } } }