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 <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2020-08-18 12:18:20 +02:00
parent 0646e4d0b5
commit c88aba6587
2 changed files with 76 additions and 7 deletions

View File

@ -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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
public boolean headerComplete()
{
HttpExchange exchange = getHttpExchange();
if (exchange == null)
unsolicited |= exchange == null;
if (unsolicited)
return false;
return !responseHeaders(exchange);
@ -299,7 +303,8 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
{
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 class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
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());

View File

@ -1839,6 +1839,61 @@ public class HttpClientTest extends AbstractHttpClientServerTest
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());