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.
This commit is contained in:
Simone Bordet 2015-12-08 22:10:27 +01:00
parent 657b570716
commit 1693dd135d
5 changed files with 82 additions and 31 deletions

View File

@ -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);
}
}

View File

@ -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

View File

@ -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

View File

@ -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
{

View File

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