From 02c95a6d7eb77ed17ae64d0f585197e87a67cc4a Mon Sep 17 00:00:00 2001 From: robertengels Date: Tue, 7 May 2024 13:18:24 +0000 Subject: [PATCH] 6968351: httpserver clashes with delayed TCP ACKs for low Content-Length Reviewed-by: dfuchs, djelinski, michaelm, jpai --- .../net/httpserver/ChunkedOutputStream.java | 10 +- .../sun/net/httpserver/ExchangeImpl.java | 8 +- .../sun/net/httpserver/ServerImpl.java | 4 +- .../net/httpserver/TcpNoDelayNotRequired.java | 144 ++++++++++++++++++ .../simpleserver/StressDirListings.java | 2 +- test/jdk/java/net/Authenticator/B4769350.java | 4 +- .../net/www/http/KeepAliveCache/B8293562.java | 3 +- 7 files changed, 164 insertions(+), 11 deletions(-) create mode 100644 test/jdk/com/sun/net/httpserver/TcpNoDelayNotRequired.java diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/ChunkedOutputStream.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/ChunkedOutputStream.java index 11ad6c3eaaa..aefb9b5d1fc 100644 --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ChunkedOutputStream.java +++ b/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(); diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java index 770642fda11..1119d1e386b 100644 --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java +++ b/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 = null; + tmpout.writeTo(ros); sentHeaders = true; logger.log(Level.TRACE, "Sent headers: noContentToSend=" + noContentToSend); if (noContentToSend) { + ros.flush(); close(); } server.logReply (rCode, req.requestLine(), null); diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java index add50920336..49377475719 100644 --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java +++ b/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; } diff --git a/test/jdk/com/sun/net/httpserver/TcpNoDelayNotRequired.java b/test/jdk/com/sun/net/httpserver/TcpNoDelayNotRequired.java new file mode 100644 index 00000000000..b7366188821 --- /dev/null +++ b/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)); + } + } + } +} diff --git a/test/jdk/com/sun/net/httpserver/simpleserver/StressDirListings.java b/test/jdk/com/sun/net/httpserver/simpleserver/StressDirListings.java index 8197fd436e7..18a1f060c48 100644 --- a/test/jdk/com/sun/net/httpserver/simpleserver/StressDirListings.java +++ b/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; diff --git a/test/jdk/java/net/Authenticator/B4769350.java b/test/jdk/java/net/Authenticator/B4769350.java index aba41d222ea..813bf02de93 100644 --- a/test/jdk/java/net/Authenticator/B4769350.java +++ b/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 { diff --git a/test/jdk/sun/net/www/http/KeepAliveCache/B8293562.java b/test/jdk/sun/net/www/http/KeepAliveCache/B8293562.java index 2e6dbb84e2d..0a6e7ef1416 100644 --- a/test/jdk/sun/net/www/http/KeepAliveCache/B8293562.java +++ b/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(); } } }