Hi Pavel
Updated the patch with the jtreg test.
The test hangs when the fix is not applied. I don't know why
main/timeout=20 does not work for me.
On Tue, 20 Aug 2019 at 00:08, Pavel Rappo <[email protected]> wrote:
>
> Thanks for doing that. I've only skimmed through the patch and I’d recommend
> that no matter if the changes are adequate or not there should be a test [1]:
>
> "...A unit test, written for the jtreg test harness, that validates your
> change. The test should fail when run against a build without the change and
> succeed when run against a build with the change. Unit tests aren't always
> practical, especially for changes such as performance improvements or fixes
> to bugs that are highly platform-dependent, but if practical then a unit test
> is required."
>
> Please consider adding it to the patch while the changes are being
> (preliminary) reviewed.
>
> -Pavel
>
> -------------------------------------------------------------------------------
> [1] https://openjdk.java.net/contribute/
>
> > On 19 Aug 2019, at 17:20, Milan Mimica <[email protected]> wrote:
> >
> > Hello list
> >
> > Please find the attached patch that fixes JDK-8228580.
> >
> > It works the similar way UDP timeout does: calculate the initial
> > timeout from retry attempt, and account for duration of every blocking
> > call on the TCP socket.
> >
> > I am listed as Author[1] on "jdk" project.
> >
> > [1] https://openjdk.java.net/census#mmimica
> >
> >
> > --
> > Milan Mimica
> > <JDK-8228580.patch>
>
--
Milan Mimica
diff -r 56df9a08ed9c src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java
--- a/src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java Mon Aug 19 14:28:43 2019 +0100
+++ b/src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java Wed Aug 21 08:26:40 2019 +0000
@@ -30,6 +30,7 @@
import java.net.DatagramPacket;
import java.net.InetAddress;
import java.net.Socket;
+import java.net.SocketTimeoutException;
import java.security.SecureRandom;
import javax.naming.*;
@@ -82,7 +83,7 @@
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 +101,7 @@
/*
* 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,6 +238,7 @@
// 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]) {
@@ -244,7 +246,7 @@
}
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 +329,7 @@
// 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,11 +464,11 @@
*/
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");
}
@@ -474,7 +476,7 @@
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");
@@ -683,19 +685,47 @@
class Tcp {
private Socket sock;
- java.io.InputStream in;
+ private java.io.InputStream in;
java.io.OutputStream out;
+ private int timeoutLeft;
- Tcp(InetAddress server, int port) throws IOException {
+ Tcp(InetAddress server, int port, int timeout) throws IOException {
sock = new Socket(server, port);
sock.setTcpNoDelay(true);
out = new java.io.BufferedOutputStream(sock.getOutputStream());
in = new java.io.BufferedInputStream(sock.getInputStream());
+ timeoutLeft = timeout;
}
void close() throws IOException {
sock.close();
}
+
+ private interface SockerReadOp {
+ int read() throws IOException;
+ }
+
+ private int readWithTimeout(SockerReadOp 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));
+ }
}
/*
diff -r 56df9a08ed9c test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java Wed Aug 21 08:26:40 2019 +0000
@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2002, 2018, 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 javax.naming.CommunicationException;
+import javax.naming.Context;
+import javax.naming.directory.InitialDirContext;
+
+import java.lang.reflect.Array;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.net.SocketTimeoutException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Arrays;
+
+/*
+ * @test
+ * @bug 8228580
+ * @summary Tests that we get DNS response when the DNS UDP server returns a
+ * truncated response and TCP DNS server does not respond at all after
+ * connect.
+ * @library ../lib/
+ * @modules java.base/sun.security.util
+ * @run main/timeout=20 TcpTimeout
+ */
+
+public class TcpTimeout extends DNSTestBase {
+ private TcpDnsServer tcpDnsServer;
+
+ public static void main(String[] args) throws Exception {
+ new TcpTimeout().run(args);
+ }
+
+ @Override
+ public void runTest() throws Exception {
+ setContext(new InitialDirContext(env()));
+
+ /* perform query */
+ var attrs = context().getAttributes("host1");
+ DNSTestUtils.debug(attrs);
+
+ /* Note that the retunred attributes are truncated and the reponse is
+ not valid */
+ var txtAttr = attrs.get("TXT");
+ if (txtAttr == null)
+ throw new RuntimeException("TXT attribute missing.");
+
+ tcpDnsServer.stopServer();
+ }
+
+ @Override
+ public void initTest(String[] args) {
+ super.initTest(args);
+ tcpDnsServer = new TcpDnsServer();
+ }
+
+ @Override
+ public void cleanupTest() {
+ super.cleanupTest();
+ if (tcpDnsServer != null) {
+ try {
+ tcpDnsServer.stopServer();
+ }
+ catch (Exception e) {}
+ }
+ }
+
+ private class TcpDnsServer {
+ final Thread thread;
+ ServerSocket serverSocket;
+ Socket socket;
+
+ TcpDnsServer() {
+ var udpServer = (Server) env().get(DNSTestUtils.TEST_DNS_SERVER_THREAD);
+ int port = udpServer.getPort();
+ thread = new Thread(() -> {
+ try {
+ System.out.println(
+ "TcpDnsServer: listening on port " + port);
+ serverSocket = new ServerSocket(port);
+ socket = serverSocket.accept();
+ /* simulate timeout */
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ }, "TcpDnsServer");
+ thread.start();
+ }
+
+ void stopServer() throws Exception {
+ thread.interrupt();
+ if (socket != null)
+ socket.close();
+ if (serverSocket != null)
+ serverSocket.close();
+ thread.join();
+ }
+ }
+}
\ No newline at end of file
