changeset 59209:868fe697bad4

8062947: Fix exception message to correctly represent LDAP connection failure Reviewed-by: dfuchs, xyin, vtewari
author aefimov
date Thu, 07 May 2020 19:18:22 +0100
parents 73c3fe1eefd7
children ca61965cfcf4
files src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java src/java.naming/share/classes/com/sun/jndi/ldap/LdapRequest.java test/jdk/com/sun/jndi/ldap/NamingExceptionMessageTest.java
diffstat 3 files changed, 205 insertions(+), 51 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java	Thu May 07 11:13:21 2020 -0700
+++ b/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java	Thu May 07 19:18:22 2020 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2020, 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
@@ -419,22 +419,34 @@
             }
         }
 
+        NamingException namingException = null;
         try {
             // if no timeout is set so we wait infinitely until
-            // a response is received
+            // a response is received OR until the connection is closed or cancelled
             // http://docs.oracle.com/javase/8/docs/technotes/guides/jndi/jndi-ldap.html#PROP
             rber = ldr.getReplyBer(readTimeout);
         } catch (InterruptedException ex) {
             throw new InterruptedNamingException(
                 "Interrupted during LDAP operation");
+        } catch (CommunicationException ce) {
+            // Re-throw
+            throw ce;
+        } catch (NamingException ne) {
+            // Connection is timed out OR closed/cancelled
+            namingException = ne;
+            rber = null;
         }
 
         if (rber == null) {
             abandonRequest(ldr, null);
-            throw new NamingException(
-                    "LDAP response read timed out, timeout used:"
-                            + readTimeout + "ms." );
-
+        }
+        // namingException can be not null in the following cases:
+        //  a) The response is timed-out
+        //  b) LDAP request connection has been closed or cancelled
+        // The exception message is initialized in LdapRequest::getReplyBer
+        if (namingException != null) {
+            // Re-throw NamingException after all cleanups are done
+            throw namingException;
         }
         return rber;
     }
@@ -853,15 +865,6 @@
                     inbuf = Arrays.copyOf(inbuf, offset + left.length);
                     System.arraycopy(left, 0, inbuf, offset, left.length);
                     offset += left.length;
-/*
-if (dump > 0) {
-System.err.println("seqlen: " + seqlen);
-System.err.println("bufsize: " + offset);
-System.err.println("bytesleft: " + bytesleft);
-System.err.println("bytesread: " + bytesread);
-}
-*/
-
 
                     try {
                         retBer = new BerDecoder(inbuf, 0, offset);
@@ -969,36 +972,4 @@
         }
         return buf;
     }
-
-    // This code must be uncommented to run the LdapAbandonTest.
-    /*public void sendSearchReqs(String dn, int numReqs) {
-        int i;
-        String attrs[] = null;
-        for(i = 1; i <= numReqs; i++) {
-            BerEncoder ber = new BerEncoder(2048);
-
-            try {
-            ber.beginSeq(Ber.ASN_SEQUENCE | Ber.ASN_CONSTRUCTOR);
-                ber.encodeInt(i);
-                ber.beginSeq(LdapClient.LDAP_REQ_SEARCH);
-                    ber.encodeString(dn == null ? "" : dn);
-                    ber.encodeInt(0, LdapClient.LBER_ENUMERATED);
-                    ber.encodeInt(3, LdapClient.LBER_ENUMERATED);
-                    ber.encodeInt(0);
-                    ber.encodeInt(0);
-                    ber.encodeBoolean(true);
-                    LdapClient.encodeFilter(ber, "");
-                    ber.beginSeq(Ber.ASN_SEQUENCE | Ber.ASN_CONSTRUCTOR);
-                        ber.encodeStringArray(attrs);
-                    ber.endSeq();
-                ber.endSeq();
-            ber.endSeq();
-            writeRequest(ber, i);
-            //System.err.println("wrote request " + i);
-            } catch (Exception ex) {
-            //System.err.println("ldap.search: Caught " + ex + " building req");
-            }
-
-        }
-    } */
 }
--- a/src/java.naming/share/classes/com/sun/jndi/ldap/LdapRequest.java	Thu May 07 11:13:21 2020 -0700
+++ b/src/java.naming/share/classes/com/sun/jndi/ldap/LdapRequest.java	Thu May 07 19:18:22 2020 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2020, 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,11 +29,14 @@
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 import javax.naming.CommunicationException;
+import javax.naming.NamingException;
 import java.util.concurrent.TimeUnit;
 
 final class LdapRequest {
 
     private final static BerDecoder EOF = new BerDecoder(new byte[]{}, -1, 0);
+    private final static String CLOSE_MSG = "LDAP connection has been closed";
+    private final static String TIMEOUT_MSG_FMT = "LDAP response read timed out, timeout used: %d ms.";
 
     LdapRequest next;   // Set/read in synchronized Connection methods
     final int msgId;          // read-only
@@ -95,14 +98,22 @@
         return pauseAfterReceipt;
     }
 
-    BerDecoder getReplyBer(long millis) throws CommunicationException,
+    /**
+     * Read reply BER
+     * @param millis timeout, infinite if the value is negative
+     * @return BerDecoder if reply was read successfully
+     * @throws CommunicationException request has been canceled and request does not need to be abandoned
+     * @throws NamingException request has been closed or timed out. Request does need to be abandoned
+     * @throws InterruptedException LDAP operation has been interrupted
+     */
+    BerDecoder getReplyBer(long millis) throws NamingException,
                                                InterruptedException {
         if (cancelled) {
             throw new CommunicationException("Request: " + msgId +
                 " cancelled");
         }
         if (isClosed()) {
-            return null;
+            throw new NamingException(CLOSE_MSG);
         }
 
         BerDecoder result = millis > 0 ?
@@ -113,7 +124,15 @@
                 " cancelled");
         }
 
