8273402: Use derived NamingExceptions in com.sun.jndi.ldap.Connection#readReply

Reviewed-by: dfuchs

src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java

@@ -425,7 +425,7 @@ public final class Connection implements Runnable {
     /**
      * Reads a reply; waits until one is ready.
      */
-    BerDecoder readReply(LdapRequest ldr) throws IOException, NamingException {
+    BerDecoder readReply(LdapRequest ldr) throws NamingException {
         BerDecoder rber;
 
         // If socket closed, don't even try
@@ -436,7 +436,7 @@ public final class Connection implements Runnable {
             }
         }
 
-        NamingException namingException = null;
+        IOException ioException = null;
         try {
             // if no timeout is set so we wait infinitely until
             // a response is received OR until the connection is closed or cancelled
@@ -445,25 +445,29 @@ public final class Connection implements Runnable {
         } catch (InterruptedException ex) {
             throw new InterruptedNamingException(
                 "Interrupted during LDAP operation");
-        } catch (CommunicationException ce) {
+        } catch (IOException ioe) {
-            // Re-throw
-            throw ce;
-        } catch (NamingException ne) {
             // Connection is timed out OR closed/cancelled
-            namingException = ne;
+            // getReplyBer throws IOException when the requests needs to be abandoned
+            ioException = ioe;
             rber = null;
         }
 
         if (rber == null) {
             abandonRequest(ldr, null);
         }
-        // namingException can be not null in the following cases:
+        // ioException can be not null in the following cases:
         //  a) The response is timed-out
-        //  b) LDAP request connection has been closed or cancelled
+        //  b) LDAP request connection has been closed
+        // If the request has been cancelled - CommunicationException is
+        // thrown directly from LdapRequest.getReplyBer, since there is no
+        // need to abandon request.
         // The exception message is initialized in LdapRequest::getReplyBer
-        if (namingException != null) {
+        if (ioException != null) {
-            // Re-throw NamingException after all cleanups are done
+            // Throw CommunicationException after all cleanups are done
-            throw namingException;
+            String message = ioException.getMessage();
+            var ce = new CommunicationException(message);
+            ce.initCause(ioException);
+            throw ce;
         }
         return rber;
     }

src/java.naming/share/classes/com/sun/jndi/ldap/LdapRequest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2021, 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
@@ -103,17 +103,17 @@ final class LdapRequest {
      * @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 IOException            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,
+    BerDecoder getReplyBer(long millis) throws IOException, CommunicationException,
                                                InterruptedException {
         if (cancelled) {
             throw new CommunicationException("Request: " + msgId +
                 " cancelled");
         }
         if (isClosed()) {
-            throw new NamingException(CLOSE_MSG);
+            throw new IOException(CLOSE_MSG);
         }
 
         BerDecoder result = millis > 0 ?
@@ -126,11 +126,11 @@ final class LdapRequest {
 
         // poll from 'replies' blocking queue ended-up with timeout
         if (result == null) {
-            throw new NamingException(String.format(TIMEOUT_MSG_FMT, millis));
+            throw new IOException(String.format(TIMEOUT_MSG_FMT, millis));
         }
         // Unexpected EOF can be caused by connection closure or cancellation
         if (result == EOF) {
-            throw new NamingException(CLOSE_MSG);
+            throw new IOException(CLOSE_MSG);
         }
         return result;
     }

test/jdk/com/sun/jndi/ldap/NamingExceptionMessageTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, 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
@@ -23,12 +23,14 @@
 
 /*
  * @test
- * @bug 8062947
+ * @bug 8062947 8273402
- * @summary Test that NamingException message text matches the failure reason
+ * @summary Test that CommunicationException is thrown when connection is timed out or closed/cancelled,
+ *  and it's text matches the failure reason.
  * @library /test/lib lib
  * @run testng NamingExceptionMessageTest
  */
 
+import javax.naming.CommunicationException;
 import javax.naming.Context;
 import javax.naming.NamingException;
 import javax.naming.ServiceUnavailableException;
@@ -55,9 +57,10 @@ public class NamingExceptionMessageTest {
             ldapServer.start();
             ldapServer.awaitStartup();
             var env = ldapServer.getInitialLdapCtxEnvironment(TIMEOUT_VALUE);
-            var namingException = Assert.expectThrows(NamingException.class, () -> new InitialDirContext(env));
+            var communicationException =
-            System.out.println("Got naming exception:" + namingException);
+                    Assert.expectThrows(CommunicationException.class, () -> new InitialDirContext(env));
-            Assert.assertEquals(namingException.getMessage(), EXPECTED_TIMEOUT_MESSAGE);
+            System.out.println("Got CommunicationException:" + communicationException);
+            Assert.assertEquals(communicationException.getMessage(), EXPECTED_TIMEOUT_MESSAGE);
         }
     }
 
@@ -74,9 +77,12 @@ public class NamingExceptionMessageTest {
                 // read-out of the reply message. For such cases test run is considered as successful.
                 System.out.println("Got ServiceUnavailableException: Test PASSED");
             } else {
-                // If exception is not ServiceUnavailableException - check the exception message
+                // If exception is not ServiceUnavailableException, CommunicationException is expected
-                System.out.println("Got NamingException:" + namingException);
+                Assert.assertTrue(namingException instanceof CommunicationException);
-                Assert.assertEquals(namingException.getMessage(), EXPECTED_CLOSURE_MESSAGE);
+                var communicationException = (CommunicationException) namingException;
+                System.out.println("Got CommunicationException:" + communicationException);
+                // Check exception message
+                Assert.assertEquals(communicationException.getMessage(), EXPECTED_CLOSURE_MESSAGE);
             }
         }
     }