Fixes #2518 - How to handle 100-continue responses that don't fire onComplete callback.
HttpClient was confused by servers that responded with two 100 Continue in the same HTTP conversation. Now, whether the 100 Continue response has been handled already is stored per-request, not per-conversation. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
parent
67a3497755
commit
d4144f2863
|
@ -54,8 +54,7 @@ public class ContinueProtocolHandler implements ProtocolHandler
|
||||||
{
|
{
|
||||||
boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100;
|
boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100;
|
||||||
boolean expect100 = request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString());
|
boolean expect100 = request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString());
|
||||||
HttpConversation conversation = ((HttpRequest)request).getConversation();
|
boolean handled100 = request.getAttributes().containsKey(ATTRIBUTE);
|
||||||
boolean handled100 = conversation.getAttribute(ATTRIBUTE) != null;
|
|
||||||
return (is100 || expect100) && !handled100;
|
return (is100 || expect100) && !handled100;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -81,7 +80,7 @@ public class ContinueProtocolHandler implements ProtocolHandler
|
||||||
Request request = response.getRequest();
|
Request request = response.getRequest();
|
||||||
HttpConversation conversation = ((HttpRequest)request).getConversation();
|
HttpConversation conversation = ((HttpRequest)request).getConversation();
|
||||||
// Mark the 100 Continue response as handled
|
// Mark the 100 Continue response as handled
|
||||||
conversation.setAttribute(ATTRIBUTE, Boolean.TRUE);
|
request.attribute(ATTRIBUTE, Boolean.TRUE);
|
||||||
|
|
||||||
// Reset the conversation listeners, since we are going to receive another response code
|
// Reset the conversation listeners, since we are going to receive another response code
|
||||||
conversation.updateResponseListeners(null);
|
conversation.updateResponseListeners(null);
|
||||||
|
|
|
@ -657,18 +657,7 @@ public class HttpClientContinueTest extends AbstractTest
|
||||||
try (Socket socket = server.accept())
|
try (Socket socket = server.accept())
|
||||||
{
|
{
|
||||||
// Read the request headers.
|
// Read the request headers.
|
||||||
InputStream input = socket.getInputStream();
|
readRequestHeaders(socket.getInputStream());
|
||||||
int crlfs = 0;
|
|
||||||
while (true)
|
|
||||||
{
|
|
||||||
int read = input.read();
|
|
||||||
if (read == '\r' || read == '\n')
|
|
||||||
++crlfs;
|
|
||||||
else
|
|
||||||
crlfs = 0;
|
|
||||||
if (crlfs == 4)
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
OutputStream output = socket.getOutputStream();
|
OutputStream output = socket.getOutputStream();
|
||||||
String responses = "" +
|
String responses = "" +
|
||||||
|
@ -723,4 +712,74 @@ public class HttpClientContinueTest extends AbstractTest
|
||||||
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
|
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
|
||||||
Assert.assertArrayEquals(bytes, response.getContent());
|
Assert.assertArrayEquals(bytes, response.getContent());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void test_NoExpect_100Continue_ThenRedirect_Then100Continue_ThenResponse() throws Exception
|
||||||
|
{
|
||||||
|
Assume.assumeThat(transport, Matchers.is(Transport.HTTP));
|
||||||
|
|
||||||
|
startClient();
|
||||||
|
client.setMaxConnectionsPerDestination(1);
|
||||||
|
|
||||||
|
try (ServerSocket server = new ServerSocket())
|
||||||
|
{
|
||||||
|
server.bind(new InetSocketAddress("localhost", 0));
|
||||||
|
|
||||||
|
// No Expect header, no content.
|
||||||
|
CountDownLatch latch = new CountDownLatch(1);
|
||||||
|
client.newRequest("localhost", server.getLocalPort())
|
||||||
|
.send(result ->
|
||||||
|
{
|
||||||
|
if (result.isSucceeded() && result.getResponse().getStatus() == HttpStatus.OK_200)
|
||||||
|
latch.countDown();
|
||||||
|
});
|
||||||
|
|
||||||
|
try (Socket socket = server.accept())
|
||||||
|
{
|
||||||
|
InputStream input = socket.getInputStream();
|
||||||
|
OutputStream output = socket.getOutputStream();
|
||||||
|
|
||||||
|
readRequestHeaders(input);
|
||||||
|
String response1 = "" +
|
||||||
|
"HTTP/1.1 100 Continue\r\n" +
|
||||||
|
"\r\n" +
|
||||||
|
"HTTP/1.1 303 See Other\r\n" +
|
||||||
|
"Location: /redirect\r\n" +
|
||||||
|
"Content-Length: 0\r\n" +
|
||||||
|
"\r\n";
|
||||||
|
output.write(response1.getBytes(StandardCharsets.UTF_8));
|
||||||
|
output.flush();
|
||||||
|
|
||||||
|
readRequestHeaders(input);
|
||||||
|
String response2 = "" +
|
||||||
|
"HTTP/1.1 100 Continue\r\n" +
|
||||||
|
"\r\n" +
|
||||||
|
"HTTP/1.1 200 OK\r\n" +
|
||||||
|
"Content-Length: 0\r\n" +
|
||||||
|
"Connection: close\r\n" +
|
||||||
|
"\r\n";
|
||||||
|
output.write(response2.getBytes(StandardCharsets.UTF_8));
|
||||||
|
output.flush();
|
||||||
|
}
|
||||||
|
|
||||||
|
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private void readRequestHeaders(InputStream input) throws IOException
|
||||||
|
{
|
||||||
|
int crlfs = 0;
|
||||||
|
while (true)
|
||||||
|
{
|
||||||
|
int read = input.read();
|
||||||
|
if (read < 0)
|
||||||
|
break;
|
||||||
|
if (read == '\r' || read == '\n')
|
||||||
|
++crlfs;
|
||||||
|
else
|
||||||
|
crlfs = 0;
|
||||||
|
if (crlfs == 4)
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue