From 94c14240294806f4e3e28b37e8f0f0b8cd9390e4 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 15 Aug 2016 19:37:36 +1000 Subject: [PATCH] Issue #841 support reset in buffering interceptors Added resetBuffers to the output interceptor. --- .../org/eclipse/jetty/server/HttpChannel.java | 7 ++ .../org/eclipse/jetty/server/HttpOutput.java | 66 +++++++++++++++++-- .../org/eclipse/jetty/server/Response.java | 8 +-- .../handler/BufferedResponseHandler.java | 8 +++ .../handler/BufferedResponseHandlerTest.java | 29 ++++++-- 5 files changed, 104 insertions(+), 14 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index d687f8454bc..05196e33a6a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -764,6 +764,13 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor _written+=BufferUtil.length(content); sendResponse(null,content,complete,callback); } + + @Override + public void resetBuffer() + { + if(isCommitted()) + throw new IllegalStateException("Committed"); + } public HttpOutput.Interceptor getNextInterceptor() { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 56582f8e580..1f83ee1623c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -54,12 +54,66 @@ import org.eclipse.jetty.util.log.Logger; * close the stream, to be reopened after the inclusion ends.

*/ public class HttpOutput extends ServletOutputStream implements Runnable -{ +{ + + /** + * The HttpOutput.Inteceptor is a single intercept point for all + * output written to the HttpOutput: via writer; via output stream; + * asynchronously; or blocking. + *

+ * The Interceptor can be used to implement translations (eg Gzip) or + * additional buffering that acts on all output. Interceptors are + * created in a chain, so that multiple concerns may intercept. + *

+ * The {@link HttpChannel} is an {@link Interceptor} and is always the + * last link in any Interceptor chain. + *

+ * Responses are committed by the first call to + * {@link #write(ByteBuffer, boolean, Callback)} + * and closed by a call to {@link #write(ByteBuffer, boolean, Callback)} + * with the last boolean set true. If no content is available to commit + * or close, then a null buffer is passed. + */ public interface Interceptor { - void write(ByteBuffer content, boolean complete, Callback callback); + /** + * Write content. + * The response is committed by the first call to write and is closed by + * a call with last == true. Empty content buffers may be passed to + * force a commit or close. + * @param content The content to be written or an empty buffer. + * @param last True if this is the last call to write + * @param callback The callback to use to indicate {@link Callback#succeeded()} + * or {@link Callback#failed(Throwable)}. + */ + void write(ByteBuffer content, boolean last, Callback callback); + + /** + * @return The next Interceptor in the chain or null if this is the + * last Interceptor in the chain. + */ Interceptor getNextInterceptor(); + + /** + * @return True if the Interceptor is optimized to receive direct + * {@link ByteBuffer}s in the {@link #write(ByteBuffer, boolean, Callback)} + * method. If false is returned, then passing direct buffers may cause + * inefficiencies. + */ boolean isOptimizedForDirectBuffers(); + + /** + * Reset the buffers. + *

If the Interceptor contains buffers then reset them. + * @throws IllegalStateException Thrown if the response has been + * committed and buffers and/or headers cannot be reset. + */ + default void resetBuffer() throws IllegalStateException + { + Interceptor next = getNextInterceptor(); + if (next!=null) + next.resetBuffer(); + }; } private static Logger LOG = Log.getLogger(HttpOutput.class); @@ -820,15 +874,19 @@ public class HttpOutput extends ServletOutputStream implements Runnable public void recycle() { - resetBuffer(); _interceptor=_channel; + if (BufferUtil.hasContent(_aggregate)) + BufferUtil.clear(_aggregate); + _written = 0; + reopen(); } public void resetBuffer() { - _written = 0; + _interceptor.resetBuffer(); if (BufferUtil.hasContent(_aggregate)) BufferUtil.clear(_aggregate); + _written = 0; reopen(); } 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 d4f06af9aed..9b1acc61aa0 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 @@ -543,8 +543,9 @@ public class Response implements HttpServletResponse if (isCommitted()) LOG.warn("cannot sendError("+code+", "+message+") response already committed"); - - resetBuffer(); + else + resetBuffer(); + _characterEncoding=null; setHeader(HttpHeader.EXPIRES,null); setHeader(HttpHeader.LAST_MODIFIED,null); @@ -1255,9 +1256,6 @@ public class Response implements HttpServletResponse @Override public void resetBuffer() { - if (isCommitted()) - throw new IllegalStateException("cannot reset buffer on committed response"); - _out.resetBuffer(); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java index 9a68cae0be4..19cccb6326a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java @@ -201,6 +201,14 @@ public class BufferedResponseHandler extends HandlerWrapper _next=interceptor; _channel=httpChannel; } + + @Override + public void resetBuffer() + { + _buffers.clear(); + _aggregating=null; + _aggregate=null; + }; @Override public void write(ByteBuffer content, boolean last, Callback callback) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java index 4d6721e522d..70b18439702 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java @@ -41,8 +41,6 @@ import org.junit.Test; /** * Resource Handler test - * - * TODO: increase the testing going on here */ public class BufferedResponseHandlerTest { @@ -95,6 +93,7 @@ public class BufferedResponseHandlerTest _test._writes=10; _test._flush=false; _test._close=false; + _test._reset=false; } @Test @@ -218,6 +217,18 @@ public class BufferedResponseHandlerTest assertThat(response,not(containsString("Write: 1"))); assertThat(response,containsString("Written: true")); } + + @Test + public void testReset() throws Exception + { + _test._reset=true; + String response = _local.getResponse("GET /ctx/include/path HTTP/1.1\r\nHost: localhost\r\n\r\n"); + assertThat(response,containsString(" 200 OK")); + assertThat(response,containsString("Write: 0")); + assertThat(response,containsString("Write: 9")); + assertThat(response,containsString("Written: true")); + assertThat(response,not(containsString("RESET"))); + } public static class TestHandler extends AbstractHandler { @@ -227,16 +238,25 @@ public class BufferedResponseHandlerTest int _writes; boolean _flush; boolean _close; + boolean _reset; @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { baseRequest.setHandled(true); + if (_bufferSize>0) response.setBufferSize(_bufferSize); if (_mimeType!=null) response.setContentType(_mimeType); - + + if (_reset) + { + response.getOutputStream().print("THIS WILL BE RESET"); + response.getOutputStream().flush(); + response.getOutputStream().print("THIS WILL BE RESET"); + response.resetBuffer(); + } for (int i=0;i<_writes;i++) { response.addHeader("Write",Integer.toString(i)); @@ -248,7 +268,6 @@ public class BufferedResponseHandlerTest if (_close) response.getOutputStream().close(); response.addHeader("Written","true"); - } - + } } }