8307648: java/net/httpclient/ExpectContinueTest.java timed out

Reviewed-by: djelinski

src/java.net.http/share/classes/jdk/internal/net/http/Stream.java

@@ -475,7 +475,7 @@ class Stream<T> extends ExchangeImpl<T> {
         if ((frame instanceof HeaderFrame hf)) {
             if (hf.endHeaders()) {
                 Log.logTrace("handling response (streamid={0})", streamid);
-                handleResponse();
+                handleResponse(hf);
             }
             if (hf.getFlag(HeaderFrame.END_STREAM)) {
                 if (debug.on()) debug.log("handling END_STREAM: %d", streamid);
@@ -511,7 +511,7 @@ class Stream<T> extends ExchangeImpl<T> {
         return rspHeadersConsumer::onDecoded;
     }
 
-    protected void handleResponse() throws IOException {
+    protected void handleResponse(HeaderFrame hf) throws IOException {
         HttpHeaders responseHeaders = responseHeadersBuilder.build();
 
         if (!finalResponseCodeReceived) {
@@ -519,8 +519,20 @@ class Stream<T> extends ExchangeImpl<T> {
                     .firstValueAsLong(":status")
                     .orElseThrow(() -> new IOException("no statuscode in response"));
             // If informational code, response is partially complete
-            if (responseCode < 100 || responseCode > 199)
+            if (responseCode < 100 || responseCode > 199) {
                 this.finalResponseCodeReceived = true;
+            } else if (hf.getFlag(HeaderFrame.END_STREAM)) {
+                // see RFC 9113 section 8.1:
+                // A HEADERS frame with the END_STREAM flag set that carries an
+                // informational status code is malformed
+                String msg = ("Stream %s PROTOCOL_ERROR: " +
+                        "HEADERS frame with status %s has END_STREAM flag set")
+                        .formatted(streamid, responseCode);
+                if (debug.on()) {
+                    debug.log(msg);
+                }
+                cancelImpl(new IOException(msg), ResetFrame.PROTOCOL_ERROR);
+            }
 
             response = new Response(
                     request, exchange, responseHeaders, connection(),
@@ -567,12 +579,20 @@ class Stream<T> extends ExchangeImpl<T> {
                 // response to be read before the Reset is handled in the case where the client's
                 // input stream is partially consumed or not consumed at all by the server.
                 if (frame.getErrorCode() != ResetFrame.NO_ERROR) {
+                    if (debug.on()) {
+                        debug.log("completing requestBodyCF exceptionally due to received" +
+                                " RESET(%s) (stream=%s)", frame.getErrorCode(), streamid);
+                    }
                     requestBodyCF.completeExceptionally(new IOException("RST_STREAM received"));
                 } else {
+                    if (debug.on()) {
+                        debug.log("completing requestBodyCF normally due to received" +
+                                " RESET(NO_ERROR) (stream=%s)", streamid);
+                    }
                     requestBodyCF.complete(null);
                 }
             }
-            if (response == null && subscriber == null) {
+            if ((response == null || !finalResponseCodeReceived) && subscriber == null) {
                 // we haven't received the headers yet, and won't receive any!
                 // handle reset now.
                 handleReset(frame, null);
@@ -950,6 +970,10 @@ class Stream<T> extends ExchangeImpl<T> {
         private void onNextImpl(ByteBuffer item) {
             // Got some more request body bytes to send.
             if (requestBodyCF.isDone()) {
+                if (debug.on()) {
+                    debug.log("RequestSubscriber: requestBodyCf is done: " +
+                            "cancelling subscription");
+                }
                 // stream already cancelled, probably in timeout
                 sendScheduler.stop();
                 subscription.cancel();
@@ -1528,7 +1552,7 @@ class Stream<T> extends ExchangeImpl<T> {
 
         // create and return the PushResponseImpl
         @Override
-        protected void handleResponse() {
+        protected void handleResponse(HeaderFrame hf) {
             HttpHeaders responseHeaders = responseHeadersBuilder.build();
 
             if (!finalPushResponseCodeReceived) {

test/jdk/java/net/httpclient/ExpectContinueTest.java

@@ -28,7 +28,7 @@
  * @summary Tests that when the httpclient sends a 100 Expect Continue header and receives
  *          a response code of 417 Expectation Failed, that the client does not hang
  *          indefinitely and closes the connection.
- * @bug 8286171
+ * @bug 8286171 8307648
  * @library /test/lib /test/jdk/java/net/httpclient/lib
  * @build jdk.httpclient.test.lib.common.HttpServerAdapters
  * @run testng/othervm -Djdk.internal.httpclient.debug=err ExpectContinueTest
@@ -56,6 +56,7 @@ import java.net.ServerSocket;
 import java.net.Socket;
 import java.net.URI;
 import java.net.http.HttpClient;
+import java.net.http.HttpClient.Builder;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
 import java.util.StringTokenizer;
@@ -106,6 +107,10 @@ public class ExpectContinueTest implements HttpServerAdapters {
         h2postUri = URI.create("http://" + http2TestServer.serverAuthority() + "/http2/post");
         h2hangUri = URI.create("http://" + http2TestServer.serverAuthority() + "/http2/hang");
 
+        System.out.println("HTTP/1.1 server listening at: " + http1TestServer.serverAuthority());
+        System.out.println("HTTP/1.1 hang server listening at: " + hangUri.getRawAuthority());
+        System.out.println("HTTP/2 clear server listening at: " + http2TestServer.serverAuthority());
+
         http1TestServer.start();
         http1HangServer.start();
         http2TestServer.start();
@@ -124,8 +129,10 @@ public class ExpectContinueTest implements HttpServerAdapters {
         public void handle(HttpTestExchange exchange) throws IOException {
             try (InputStream is = exchange.getRequestBody();
                  OutputStream os = exchange.getResponseBody()) {
+                System.err.println("Server reading body");
                 is.readAllBytes();
                 byte[] bytes = "RESPONSE_BODY".getBytes(UTF_8);
+                System.err.println("Server sending 200  (length="+bytes.length+")");
                 exchange.sendResponseHeaders(200, bytes.length);
                 os.write(bytes);
             }
@@ -139,13 +146,16 @@ public class ExpectContinueTest implements HttpServerAdapters {
             // Http1 server has already sent 100 response at this point but not Http2 server
             if (exchange.getExchangeVersion().equals(HttpClient.Version.HTTP_2)) {
                 // Send 100 Headers, tell client that we're ready for body
+                System.err.println("Server sending 100 (length = 0)");
                 exchange.sendResponseHeaders(100, 0);
             }
 
             // Read body from client and acknowledge with 200
             try (InputStream is = exchange.getRequestBody();
                 OutputStream os = exchange.getResponseBody()) {
+                System.err.println("Server reading body");
                 is.readAllBytes();
+                System.err.println("Server send 200 (length=0)");
                 exchange.sendResponseHeaders(200, 0);
             }
         }
@@ -159,6 +169,7 @@ public class ExpectContinueTest implements HttpServerAdapters {
             try (InputStream is = exchange.getRequestBody();
                  OutputStream os = exchange.getResponseBody()) {
                 byte[] bytes = EXPECTATION_FAILED_417.getBytes();
+                System.err.println("Server send 417 (length="+bytes.length+")");
                 exchange.sendResponseHeaders(417, bytes.length);
                 os.write(bytes);
             }
@@ -184,11 +195,14 @@ public class ExpectContinueTest implements HttpServerAdapters {
         public void run() {
             byte[] bytes = EXPECTATION_FAILED_417.getBytes();
 
+            boolean closed = this.closed;
             while (!closed) {
                 try {
                     // Not using try with resources here as we expect the client to close resources when
                     // 417 is received
-                    client = ss.accept();
+                    System.err.println("Http1HangServer accepting connections");
+                    var client = this.client = ss.accept();
+                    System.err.println("Http1HangServer accepted connection: " + client);
                     InputStream is = client.getInputStream();
                     OutputStream os = client.getOutputStream();
 
@@ -213,7 +227,8 @@ public class ExpectContinueTest implements HttpServerAdapters {
                                         && version.equals("HTTP/1.1");
                     // If correct request, send 417 reply. Otherwise, wait for correct one
                     if (validRequest) {
-                        closed = true;
+                        System.err.println("Http1HangServer sending 417");
+                        closed = this.closed = true;
                         response.append("HTTP/1.1 417 Expectation Failed\r\n")
                                 .append("Content-Length: ")
                                 .append(0)
@@ -224,17 +239,25 @@ public class ExpectContinueTest implements HttpServerAdapters {
                         os.write(bytes);
                         os.flush();
                     } else {
+                        System.err.println("Http1HangServer received invalid request: closing");
                         client.close();
                     }
                 } catch (IOException e) {
-                    closed = true;
+                    closed = this.closed = true;
                     e.printStackTrace();
+                } finally {
+                    if (closed = this.closed) {
+                        System.err.println("Http1HangServer: finished");
+                    } else {
+                        System.err.println("Http1HangServer: looping for accepting next connection");
+                    }
                 }
             }
         }
 
         @Override
         public void close() throws IOException {
+            var client = this.client;
             if (client != null) client.close();
             if (ss != null) ss.close();
         }
@@ -244,13 +267,15 @@ public class ExpectContinueTest implements HttpServerAdapters {
     public Object[][] urisData() {
         return new Object[][]{
                 { getUri,   postUri, hangUri, HTTP_1_1 },
-                { h2getUri,  h2postUri, h2hangUri, HttpClient.Version.HTTP_2 }
+                { h2getUri,  h2postUri, h2hangUri, HTTP_2 }
         };
     }
 
     @Test(dataProvider = "uris")
     public void test(URI getUri, URI postUri, URI hangUri, HttpClient.Version version) throws IOException, InterruptedException {
+        System.out.println("Testing with version: " + version);
         HttpClient client = HttpClient.newBuilder()
+                .proxy(Builder.NO_PROXY)
                 .version(version)
                 .build();
 
@@ -268,18 +293,24 @@ public class ExpectContinueTest implements HttpServerAdapters {
                 .expectContinue(true)
                 .build();
 
+        System.out.printf("Sending request (%s): %s%n", version, getRequest);
+        System.err.println("Sending request: " + getRequest);
         CompletableFuture<HttpResponse<String>> cf = client.sendAsync(getRequest, HttpResponse.BodyHandlers.ofString());
         HttpResponse<String> resp = cf.join();
         System.err.println("Response Headers: " + resp.headers());
         System.err.println("Response Status Code: " + resp.statusCode());
         assertEquals(resp.statusCode(), 200);
 
+        System.out.printf("Sending request (%s): %s%n", version, postRequest);
+        System.err.println("Sending request: " + postRequest);
         cf = client.sendAsync(postRequest, HttpResponse.BodyHandlers.ofString());
         resp = cf.join();
         System.err.println("Response Headers: " + resp.headers());
         System.err.println("Response Status Code: " + resp.statusCode());
         assertEquals(resp.statusCode(), 200);
 
+        System.out.printf("Sending request (%s): %s%n", version, hangRequest);
+        System.err.println("Sending request: " + hangRequest);
         cf = client.sendAsync(hangRequest, HttpResponse.BodyHandlers.ofString());
         resp = cf.join();
         System.err.println("Response Headers: " + resp.headers());

test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http2/Http2TestExchangeImpl.java

@@ -133,6 +133,9 @@ public class Http2TestExchangeImpl implements Http2TestExchange {
 
     @Override
     public void sendResponseHeaders(int rCode, long responseLength) throws IOException {
+        // Do not set Content-Length for 100, and do not set END_STREAM
+        if (rCode == 100) responseLength = 0;
+
         this.responseLength = responseLength;
         if (responseLength !=0 && rCode != 204 && !isHeadRequest()) {
                 long clen = responseLength > 0 ? responseLength : 0;