6968351: httpserver clashes with delayed TCP ACKs for low Content-Length

Reviewed-by: dfuchs, djelinski, michaelm, jpai

src/jdk.httpserver/share/classes/sun/net/httpserver/ChunkedOutputStream.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2024, 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
@@ -137,8 +137,14 @@ class ChunkedOutputStream extends FilterOutputStream
         if (closed) {
             return;
         }
-        flush();
         try {
+            /*
+            * write any pending chunk data. manually write chunk rather than
+            * calling flush to avoid sending small packets
+            */
+            if (count > 0) {
+                writeChunk();
+            }
             /* write an empty chunk */
             writeChunk();
             out.flush();

src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2024, 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
@@ -207,7 +207,7 @@ class ExchangeImpl {
         }
         this.rcode = rCode;
         String statusLine = "HTTP/1.1 "+rCode+Code.msg(rCode)+"\r\n";
-        OutputStream tmpout = new BufferedOutputStream (ros);
+        ByteArrayOutputStream tmpout = new ByteArrayOutputStream();
         PlaceholderOutputStream o = getPlaceholderResponseBody();
         tmpout.write (bytes(statusLine, 0), 0, statusLine.length());
         boolean noContentToSend = false; // assume there is content
@@ -278,11 +278,11 @@ class ExchangeImpl {
 
         write (rspHdrs, tmpout);
         this.rspContentLen = contentLen;
-        tmpout.flush() ;
+        tmpout.writeTo(ros);
-        tmpout = null;
         sentHeaders = true;
         logger.log(Level.TRACE, "Sent headers: noContentToSend=" + noContentToSend);
         if (noContentToSend) {
+            ros.flush();
             close();
         }
         server.logReply (rCode, req.requestLine(), null);

src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2024, 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
@@ -37,6 +37,7 @@ import sun.net.httpserver.HttpConnection.State;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -686,6 +687,7 @@ class ServerImpl {
                             ServerImpl.this, chan
                         );
                     }
+                    rawout = new BufferedOutputStream(rawout);
                     connection.raw = rawin;
                     connection.rawout = rawout;
                 }

test/jdk/com/sun/net/httpserver/TcpNoDelayNotRequired.java

