Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #5152 - HttpClient should handle unsolicited responses. #5166

Merged
merged 1 commit into from Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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)
Expand All @@ -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)));
Expand All @@ -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());
Expand Down
Expand Up @@ -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());
Expand Down