8278067: Make HttpURLConnection default keep alive timeout configurable

Reviewed-by: dfuchs

src/java.base/share/classes/java/net/doc-files/net-properties.html

@@ -176,6 +176,16 @@ <H2>Misc HTTP URL stream protocol handler properties</H2>
 	If HTTP keepalive is enabled (see above) this value determines the
 	maximum number of idle connections that will be simultaneously kept
 	alive, per destination.</P>
+	<LI><P><B>{@systemProperty http.keepAlive.time.server}</B> and
+        <B>{@systemProperty http.keepAlive.time.proxy}</B> </P>
+	<P>These properties modify the behavior of the HTTP keepalive cache in the case
+        where the server (or proxy) has not specified a keepalive time. If the
+        property is set in this case, then idle connections will be closed after the 
+        specified number of seconds. If the property is set, and the server does
+        specify a keepalive time in a "Keep-Alive" response header, then the time specified 
+        by the server is used. If the property is not set and also the server
+        does not specify a keepalive time, then connections are kept alive for an
+        implementation defined time, assuming {@code http.keepAlive} is {@code true}.</P>
 	<LI><P><B>{@systemProperty http.maxRedirects}</B> (default: {@code 20})<BR>
 	This integer value determines the maximum number, for a given request,
 	of HTTP redirects that will be automatically followed by the

src/java.base/share/classes/sun/net/www/http/HttpClient.java