@@ -0,0 +1,144 @@
+/*
+ * Copyright (c) 2024, 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 6968351
+ * @summary  tcp no delay not required for small payloads
+ * @library /test/lib
+ * @run main/othervm/timeout=5 -Dsun.net.httpserver.nodelay=false  TcpNoDelayNotRequired
+ */
+
+import com.sun.net.httpserver.Headers;
+import com.sun.net.httpserver.HttpContext;
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+
+import com.sun.net.httpserver.HttpsConfigurator;
+import com.sun.net.httpserver.HttpsServer;
+import jdk.test.lib.net.SimpleSSLContext;
+import jdk.test.lib.net.URIBuilder;
+
+import javax.net.ssl.SSLContext;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.logging.SimpleFormatter;
+import java.util.logging.StreamHandler;
+
+public class TcpNoDelayNotRequired {
+
+    public static final Logger LOGGER = Logger.getLogger("sun.net.www.protocol.http");
+
+    public static void main (String[] args) throws Exception {
+
+        java.util.logging.Handler outHandler = new StreamHandler(System.out,
+                new SimpleFormatter());
+        outHandler.setLevel(Level.FINEST);
+        LOGGER.setLevel(Level.FINEST);
+        LOGGER.addHandler(outHandler);
+
+        InetAddress loopback = InetAddress.getLoopbackAddress();
+        InetSocketAddress addr = new InetSocketAddress (loopback, 0);
+
+        SSLContext sslContext = new SimpleSSLContext().get();
+
+        HttpServer httpServer = HttpServer.create (addr, 0);
+        testHttpServer("http",httpServer,sslContext);
+
+        HttpsServer httpsServer = HttpsServer.create (addr, 0);
+        httpsServer.setHttpsConfigurator(new HttpsConfigurator(sslContext));
+
+        testHttpServer("https",httpsServer,sslContext);
+    }
+
+    private static void testHttpServer(String scheme,HttpServer server,SSLContext sslContext) throws Exception {
+        HttpContext ctx = server.createContext ("/test", new Handler());
+        HttpContext ctx2 = server.createContext ("/chunked", new ChunkedHandler());
+        ExecutorService executor = Executors.newCachedThreadPool();
+        server.setExecutor (executor);
+        server.start ();
+        try {
+            try (HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build()) {
+                long start = System.currentTimeMillis();
+                for (int i = 0; i < 1000; i++) {
+                    var uri = URIBuilder.newBuilder().scheme(scheme).loopback().port(server.getAddress().getPort()).path("/test").build();
+                    var response = client.send(HttpRequest.newBuilder(uri).build(), HttpResponse.BodyHandlers.ofString());
+                    if (!response.body().equals("hello"))
+                        throw new IllegalStateException("incorrect body " + response.body());
+                }
+                for (int i = 0; i < 1000; i++) {
+                    var uri = URIBuilder.newBuilder().scheme(scheme).loopback().port(server.getAddress().getPort()).path("/chunked").build();
+                    var response = client.send(HttpRequest.newBuilder(uri).build(), HttpResponse.BodyHandlers.ofString());
+                    if (!response.body().equals("hello"))
+                        throw new IllegalStateException("incorrect body " + response.body());
+                }
+                long time = System.currentTimeMillis() - start;
+                System.out.println("time " + time);
+            }
+        } finally {
+            server.stop(0);
+        }
+        executor.shutdown();
+    }
+
+    static class Handler implements HttpHandler {
+        public void handle (HttpExchange t)
+                throws IOException
+        {
+            Headers rmap = t.getResponseHeaders();
+            try (var is = t.getRequestBody()) {
+                is.readAllBytes();
+            }
+            rmap.add("content-type","text/plain");
+            t.sendResponseHeaders(200,5);
+            try (var os = t.getResponseBody()) {
+                os.write("hello".getBytes(StandardCharsets.ISO_8859_1));
+            }
+        }
+    }
+    static class ChunkedHandler implements HttpHandler {
+        public void handle (HttpExchange t)
+                throws IOException
+        {
+            Headers rmap = t.getResponseHeaders();
+            try (var is = t.getRequestBody()) {
+                is.readAllBytes();
+            }
+            rmap.add("content-type","text/plain");
+            t.sendResponseHeaders(200,0);
+            try (var os = t.getResponseBody()) {
+                os.write("hello".getBytes(StandardCharsets.ISO_8859_1));
+            }
+        }
+    }
+}

test/jdk/com/sun/net/httpserver/simpleserver/StressDirListings.java

@@ -25,7 +25,7 @@
  * @test
  * @summary Test to stress directory listings
  * @library /test/lib
- * @run testng/othervm/timeout=180 -Dsun.net.httpserver.nodelay=true StressDirListings
+ * @run testng/othervm/timeout=180 StressDirListings
  */
 
 import java.io.IOException;

test/jdk/java/net/Authenticator/B4769350.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2024, 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
@@ -355,7 +355,7 @@ public class B4769350 {
                 throws IOException
         {
             exchange.getResponseHeaders().add("Proxy-Authenticate", reply);
-            exchange.sendResponseHeaders(407, 0);
+            exchange.sendResponseHeaders(407, -1);
         }
 
         static void okReply (HttpExchange exchange) throws IOException {

test/jdk/sun/net/www/http/KeepAliveCache/B8293562.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, 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
@@ -244,6 +244,7 @@ public class B8293562 {
         public void handle(HttpExchange t) throws IOException {
             t.sendResponseHeaders(404, 3);
             t.getResponseBody().write("abc".getBytes(StandardCharsets.UTF_8));
+            t.getResponseBody().close();
         }
     }
 }