From 657b570716b80cc7d62adfb7a45ac7782de08572 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 8 Dec 2015 21:57:04 +0100 Subject: 482243 Fixed GzipHandler for Include. Because AbstractCompressedStream is a jetty-servlets class that can be included in web applications, it cannot reference server classes such as Response. The check for inclusion is now done by looking at the dispatcher type. --- .../servlets/gzip/AbstractCompressedStream.java | 23 +++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/AbstractCompressedStream.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/AbstractCompressedStream.java index 3110285a2a..44c02fbdbf 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/AbstractCompressedStream.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/AbstractCompressedStream.java @@ -24,6 +24,7 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; +import javax.servlet.DispatcherType; import javax.servlet.ServletOutputStream; import javax.servlet.WriteListener; import javax.servlet.http.HttpServletRequest; @@ -42,6 +43,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream { private final String _encoding; protected final String _vary; + private final HttpServletRequest _request; protected final CompressedResponseWrapper _wrapper; protected final HttpServletResponse _response; protected OutputStream _out; @@ -58,10 +60,11 @@ public abstract class AbstractCompressedStream extends ServletOutputStream throws IOException { _encoding=encoding; + _request = request; _wrapper = wrapper; _response = (HttpServletResponse)wrapper.getResponse(); _vary=vary; - + if (_wrapper.getMinCompressSize()==0) doCompress(); } @@ -90,7 +93,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream _bOut=b; } } - + /* ------------------------------------------------------------ */ public void setContentLength() { @@ -241,9 +244,11 @@ public abstract class AbstractCompressedStream extends ServletOutputStream if (_encoding!=null) { - String prefix=Response.getResponse(_response).isIncluding()?Response.SET_INCLUDE_HEADER_PREFIX:""; - + String prefix = ""; + if (_request.getDispatcherType() == DispatcherType.INCLUDE) + prefix = Response.SET_INCLUDE_HEADER_PREFIX; setHeader(prefix+"Content-Encoding", _encoding); + if (_response.containsHeader("Content-Encoding")) { addHeader(prefix+"Vary",_vary); @@ -269,7 +274,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream } } } - + doNotCompress(true); // Send vary as it could have been compressed if encoding was present } } @@ -290,7 +295,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream addHeader("Vary",_vary); if (_wrapper.getETag()!=null) setHeader("ETag",_wrapper.getETag()); - + _doNotCompress = true; _out = _response.getOutputStream(); @@ -316,7 +321,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream throw new IOException("CLOSED"); if (_out == null) - { + { // If this first write is larger than buffer size, then we are committing now if (lengthToWrite>_wrapper.getBufferSize()) { @@ -336,7 +341,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream // else are we aggregating writes? else if (_bOut !=null) { - // We are aggregating into the buffered output stream. + // We are aggregating into the buffered output stream. // If this write fills the buffer, then we are committing if (lengthToWrite>=(_bOut.getBuf().length - _bOut.getCount())) @@ -384,7 +389,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream { throw new UnsupportedOperationException("Use AsyncGzipFilter"); } - + @Override public boolean isReady() -- cgit v1.2.3 From 1693dd135d70b892da51a6a810e1a7379a64067e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 8 Dec 2015 22:10:27 +0100 Subject: 483857 - jetty-client onComplete isn't called in case of exception in GZIPContentDecoder. Fixed by catching the exceptions and failing the callbacks. Also using return values from HttpReceiver to compute what to return to the parser. --- .../org/eclipse/jetty/client/HttpReceiver.java | 41 +++++++++++++--------- .../jetty/client/http/HttpReceiverOverHTTP.java | 22 ++++++------ .../eclipse/jetty/client/HttpClientGZIPTest.java | 35 ++++++++++++++++++ .../fcgi/client/http/HttpConnectionOverFCGI.java | 7 ++-- .../eclipse/jetty/util/CompletableCallback.java | 8 +++++ 5 files changed, 82 insertions(+), 31 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index deb6069e52..6c63293275 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -323,27 +323,34 @@ public abstract class HttpReceiver } else { - List decodeds = new ArrayList<>(2); - while (buffer.hasRemaining()) + try { - ByteBuffer decoded = decoder.decode(buffer); - if (!decoded.hasRemaining()) - continue; - decodeds.add(decoded); - if (LOG.isDebugEnabled()) - LOG.debug("Response content decoded ({}) {}{}{}", decoder, response, System.lineSeparator(), BufferUtil.toDetailString(decoded)); - } + List decodeds = new ArrayList<>(2); + while (buffer.hasRemaining()) + { + ByteBuffer decoded = decoder.decode(buffer); + if (!decoded.hasRemaining()) + continue; + decodeds.add(decoded); + if (LOG.isDebugEnabled()) + LOG.debug("Response content decoded ({}) {}{}{}", decoder, response, System.lineSeparator(), BufferUtil.toDetailString(decoded)); + } - if (decodeds.isEmpty()) - { - callback.succeeded(); + if (decodeds.isEmpty()) + { + callback.succeeded(); + } + else + { + int size = decodeds.size(); + CountingCallback counter = new CountingCallback(callback, size); + for (int i = 0; i < size; ++i) + notifier.notifyContent(listeners, response, decodeds.get(i), counter); + } } - else + catch (Throwable x) { - int size = decodeds.size(); - CountingCallback counter = new CountingCallback(callback, size); - for (int i = 0; i < size; ++i) - notifier.notifyContent(listeners, response, decodeds.get(i), counter); + callback.failed(x); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java index d4d037844b..02b6c37f0f 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java @@ -205,8 +205,7 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res parser.setHeadResponse(HttpMethod.HEAD.is(method) || HttpMethod.CONNECT.is(method)); exchange.getResponse().version(version).status(status).reason(reason); - responseBegin(exchange); - return false; + return !responseBegin(exchange); } @Override @@ -216,8 +215,7 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res if (exchange == null) return false; - responseHeader(exchange, field); - return false; + return !responseHeader(exchange, field); } @Override @@ -227,8 +225,7 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res if (exchange == null) return false; - responseHeaders(exchange); - return false; + return !responseHeaders(exchange); } @Override @@ -253,17 +250,20 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res failAndClose(x); } }; - responseContent(exchange, buffer, callback); - return callback.tryComplete(); + // Do not short circuit these calls. + boolean proceed = responseContent(exchange, buffer, callback); + boolean async = callback.tryComplete(); + return !proceed || async; } @Override public boolean messageComplete() { HttpExchange exchange = getHttpExchange(); - if (exchange != null) - responseSuccess(exchange); - return false; + if (exchange == null) + return false; + + return !responseSuccess(exchange); } @Override diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java index bd45bb8201..d348c6cea3 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java @@ -22,14 +22,18 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InterruptedIOException; import java.util.Arrays; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPOutputStream; + import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -195,6 +199,37 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest Assert.assertArrayEquals(data, response.getContent()); } + @Test + public void testGZIPContentCorrupted() throws Exception + { + start(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setHeader("Content-Encoding", "gzip"); + // Not gzipped, will cause the client to blow up. + response.getOutputStream().print("0123456789"); + } + }); + + final CountDownLatch latch = new CountDownLatch(1); + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .send(new Response.CompleteListener() + { + @Override + public void onComplete(Result result) + { + if (result.isFailed()) + latch.countDown(); + } + }); + + Assert.assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + private static void sleep(long ms) throws IOException { try diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java index 8d7c224da9..e68999c4c3 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java @@ -377,9 +377,10 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec close(x); } }; - if (!channel.content(buffer, callback)) - return true; - return callback.tryComplete(); + // Do not short circuit these calls. + boolean proceed = channel.content(buffer, callback); + boolean async = callback.tryComplete(); + return !proceed || async; } else { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableCallback.java b/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableCallback.java index 1020bb6267..5c401341dc 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableCallback.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableCallback.java @@ -82,6 +82,10 @@ public abstract class CompletableCallback implements Callback } break; } + case FAILED: + { + return; + } default: { throw new IllegalStateException(current.toString()); @@ -108,6 +112,10 @@ public abstract class CompletableCallback implements Callback } break; } + case FAILED: + { + return; + } default: { throw new IllegalStateException(current.toString()); -- cgit v1.2.3