@@ -122,13 +122,13 @@ private static int getDefaultPort(String proto) {
                                          recomputing the value of keepingAlive */
     int keepAliveConnections = -1;    /* number of keep-alives left */
 
-    /**Idle timeout value, in milliseconds. Zero means infinity,
-     * iff keepingAlive=true.
-     * Unfortunately, we can't always believe this one.  If I'm connected
-     * through a Netscape proxy to a server that sent me a keep-alive
-     * time of 15 sec, the proxy unilaterally terminates my connection
-     * after 5 sec.  So we have to hard code our effective timeout to
-     * 4 sec for the case where we're using a proxy. *SIGH*
+    /*
+     * The timeout if specified by the server. Following values possible
+     *  0: the server specified no keep alive headers
+     * -1: the server provided "Connection: keep-alive" but did not specify a
+     *     a particular time in a "Keep-Alive:" headers
+     * Positive values are the number of seconds specified by the server
+     * in a "Keep-Alive" header
      */
     int keepAliveTimeout = 0;
 
@@ -235,7 +235,6 @@ public boolean getHttpKeepAliveSet() {
         return keepAliveProp;
     }
 
-
     public String getSpnegoCBT() {
         return spnegoCBT;
     }
@@ -898,7 +897,7 @@ private boolean parseHTTPHeader(MessageHeader responses, ProgressSource pi, Http
                             responses.findValue("Keep-Alive"));
                         /* default should be larger in case of proxy */
                         keepAliveConnections = p.findInt("max", usingProxy?50:5);
-                        keepAliveTimeout = p.findInt("timeout", usingProxy?60:5);
+                        keepAliveTimeout = p.findInt("timeout", -1);
                     }
                 } else if (b[7] != '0') {
                     /*
@@ -1151,6 +1150,10 @@ public String getProxyHostUsed() {
         }
     }
 
+    public boolean getUsingProxy() {
+        return usingProxy;
+    }
+
     /**
      * @return the proxy port being used for this client.  Meaningless
      *          if getProxyHostUsed() gives null.

src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java

@@ -41,6 +41,8 @@
 
 import jdk.internal.misc.InnocuousThread;
 import sun.security.action.GetIntegerAction;
+import sun.net.www.protocol.http.HttpURLConnection;
+import sun.util.logging.PlatformLogger;
 
 /**
  * A class that implements a cache of idle Http connections for keep-alive
@@ -54,6 +56,32 @@ public class KeepAliveCache
     @java.io.Serial
     private static final long serialVersionUID = -2937172892064557949L;
 
+    // Keep alive time set according to priority specified here:
+    // 1. If server specifies a time with a Keep-Alive header
+    // 2. If user specifies a time with system property below
+    // 3. Default values which depend on proxy vs server and whether
+    //    a Connection: keep-alive header was sent by server
+
+    // name suffixed with "server" or "proxy"
+    private static final String keepAliveProp = "http.keepAlive.time.";
+
+    private static final int userKeepAliveServer;
+    private static final int userKeepAliveProxy;
+
+    static final PlatformLogger logger = HttpURLConnection.getHttpLogger();
+
+    @SuppressWarnings("removal")
+    static int getUserKeepAliveSeconds(String type) {
+        int v = AccessController.doPrivileged(
+            new GetIntegerAction(keepAliveProp+type, -1)).intValue();
+        return v < -1 ? -1 : v;
+    }
+
+    static {
+        userKeepAliveServer = getUserKeepAliveSeconds("server");
+        userKeepAliveProxy = getUserKeepAliveSeconds("proxy");
+    }
+
     /* maximum # keep-alive connections to maintain at once
      * This should be 2 by the HTTP spec, but because we don't support pipe-lining
      * a larger value is more appropriate. So we now set a default of 5, and the value
@@ -127,10 +155,29 @@ public Void run() {
 
             if (v == null) {
                 int keepAliveTimeout = http.getKeepAliveTimeout();
-                v = new ClientVector(keepAliveTimeout > 0 ?
-                        keepAliveTimeout * 1000 : LIFETIME);
-                v.put(http);
-                super.put(key, v);
+                if (keepAliveTimeout == 0) {
+                    keepAliveTimeout = getUserKeepAlive(http.getUsingProxy());
+                    if (keepAliveTimeout == -1) {
+                        // same default for server and proxy
+                        keepAliveTimeout = 5;
+                    }
+                } else if (keepAliveTimeout == -1) {
+                    keepAliveTimeout = getUserKeepAlive(http.getUsingProxy());
+                    if (keepAliveTimeout == -1) {
+                        // different default for server and proxy
+                        keepAliveTimeout = http.getUsingProxy() ? 60 : 5;
+                    }
+                }
+                // at this point keepAliveTimeout is the number of seconds to keep
+                // alive, which could be 0, if the user specified 0 for the property
+                assert keepAliveTimeout >= 0;
+                if (keepAliveTimeout == 0) {
+                    http.closeServer();
+                } else {
+                    v = new ClientVector(keepAliveTimeout * 1000);
+                    v.put(http);
+                    super.put(key, v);
+                }
             } else {
                 v.put(http);
             }
@@ -139,6 +186,11 @@ public Void run() {
         }
     }
 
+    // returns the keep alive set by user in system property or -1 if not set
+    private static int getUserKeepAlive(boolean isProxy) {
+        return isProxy ? userKeepAliveProxy : userKeepAliveServer;
+    }
+
     /* remove an obsolete HttpClient from its VectorCache */
     public void remove(HttpClient h, Object obj) {
         cacheLock.lock();
@@ -277,6 +329,11 @@ HttpClient get() {
                     e.hc.closeServer();
                 } else {
                     hc = e.hc;
+                    if (KeepAliveCache.logger.isLoggable(PlatformLogger.Level.FINEST)) {
+                        String msg = "cached HttpClient was idle for "
+                                + Long.toString(currentTime - e.idleStartTime);
+                        KeepAliveCache.logger.finest(msg);
+                    }
                 }
             } while ((hc == null) && (!isEmpty()));
             return hc;

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

@@ -0,0 +1,178 @@
+/*
+ * Copyright (c) 2022, 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
+ * @library /test/lib
+ * @bug 8278067
+ * @run main/othervm -Dhttp.keepAlive.time.server=30 KeepAliveProperty long
+ * @run main/othervm -Dhttp.keepAlive.time.server=1 KeepAliveProperty short
+ * @run main/othervm -ea -Dhttp.keepAlive.time.server=0 KeepAliveProperty short
+ */
+
+import java.net.*;
+import java.io.*;
+import java.nio.charset.*;
+import java.util.logging.*;
+import jdk.test.lib.net.URIBuilder;
+import static java.net.Proxy.NO_PROXY;
+
+public class KeepAliveProperty {
+
+    static volatile boolean pass = false;
+
+    static class Server extends Thread {
+        final ServerSocket server;
+
+        Server (ServerSocket server) {
+            super ();
+            this.server = server;
+        }
+
+        void readAll (Socket s) throws IOException {
+            byte[] buf = new byte [128];
+            int c;
+            String request = "";
+            InputStream is = s.getInputStream ();
+            while ((c=is.read(buf)) > 0) {
+                request += new String(buf, 0, c, StandardCharsets.US_ASCII);
+                if (request.contains("\r\n\r\n")) {
+                    return;
+                }
+            }
+            if (c == -1)
+                throw new IOException("Socket closed");
+        }
+
+        Socket s = null;
+        String BODY;
+        String CLEN;
+        PrintStream out;
+
+        public void run() {
+            try {
+                s = server.accept();
+                readAll(s);
+
+                BODY = "Hello world";
+                CLEN = "Content-Length: " + BODY.length() + "\r\n";
+                out = new PrintStream(new BufferedOutputStream(s.getOutputStream() ));
+
+                /* send the header */
+                out.print("HTTP/1.1 200 OK\r\n");
+                out.print("Content-Type: text/plain; charset=iso-8859-1\r\n");
+                out.print(CLEN);
+                out.print("\r\n");
+                out.print(BODY);
+                out.flush();
+            } catch (Exception e) {
+                pass = false;
+                try {
+                    if (s != null)
+                        s.close();
+                    server.close();
+                } catch (IOException unused) {}
+                return;
+            }
+
+            // second request may legitimately fail
+
+            try (Socket s2 = s; ServerSocket server2 = server; PrintStream out2 = out) {
+                // wait for second request.
+                readAll(s2);
+
+                BODY = "Goodbye world";
+                CLEN = "Content-Length: " + BODY.length() + "\r\n";
+
+                /* send the header */
+                out2.print("HTTP/1.1 200 OK\r\n");
+                out2.print("Content-Type: text/plain; charset=iso-8859-1\r\n");
+                out2.print(CLEN);
+                out2.print("\r\n");
+                out2.print(BODY);
+                out2.flush();
+                pass = !expectClose;
+                if (!pass) System.out.println("Failed: expected close");
+            } catch (Exception e) {
+                pass = expectClose;
+                if (!pass) System.out.println("Failed: did not expect close");
+            }
+        }
+    }
+
+    static String fetch(URL url) throws Exception {
+        InputStream in = url.openConnection(NO_PROXY).getInputStream();
+        String s = "";
+        byte b[] = new byte[128];
+        int n;
+        do {
+            n = in.read(b);
+            if (n > 0)
+                s += new String(b, 0, n, StandardCharsets.US_ASCII);
+        } while (n > 0);
+        in.close();
+        return s;
+    }
+
+    static volatile boolean expectClose;
+
+    public static void main(String args[]) throws Exception {
+        // exercise the logging code
+        Logger logger = Logger.getLogger("sun.net.www.protocol.http.HttpURLConnection");
+        logger.setLevel(Level.FINEST);
+        ConsoleHandler h = new ConsoleHandler();
+        h.setLevel(Level.FINEST);
+        logger.addHandler(h);
+
+        expectClose = args[0].equals("short");
+        InetAddress loopback = InetAddress.getLoopbackAddress();
+        ServerSocket ss = new ServerSocket();
+        ss.bind(new InetSocketAddress(loopback, 0));
+        Server s = new Server(ss);
+        s.start();
+
+        URL url = URIBuilder.newBuilder()
+            .scheme("http")
+            .loopback()
+            .port(ss.getLocalPort())
+            .toURL();
+        System.out.println("URL: " + url);
+
+        if (!fetch(url).equals("Hello world"))
+            throw new RuntimeException("Failed on first request");
+
+        // Wait a while to see if connection is closed
+        Thread.sleep(3 * 1000);
+
+        try {
+            if (!fetch(url).equals("Goodbye world"))
+                throw new RuntimeException("Failed on second request");
+        } catch (Exception e) {
+            if (!expectClose)
+                throw e;
+        }
+
+        if (!pass)
+            throw new RuntimeException("Failed in server");
+    }
+}