-        return result == EOF ? null : result;
+        // poll from 'replies' blocking queue ended-up with timeout
+        if (result == null) {
+            throw new NamingException(String.format(TIMEOUT_MSG_FMT, millis));
+        }
+        // Unexpected EOF can be caused by connection closure or cancellation
+        if (result == EOF) {
+            throw new NamingException(CLOSE_MSG);
+        }
+        return result;
     }
 
     boolean hasSearchCompleted() {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/com/sun/jndi/ldap/NamingExceptionMessageTest.java	Thu May 07 19:18:22 2020 +0100
@@ -0,0 +1,164 @@
+/*
+ * Copyright (c) 2020, 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 8062947
+ * @summary Test that NamingException message text matches the failure reason
+ * @library /test/lib lib
+ * @run testng NamingExceptionMessageTest
+ */
+
+import javax.naming.Context;
+import javax.naming.NamingException;
+import javax.naming.directory.InitialDirContext;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.util.Hashtable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.testng.annotations.Test;
+import org.testng.Assert;
+import jdk.test.lib.net.URIBuilder;
+
+public class NamingExceptionMessageTest {
+
+    @Test
+    public void timeoutMessageTest() throws Exception {
+        try (var ldapServer = TestLdapServer.newInstance(false)) {
+            ldapServer.start();
+            ldapServer.awaitStartup();
+            var env = ldapServer.getInitialLdapCtxEnvironment(TIMEOUT_VALUE);
+            var namingException = Assert.expectThrows(NamingException.class, () -> new InitialDirContext(env));
+            System.out.println("Got naming exception:" + namingException);
+            Assert.assertEquals(namingException.getMessage(), EXPECTED_TIMEOUT_MESSAGE);
+        }
+    }
+
+    @Test
+    public void connectionClosureMessageTest() throws Exception {
+        try (var ldapServer = TestLdapServer.newInstance(true)) {
+            ldapServer.start();
+            ldapServer.awaitStartup();
+            var env = ldapServer.getInitialLdapCtxEnvironment(0);
+            var namingException = Assert.expectThrows(NamingException.class, () -> new InitialDirContext(env));
+            System.out.println("Got naming exception:" + namingException);
+            Assert.assertEquals(namingException.getMessage(), EXPECTED_CLOSURE_MESSAGE);
+        }
+    }
+
+    // Test LDAP server
+    private static class TestLdapServer extends BaseLdapServer {
+
+        private final boolean closeConnections;
+        private final CountDownLatch startupLatch = new CountDownLatch(1);
+
+        public Hashtable<Object, Object> getInitialLdapCtxEnvironment(int readTimeoutValue) {
+            // Create environment for initial LDAP context
+            Hashtable<Object, Object> env = new Hashtable<>();
+
+            // Activate LDAPv3
+            env.put("java.naming.ldap.version", "3");
+
+            // De-activate the ManageDsaIT control
+            env.put(Context.REFERRAL, "follow");
+            env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
+            env.put(Context.PROVIDER_URL, getUrlString());
+            env.put(Context.SECURITY_AUTHENTICATION, "simple");
+            env.put(Context.SECURITY_PRINCIPAL, "name");
+            env.put(Context.SECURITY_CREDENTIALS, "pwd");
+
+            if (readTimeoutValue > 0) {
+                env.put("com.sun.jndi.ldap.read.timeout", String.valueOf(readTimeoutValue));
+                env.put("com.sun.jndi.ldap.connect.timeout", String.valueOf(readTimeoutValue));
+            }
+
+            return env;
+        }
+
+        private String getUrlString() {
+            String url = URIBuilder.newBuilder()
+                    .scheme("ldap")
+                    .loopback()
+                    .port(getPort())
+                    .buildUnchecked()
+                    .toString();
+            return url;
+        }
+
+        public static TestLdapServer newInstance(boolean closeConnections) throws IOException {
+            ServerSocket srvSock = new ServerSocket();
+            srvSock.bind(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0));
+            return new TestLdapServer(srvSock, closeConnections);
+        }
+
+        void awaitStartup() throws InterruptedException {
+            startupLatch.await();
+        }
+
+        private TestLdapServer(ServerSocket serverSocket, boolean closeConnections) {
+            super(serverSocket);
+            this.closeConnections = closeConnections;
+
+        }
+
+        @Override
+        protected void beforeAcceptingConnections() {
+            startupLatch.countDown();
+        }
+
+        @Override
+        protected void handleRequest(Socket socket,
+                                     LdapMessage msg,
+                                     OutputStream out)
+                throws IOException {
+            switch (msg.getOperation()) {
+                case BIND_REQUEST:
+                    if (closeConnections) {
+                        closeSilently(socket);
+                    } else {
+                        try {
+                            TimeUnit.DAYS.sleep(Integer.MAX_VALUE);
+                        } catch (InterruptedException e) {
+                            Thread.currentThread().interrupt();
+                        }
+                    }
+                default:
+                    break;
+            }
+        }
+    }
+
+    // Expected message for case when connection is closed on server side
+    private static final String EXPECTED_CLOSURE_MESSAGE = "LDAP connection has been closed";
+    // read and connect timeouts value
+    private static final int TIMEOUT_VALUE = 129;
+    // Expected message text when connection is timed-out
+    private static final String EXPECTED_TIMEOUT_MESSAGE = String.format(
+            "LDAP response read timed out, timeout used: %d ms.", TIMEOUT_VALUE);
+}