8054022: HttpURLConnection timeouts with Expect: 100-Continue and no chunking

Reviewed-by: djelinski, dfuchs

src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

@@ -1312,53 +1312,74 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
     }
 
     private void expect100Continue() throws IOException {
-            // Expect: 100-Continue was set, so check the return code for
+        // Expect: 100-Continue was set, so check the return code for
-            // Acceptance
+        // Acceptance
-            int oldTimeout = http.getReadTimeout();
+        int oldTimeout = http.getReadTimeout();
-            boolean enforceTimeOut = false;
+        boolean timedOut = false;
-            boolean timedOut = false;
+        boolean tempTimeOutSet = false;
-            if (oldTimeout <= 0) {
+        if (oldTimeout <= 0 || oldTimeout > 5000) {
-                // 5s read timeout in case the server doesn't understand
+            if (logger.isLoggable(PlatformLogger.Level.FINE)) {
-                // Expect: 100-Continue
+                logger.fine("Timeout currently set to " +
-                http.setReadTimeout(5000);
+                        oldTimeout + " temporarily setting it to 5 seconds");
-                enforceTimeOut = true;
             }
+            // 5s read timeout in case the server doesn't understand
+            // Expect: 100-Continue
+            http.setReadTimeout(5000);
+            tempTimeOutSet = true;
+        }
 
-            try {
+        try {
-                http.parseHTTP(responses, this);
+            http.parseHTTP(responses, this);
-            } catch (SocketTimeoutException se) {
+        } catch (SocketTimeoutException se) {
-                if (!enforceTimeOut) {
+            if (logger.isLoggable(PlatformLogger.Level.FINE)) {
-                    throw se;
+                logger.fine("SocketTimeoutException caught," +
-                }
+                        " will attempt to send body regardless");
-                timedOut = true;
-                http.setIgnoreContinue(true);
             }
-            if (!timedOut) {
+            timedOut = true;
-                // Can't use getResponseCode() yet
+        }
-                String resp = responses.getValue(0);
+
-                // Parse the response which is of the form:
+        if (!timedOut) {
-                // HTTP/1.1 417 Expectation Failed
+            // Can't use getResponseCode() yet
-                // HTTP/1.1 100 Continue
+            String resp = responses.getValue(0);
-                if (resp != null && resp.startsWith("HTTP/")) {
+            // Parse the response which is of the form:
-                    String[] sa = resp.split("\\s+");
+            // HTTP/1.1 417 Expectation Failed
-                    responseCode = -1;
+            // HTTP/1.1 100 Continue
-                    try {
+            if (resp != null && resp.startsWith("HTTP/")) {
-                        // Response code is 2nd token on the line
+                String[] sa = resp.split("\\s+");
-                        if (sa.length > 1)
+                responseCode = -1;
-                            responseCode = Integer.parseInt(sa[1]);
+                try {
-                    } catch (NumberFormatException numberFormatException) {
+                    // Response code is 2nd token on the line
+                    if (sa.length > 1)
+                        responseCode = Integer.parseInt(sa[1]);
+                    if (logger.isLoggable(PlatformLogger.Level.FINE)) {
+                        logger.fine("response code received " + responseCode);
                     }
-                }
+                } catch (NumberFormatException numberFormatException) {
-                if (responseCode != 100) {
-                    throw new ProtocolException("Server rejected operation");
                 }
             }
+            if (responseCode != 100) {
+                // responseCode will be returned to caller
+                throw new ProtocolException("Server rejected operation");
+            }
+        }
 
+        // If timeout was changed, restore to original value
+        if (tempTimeOutSet) {
+            if (logger.isLoggable(PlatformLogger.Level.FINE)) {
+                logger.fine("Restoring original timeout : " + oldTimeout);
+            }
             http.setReadTimeout(oldTimeout);
+        }
 
-            responseCode = -1;
+        // Ignore any future 100 continue messages
-            responses.reset();
+        http.setIgnoreContinue(true);
-            // Proceed
+        if (logger.isLoggable(PlatformLogger.Level.FINE)) {
+            logger.fine("Set Ignore Continue to true");
+        }
+
+        responseCode = -1;
+        responses.reset();
+        // Proceed
     }
 
     /*
@@ -1427,7 +1448,6 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
             boolean expectContinue = false;
             String expects = requests.findValue("Expect");
             if ("100-Continue".equalsIgnoreCase(expects) && streaming()) {
-                http.setIgnoreContinue(false);
                 expectContinue = true;
             }
 
@@ -1436,6 +1456,7 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
             }
 
             if (expectContinue) {
+                http.setIgnoreContinue(false);
                 expect100Continue();
             }
             ps = (PrintStream)http.getOutputStream();
@@ -1474,6 +1495,7 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
         }
     }
 
+    // Streaming returns true if there is a request body to send
     public boolean streaming () {
         return (fixedContentLength != -1) || (fixedContentLengthLong != -1) ||
                (chunkLength != -1);

test/jdk/java/net/HttpURLConnection/HttpURLConnectionExpectContinueTest.java

@@ -0,0 +1,437 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8054022
+ * @summary Verify that expect 100-continue doesn't hang
+ * @library /test/lib
+ * @run junit/othervm HttpURLConnectionExpectContinueTest
+ */
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.net.*;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+public class HttpURLConnectionExpectContinueTest {
+
+    class Control {
+        volatile ServerSocket serverSocket = null;
+        volatile boolean stop = false;
+        volatile boolean respondWith100Continue = false;
+        volatile boolean write100ContinueTwice = false;
+        volatile String response = null;
+    }
+
+    private Thread serverThread = null;
+    private volatile Control control;
+    static final Logger logger;
+
+    static {
+        logger = Logger.getLogger("sun.net.www.protocol.http.HttpURLConnection");
+        logger.setLevel(Level.ALL);
+        Logger.getLogger("").getHandlers()[0].setLevel(Level.ALL);
+    }
+
+    @BeforeAll
+    public void startServerSocket() throws Exception {
+        Control control = this.control = new Control();
+
+        control.serverSocket = new ServerSocket();
+        control.serverSocket.setReuseAddress(true);
+        control.serverSocket.bind(new InetSocketAddress("127.0.0.1", 54321));
+        Runnable runnable = () -> {
+            while (!control.stop) {
+                try {
+                    Socket socket = control.serverSocket.accept();
+                    InputStream inputStream = socket.getInputStream();
+                    InputStreamReader inputStreamReader = new InputStreamReader(inputStream);
+
+                    StringBuilder stringBuilder = new StringBuilder();
+
+                    // Read initial request
+                    byte b;
+                    while (true) {
+                        b = (byte) inputStreamReader.read();
+                        stringBuilder.append((char) b);
+
+                        if (stringBuilder.length() >= 4) {
+                            char[] lastBytes = new char[4];
+                            stringBuilder.getChars(
+                                    stringBuilder.length() - 4,
+                                    stringBuilder.length(), lastBytes, 0);
+                            if (Arrays.equals(lastBytes, new char[]{'\r', '\n', '\r', '\n'})) {
+                                break;
+                            }
+                        }
+                    }
+
+                    OutputStream outputStream = socket.getOutputStream();
+
+                    String header = stringBuilder.toString();
+                    String contentLengthString = "Content-Length:";
+
+                    // send 100 continue responses if set by test
+                    if (control.respondWith100Continue) {
+                        outputStream.write("HTTP/1.1 100 Continue\r\n\r\n".getBytes());
+                        outputStream.flush();
+                        if (control.write100ContinueTwice) {
+                            outputStream.write("HTTP/1.1 100 Continue\r\n\r\n".getBytes());
+                            outputStream.flush();
+                        }
+                    }
+
+                    // expect main request to be received
+                    int idx = header.indexOf(contentLengthString);
+
+                    if (idx >= 0) {
+                        String substr = header.substring(idx + contentLengthString.length());
+                        idx = substr.indexOf('\r');
+                        substr = substr.substring(0, idx);
+                        int contentLength = Integer.parseInt(substr.trim());
+
+                        StringBuilder contentLengthBuilder = new StringBuilder();
+                        for (int i = 0; i < contentLength; i++) {
+                            b = (byte) inputStreamReader.read();
+                            contentLengthBuilder.append((char) b);
+                        }
+
+                    } else {
+                        StringBuilder contentLengthBuilder = new StringBuilder();
+                        while (true) {
+                            b = (byte) inputStreamReader.read();
+                            contentLengthBuilder.append((char) b);
+
+                            if (contentLengthBuilder.length() >= 2) {
+                                char[] lastBytes = new char[2];
+                                contentLengthBuilder.getChars(
+                                        contentLengthBuilder.length() - 2,
+                                        contentLengthBuilder.length(), lastBytes, 0);
+                                if (Arrays.equals(lastBytes, new char[]{'\r', '\n'})) {
+                                    String lengthInHex =
+                                            contentLengthBuilder.substring(0, contentLengthBuilder.length() - 2);
+
+                                    int contentLength = Integer.parseInt(lengthInHex, 16);
+                                    char[] body = new char[contentLength];
+                                    inputStreamReader.read(body);
+                                    break;
+                                    // normally we have to parse more data,
+                                    // but for simplicity we expect no more chunks...
+                                }
+                            }
+                        }
+                    }
+
+                    // send response
+                    outputStream.write(control.response.getBytes());
+                    outputStream.flush();
+                } catch (SocketException e) {
+                    // ignore
+                } catch (IOException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        };
+        serverThread = new Thread(runnable);
+        serverThread.start();
+    }
+
+    @AfterAll
+    public void stopServerSocket() throws Exception {
+        Control control = this.control;
+        control.stop = true;
+        control.serverSocket.close();
+        serverThread.join();
+    }
+
+    @Test
+    public void testNonChunkedRequestAndNoExpect100ContinueResponse() throws Exception {
+        String body = "testNonChunkedRequestAndNoExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = false;
+        control.write100ContinueTwice = false;
+
+        HttpURLConnection connection = createConnection();
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testNonChunkedRequestWithExpect100ContinueResponse() throws Exception {
+        String body = "testNonChunkedRequestWithExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = true;
+        control.write100ContinueTwice = false;
+
+        HttpURLConnection connection = createConnection();
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testNonChunkedRequestWithDoubleExpect100ContinueResponse() throws Exception {
+        String body = "testNonChunkedRequestWithDoubleExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = true;
+        control.write100ContinueTwice = true;
+
+        HttpURLConnection connection = createConnection();
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testChunkedRequestAndNoExpect100ContinueResponse() throws Exception {
+        String body = "testChunkedRequestAndNoExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = false;
+        control.write100ContinueTwice = false;
+
+        HttpURLConnection connection = createConnection();
+        connection.setChunkedStreamingMode(body.length() / 2);
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testChunkedRequestWithExpect100ContinueResponse() throws Exception {
+        String body = "testChunkedRequestWithExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = true;
+        control.write100ContinueTwice = false;
+
+        HttpURLConnection connection = createConnection();
+        connection.setChunkedStreamingMode(body.length() / 2);
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testChunkedRequestWithDoubleExpect100ContinueResponse() throws Exception {
+        String body = "testChunkedRequestWithDoubleExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = true;
+        control.write100ContinueTwice = true;
+
+        HttpURLConnection connection = createConnection();
+        connection.setChunkedStreamingMode(body.length() / 2);
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testFixedLengthRequestAndNoExpect100ContinueResponse() throws Exception {
+        String body = "testFixedLengthRequestAndNoExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = false;
+        control.write100ContinueTwice = false;
+
+        HttpURLConnection connection = createConnection();
+        connection.setFixedLengthStreamingMode(body.length());
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testFixedLengthRequestWithExpect100ContinueResponse() throws Exception {
+        String body = "testFixedLengthRequestWithExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = true;
+        control.write100ContinueTwice = false;
+
+        HttpURLConnection connection = createConnection();
+        connection.setFixedLengthStreamingMode(body.getBytes().length);
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    @Test
+    public void testFixedLengthRequestWithDoubleExpect100ContinueResponse() throws Exception {
+        String body = "testFixedLengthRequestWithDoubleExpect100ContinueResponse";
+        Control control = this.control;
+        control.response = "HTTP/1.1 200 OK\r\n" +
+                "Connection: close\r\n" +
+                "Content-Length: " + body.length() + "\r\n" +
+                "\r\n" +
+                body + "\r\n";
+        control.respondWith100Continue = true;
+        control.write100ContinueTwice = true;
+
+        HttpURLConnection connection = createConnection();
+        connection.setFixedLengthStreamingMode(body.getBytes().length);
+        OutputStream outputStream = connection.getOutputStream();
+        outputStream.write(body.getBytes());
+        outputStream.close();
+
+        int responseCode = connection.getResponseCode();
+        String responseBody = new String(connection.getInputStream().readAllBytes(), StandardCharsets.UTF_8).strip();
+        System.err.println("response body: " + responseBody);
+        assertTrue(responseCode == 200,
+                String.format("Expected 200 response, instead received %s", responseCode));
+        assertTrue(body.equals(responseBody),
+                String.format("Expected response %s, instead received %s", body, responseBody));
+    }
+
+    // Creates a connection with all the common settings used in each test
+    private HttpURLConnection createConnection() throws IOException {
+        URL url = new URL("http://localhost:" + control.serverSocket.getLocalPort());
+
+        HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+        connection.setDoOutput(true);
+        connection.setConnectTimeout(1000);
+        connection.setReadTimeout(5000);
+        connection.setUseCaches(false);
+        connection.setInstanceFollowRedirects(false);
+        connection.setRequestMethod("POST");
+        connection.setRequestProperty("Connection", "Close");
+        connection.setRequestProperty("Expect", "100-Continue");
+
+        return connection;
+    }
+}