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.

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<ByteBuffer> 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<ByteBuffer> 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());