Hi
Indeed I have forgotten. Re-attached, with aesthetic fixes to the test.
On Thu, 22 Aug 2019 at 05:38, Vyom Tiwari <[email protected]> wrote:
>
> Hi Milan,
>
> Your test need the corresponding "TcpTimeout.dns" file to run successfully, I
> believe you forgot to add with your patch.
> please check the existing tests in the same folder if you need any additional
> information.
>
> Thanks,
> Vyom
>
> On Wed, Aug 21, 2019 at 10:48 PM Milan Mimica <[email protected]> wrote:
>>
>> 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
>
>
>
> --
> Thanks,
> Vyom
--
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 Thu Aug 22 07:19:58 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.dns
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.dns Thu Aug 22 07:19:58 2019 +0000
@@ -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
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 Thu Aug 22 07:19:58 2019 +0000
@@ -0,0 +1,110 @@
+/*
+ * 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 javax.naming.CommunicationException;
+import javax.naming.Context;
+import javax.naming.directory.InitialDirContext;
+
+import java.net.ServerSocket;
+import java.net.Socket;
+
+/*
+ * @test
+ * @bug 8228580
+ * @summary Tests that we get DNS response when the UDH DNS 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 returned attributes are truncated and the reponse is
+ not valid. */
+ var txtAttr = attrs.get("TXT");
+ if (txtAttr == null)
+ throw new RuntimeException("TXT attribute missing.");
+ }
+
+ @Override
+ public void initTest(String[] args) {
+ super.initTest(args);
+ var udpServer = (Server) env().get(DNSTestUtils.TEST_DNS_SERVER_THREAD);
+ tcpDnsServer = new TcpDnsServer(udpServer.getPort());
+ }
+
+ @Override
+ public void cleanupTest() {
+ super.cleanupTest();
+ if (tcpDnsServer != null) {
+ try {
+ tcpDnsServer.stopServer();
+ }
+ catch (Exception e) {}
+ }
+ }
+
+ private static class TcpDnsServer {
+ final Thread thread;
+ ServerSocket serverSocket;
+ Socket socket;
+
+ TcpDnsServer(int port) {
+ thread = new Thread(() -> {
+ System.out.println("TcpDnsServer: listening on port " + port);
+ try {
+ serverSocket = new ServerSocket(port);
+ socket = serverSocket.accept();
+ /* do nothing, 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();
+ }
+ }
+}
