From c88aba6587c0d81ec354409820e50945cd7ad7b2 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 18 Aug 2020 12:18:20 +0200 Subject: [PATCH] Fixes #5152 - HttpClient should handle unsolicited responses. Now closing the connection if an unsolicited response is detected, no matter what response status code, or whether it has a Connection: close header, or whether it's just random bytes from the server, and also no matter whether the client read -1. Signed-off-by: Simone Bordet --- .../client/http/HttpReceiverOverHTTP.java | 28 +++++++--- .../eclipse/jetty/client/HttpClientTest.java | 55 +++++++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) 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 b6350461cd27..9dae09c1d51d 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 @@ -46,6 +46,7 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res private RetainableByteBuffer networkBuffer; private boolean shutdown; private boolean complete; + private boolean unsolicited; public HttpReceiverOverHTTP(HttpChannelOverHTTP channel) { @@ -264,6 +265,7 @@ public int getHeaderCacheSize() public boolean startResponse(HttpVersion version, int status, String reason) { HttpExchange exchange = getHttpExchange(); + unsolicited = exchange == null; if (exchange == null) return false; @@ -279,7 +281,8 @@ public boolean startResponse(HttpVersion version, int status, String reason) public void parsedHeader(HttpField field) { HttpExchange exchange = getHttpExchange(); - if (exchange == null) + unsolicited |= exchange == null; + if (unsolicited) return; responseHeader(exchange, field); @@ -289,7 +292,8 @@ public void parsedHeader(HttpField field) public boolean headerComplete() { HttpExchange exchange = getHttpExchange(); - if (exchange == null) + unsolicited |= exchange == null; + if (unsolicited) return false; return !responseHeaders(exchange); @@ -299,7 +303,8 @@ public boolean headerComplete() public boolean content(ByteBuffer buffer) { HttpExchange exchange = getHttpExchange(); - if (exchange == null) + unsolicited |= exchange == null; + if (unsolicited) return false; RetainableByteBuffer networkBuffer = this.networkBuffer; @@ -321,7 +326,8 @@ public boolean contentComplete() public void parsedTrailer(HttpField trailer) { HttpExchange exchange = getHttpExchange(); - if (exchange == null) + unsolicited |= exchange == null; + if (unsolicited) return; exchange.getResponse().trailer(trailer); @@ -331,8 +337,12 @@ public void parsedTrailer(HttpField trailer) public boolean messageComplete() { HttpExchange exchange = getHttpExchange(); - if (exchange == null) + if (exchange == null || unsolicited) + { + // We received an unsolicited response from the server. + getHttpConnection().close(); return false; + } int status = exchange.getResponse().getStatus(); if (status != HttpStatus.CONTINUE_100) @@ -349,7 +359,7 @@ public void earlyEOF() { HttpExchange exchange = getHttpExchange(); HttpConnectionOverHTTP connection = getHttpConnection(); - if (exchange == null) + if (exchange == null || unsolicited) connection.close(); else failAndClose(new EOFException(String.valueOf(connection))); @@ -359,7 +369,11 @@ public void earlyEOF() public void badMessage(BadMessageException failure) { HttpExchange exchange = getHttpExchange(); - if (exchange != null) + if (exchange == null || unsolicited) + { + getHttpConnection().close(); + } + else { HttpResponse response = exchange.getResponse(); response.status(failure.getCode()).reason(failure.getReason()); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 21603d3c8163..00245183ed3b 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -1839,6 +1839,61 @@ public void onComplete(Result result) assertArrayEquals(bytes, baos.toByteArray()); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testUnsolicitedResponseBytesFromServer(Scenario scenario) throws Exception + { + String response = "" + + "HTTP/1.1 408 Request Timeout\r\n" + + "Content-Length: 0\r\n" + + "Connection: close\r\n" + + "\r\n"; + testUnsolicitedBytesFromServer(scenario, response); + } + + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testUnsolicitedInvalidBytesFromServer(Scenario scenario) throws Exception + { + String response = "ABCDEF"; + testUnsolicitedBytesFromServer(scenario, response); + } + + private void testUnsolicitedBytesFromServer(Scenario scenario, String bytesFromServer) throws Exception + { + try (ServerSocket server = new ServerSocket(0)) + { + HttpClientTransport transport = new HttpClientTransportOverHTTP(1); + transport.setConnectionPoolFactory(destination -> + { + ConnectionPool connectionPool = new DuplexConnectionPool(destination, 1, destination); + connectionPool.preCreateConnections(1); + return connectionPool; + }); + startClient(scenario, transport, null); + + String host = "localhost"; + int port = server.getLocalPort(); + + // Resolve the destination which will pre-create a connection. + HttpDestination destination = client.resolveDestination(new Origin("http", host, port)); + + // Accept the connection and send an unsolicited 408. + try (Socket socket = server.accept()) + { + OutputStream output = socket.getOutputStream(); + output.write(bytesFromServer.getBytes(StandardCharsets.UTF_8)); + output.flush(); + } + + // Give some time to the client to process the response. + Thread.sleep(1000); + + AbstractConnectionPool pool = (AbstractConnectionPool)destination.getConnectionPool(); + assertEquals(0, pool.getConnectionCount()); + } + } + private void assertCopyRequest(Request original) { Request copy = client.copyRequest((HttpRequest)original, original.getURI());