From d4144f28632756fa55784a83a8c65d4f9d817ce4 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 9 May 2018 22:33:11 +0200 Subject: [PATCH] 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 --- .../jetty/client/ContinueProtocolHandler.java | 5 +- .../http/client/HttpClientContinueTest.java | 83 ++++++++++++++++--- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ContinueProtocolHandler.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ContinueProtocolHandler.java index c190acfbdb9..16825e704a2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ContinueProtocolHandler.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ContinueProtocolHandler.java @@ -54,8 +54,7 @@ public class ContinueProtocolHandler implements ProtocolHandler { boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100; boolean expect100 = request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString()); - HttpConversation conversation = ((HttpRequest)request).getConversation(); - boolean handled100 = conversation.getAttribute(ATTRIBUTE) != null; + boolean handled100 = request.getAttributes().containsKey(ATTRIBUTE); return (is100 || expect100) && !handled100; } @@ -81,7 +80,7 @@ public class ContinueProtocolHandler implements ProtocolHandler Request request = response.getRequest(); HttpConversation conversation = ((HttpRequest)request).getConversation(); // 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 conversation.updateResponseListeners(null); diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java index 3f4907aaa45..05acb8725a9 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java @@ -657,18 +657,7 @@ public class HttpClientContinueTest extends AbstractTest try (Socket socket = server.accept()) { // Read the request headers. - InputStream input = socket.getInputStream(); - int crlfs = 0; - while (true) - { - int read = input.read(); - if (read == '\r' || read == '\n') - ++crlfs; - else - crlfs = 0; - if (crlfs == 4) - break; - } + readRequestHeaders(socket.getInputStream()); OutputStream output = socket.getOutputStream(); String responses = "" + @@ -723,4 +712,74 @@ public class HttpClientContinueTest extends AbstractTest Assert.assertEquals(HttpStatus.OK_200, response.getStatus()); 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; + } + } }