8228580: DnsClient TCP socket timeout

Reviewed-by: vtewari, chegar, prappo

Author: mmimica

src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2019, 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
@@ -29,7 +29,9 @@
 import java.net.DatagramSocket;
 import java.net.DatagramPacket;
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.net.SocketTimeoutException;
 import java.security.SecureRandom;
 import javax.naming.*;
 
@@ -82,7 +84,7 @@ public class DnsClient {
     private static final SecureRandom random = JCAUtil.getSecureRandom();
     private InetAddress[] servers;
     private int[] serverPorts;
-    private int timeout;                // initial timeout on UDP queries in ms
+    private int timeout;                // initial timeout on UDP and TCP queries in ms
     private int retries;                // number of UDP retries
 
     private final Object udpSocketLock = new Object();
@@ -100,7 +102,7 @@ public class DnsClient {
     /*
      * Each server is of the form "server[:port]".  IPv6 literal host names
      * include delimiting brackets.
-     * "timeout" is the initial timeout interval (in ms) for UDP queries,
+     * "timeout" is the initial timeout interval (in ms) for queries,
      * and "retries" gives the number of retries per server.
      */
     public DnsClient(String[] servers, int timeout, int retries)
@@ -237,14 +239,15 @@ ResourceRecords query(DnsName fqdn, int qclass, int qtype,
 
                             // Try each server, starting with the one that just
                             // provided the truncated message.
+                            int retryTimeout = (timeout * (1 << retry));
                             for (int j = 0; j < servers.length; j++) {
                                 int ij = (i + j) % servers.length;
                                 if (doNotRetry[ij]) {
                                     continue;
                                 }
                                 try {
                                     Tcp tcp =
-                                        new Tcp(servers[ij], serverPorts[ij]);
+                                        new Tcp(servers[ij], serverPorts[ij], retryTimeout);
                                     byte[] msg2;
                                     try {
                                         msg2 = doTcpQuery(tcp, pkt);
@@ -327,7 +330,7 @@ ResourceRecords queryZone(DnsName zone, int qclass, boolean recursion)
         // Try each name server.
         for (int i = 0; i < servers.length; i++) {
             try {
-                Tcp tcp = new Tcp(servers[i], serverPorts[i]);
+                Tcp tcp = new Tcp(servers[i], serverPorts[i], timeout);
                 byte[] msg;
                 try {
                     msg = doTcpQuery(tcp, pkt);
@@ -462,19 +465,19 @@ private byte[] doTcpQuery(Tcp tcp, Packet pkt) throws IOException {
      */
     private byte[] continueTcpQuery(Tcp tcp) throws IOException {
 
-        int lenHi = tcp.in.read();      // high-order byte of response length
+        int lenHi = tcp.read();      // high-order byte of response length
         if (lenHi == -1) {
             return null;        // EOF
         }
-        int lenLo = tcp.in.read();      // low-order byte of response length
+        int lenLo = tcp.read();      // low-order byte of response length
         if (lenLo == -1) {
             throw new IOException("Corrupted DNS response: bad length");
         }
         int len = (lenHi << 8) | lenLo;
         byte[] msg = new byte[len];
         int pos = 0;                    // next unfilled position in msg
         while (len > 0) {
-            int n = tcp.in.read(msg, pos, len);
+            int n = tcp.read(msg, pos, len);
             if (n == -1) {
                 throw new IOException(
                         "Corrupted DNS response: too little data");
@@ -682,20 +685,62 @@ private static void dprint(String mess) {
 
 class Tcp {
 
-    private Socket sock;
-    java.io.InputStream in;
-    java.io.OutputStream out;
+    private final Socket sock;
+    private final java.io.InputStream in;
+    final java.io.OutputStream out;
+    private int timeoutLeft;
 
-    Tcp(InetAddress server, int port) throws IOException {
-        sock = new Socket(server, port);
-        sock.setTcpNoDelay(true);
-        out = new java.io.BufferedOutputStream(sock.getOutputStream());
-        in = new java.io.BufferedInputStream(sock.getInputStream());
+    Tcp(InetAddress server, int port, int timeout) throws IOException {
+        sock = new Socket();
+        try {
+            long start = System.currentTimeMillis();
+            sock.connect(new InetSocketAddress(server, port), timeout);
+            timeoutLeft = (int) (timeout - (System.currentTimeMillis() - start));
+            if (timeoutLeft <= 0)
+                throw new SocketTimeoutException();
+
+            sock.setTcpNoDelay(true);
+            out = new java.io.BufferedOutputStream(sock.getOutputStream());
+            in = new java.io.BufferedInputStream(sock.getInputStream());
+        } catch (Exception e) {
+            try {
+                sock.close();
+            } catch (IOException ex) {
+                e.addSuppressed(ex);
+            }
+            throw e;
+        }
     }
 
     void close() throws IOException {
         sock.close();
     }
+
+    private interface SocketReadOp {
+        int read() throws IOException;
+    }
+
+    private int readWithTimeout(SocketReadOp reader) throws IOException {
+        if (timeoutLeft <= 0)
+            throw new SocketTimeoutException();
+
+        sock.setSoTimeout(timeoutLeft);
+        long start = System.currentTimeMillis();
+        try {
+            return reader.read();
+        }
+        finally {
+            timeoutLeft -= System.currentTimeMillis() - start;
+        }
+    }
+
+    int read() throws IOException {
+        return readWithTimeout(() -> in.read());
+    }
+
+    int read(byte b[], int off, int len) throws IOException {
+        return readWithTimeout(() -> in.read(b, off, len));
+    }
 }
 
 /*

src/jdk.naming.dns/share/classes/module-info.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, 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
@@ -26,7 +26,38 @@
 /**
  * Provides the implementation of the DNS Java Naming provider.
  *
+ * <h2>Environment Properties</h2>
+ *
+ * <p> The following JNDI environment properties may be used when creating
+ * the initial context.
+ *
+ * <ul>
+ *    <li>com.sun.jndi.dns.timeout.initial</li>
+ *    <li>com.sun.jndi.dns.timeout.retries</li>
+ *  </ul>
+ *
+ * <p> These properties are used to alter the timeout-related defaults that the
+ * DNS provider uses when submitting queries. The DNS provider submits queries
+ * using the following exponential backoff algorithm. The provider submits a
+ * query to a DNS server and waits for a response to arrive within a timeout
+ * period (1 second by default). If it receives no response within the timeout
+ * period, it queries the next server, and so on. If the provider receives no
+ * response from any server, it doubles the timeout period and repeats the
+ * process of submitting the query to each server, up to a maximum number of
+ * retries (4 by default).
+ *
+ * <p> The {@code com.sun.jndi.dns.timeout.initial} property, if set, specifies
+ * the number of milliseconds to use as the initial timeout period (i.e., before
+ * any doubling). If this property has not been set, the default initial timeout
+ * is 1000 milliseconds.
+ *
+ * <p> The {@code com.sun.jndi.dns.timeout.retries} property, if set, specifies
+ * the number of times to retry each server using the exponential backoff
+ * algorithm described previously. If this property has not been set, the
+ * default number of retries is 4.
+ *
  * @provides javax.naming.spi.InitialContextFactory
+ *
  * @moduleGraph
  * @since 9
  */

test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.dns

@@ -0,0 +1,45 @@
+#
+# Copyright (c) 2019, 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.
+#
+
+################################################################################
+# Capture file for TcpTimeout.java
+#
+# NOTE: This hexadecimal dump of DNS protocol messages was generated by
+#       running the GetEnv application program against a real DNS
+#       server along with DNSTracer
+#
+################################################################################
+
+# DNS Request
+
+0000: 32 72 01 00 00 01 00 00   00 00 00 00 05 68 6F 73  2r...........hos
+0010: 74 31 07 64 6F 6D 61 69   6E 31 03 63 6F 6D 00 00  t1.domain1.com..
+0020: FF 00 FF                                           ...
+
+
+# DNS Response
+
+0000: 32 72 82 00 00 01 00 06   00 01 00 01 05 68 6F 73  2r...........hos
+0010: 74 31 07 64 6F 6D 61 69   6E 31 03 63 6F 6D 00 00  t1.domain1.com..
+0020: FF 00 01 C0 0C 00 10 00   01 00 00 8C A0 00 15 14  ................
+0030: 41 20 76 65 72 79 20 70   6F 70 75 6C 61 72 20 68  A very popular h

test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java

@@ -0,0 +1,151 @@
+/*
+ * Copyright (c) 2019, 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.
+ */
+
+import jtreg.SkippedException;
+
+import javax.naming.directory.InitialDirContext;
+import java.io.IOException;
+import java.net.BindException;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+
+import static java.util.concurrent.TimeUnit.NANOSECONDS;
+import static jdk.test.lib.Utils.adjustTimeout;
+
+/*
+ * @test
+ * @bug 8228580
+ * @summary Tests that we get a DNS response when the UDP DNS server returns a
+ *          truncated response and the TCP DNS server does not respond at all
+ *          after connect.
+ * @library ../lib/
+ * @library /test/lib
+ * @modules java.base/sun.security.util
+ * @run main TcpTimeout
+ * @run main TcpTimeout -Dcom.sun.jndi.dns.timeout.initial=5000
+ */
+
+public class TcpTimeout extends DNSTestBase {
+    private TcpDnsServer tcpDnsServer;
+
+    /* The acceptable variation in timeout measurement. */
+    private static final long TOLERANCE = adjustTimeout(5_000);
+
+    /* The acceptable variation of early returns from timed socket operations. */
+    private static final long PREMATURE_RETURN = adjustTimeout(100);
+
+    public static void main(String[] args) throws Exception {
+        new TcpTimeout().run(args);
+    }
+
+    @Override
+    public void runTest() throws Exception {
+        /* The default timeout value is 1 second, as stated in the
+           jdk.naming.dns module docs. */
+        long timeout = 1_000;
+        var envTimeout = env().get("com.sun.jndi.dns.timeout.initial");
+        if (envTimeout != null)
+            timeout = Long.parseLong(String.valueOf(envTimeout));
+
+        setContext(new InitialDirContext(env()));
+
+        long startNanos = System.nanoTime();
+
+        /* perform query */
+        var attrs = context().getAttributes("host1");
+
+        long elapsed = NANOSECONDS.toMillis(System.nanoTime() - startNanos);
+        if (elapsed < timeout - PREMATURE_RETURN || elapsed > timeout + TOLERANCE) {
+            throw new RuntimeException(String.format(
+                    "elapsed=%s, timeout=%s, TOLERANCE=%s, PREMATURE_RETURN=%s",
+                    elapsed, timeout, TOLERANCE, PREMATURE_RETURN));
+        }
+
+        DNSTestUtils.debug(attrs);
+
+        /* Note that the returned attributes are truncated and the response
+        is not valid. */
+        var txtAttr = attrs.get("TXT");
+        if (txtAttr == null)
+            throw new RuntimeException("TXT attribute missing.");
+    }
+
+    @Override
+    public void initTest(String[] args) {
+        /* We need to bind the TCP server on the same port the UDP server is
+        listening to. This may not be possible if that port is in use. Retry
+        MAX_RETRIES times relying on UDP port randomness. */
+        final int MAX_RETRIES = 5;
+        for (int i = 0; i < MAX_RETRIES; i++) {
+            super.initTest(args);
+            var udpServer = (Server) env().get(DNSTestUtils.TEST_DNS_SERVER_THREAD);
+            int port = udpServer.getPort();
+            try {
+                tcpDnsServer = new TcpDnsServer(port);
+                break; // success
+            } catch (BindException be) {
+                DNSTestUtils.debug("Failed to bind server socket on port " + port
+                                           + ", retry no. " + (i + 1) + ", " + be.getMessage());
+            } catch (Exception ex) {
+                throw new RuntimeException("Unexpected exception during initTest", ex);
+            } finally {
+                if (tcpDnsServer == null) { // cleanup behind exceptions
+                    super.cleanupTest();
+                }
+            }
+        }
+
+        if (tcpDnsServer == null) {
+            throw new SkippedException("Cannot start TCP server after "
+                                               + MAX_RETRIES
+                                               + " tries, skip the test");
+        }
+    }
+
+    @Override
+    public void cleanupTest() {
+        super.cleanupTest();
+        if (tcpDnsServer != null)
+            tcpDnsServer.stopServer();
+    }
+
+    /**
+     * A TCP server that accepts a connection and does nothing else: causes read
+     * timeout on client side.
+     */
+    private static class TcpDnsServer {
+        final ServerSocket serverSocket;
+
+        TcpDnsServer(int port) throws IOException {
+            serverSocket = new ServerSocket(port, 0, InetAddress.getLoopbackAddress());
+            System.out.println("TcpDnsServer: listening on port " + port);
+        }
+
+        void stopServer() {
+            try {
+                if (serverSocket != null)
+                    serverSocket.close();
+            } catch (Exception ignored) { }
+        }
+    }
+}