6774170: LocalRMIServerSocketFactory should protect against ServerSocket.accept().getInetAddress() being null

Reviewed-by: emcmanus, jfdenise

jdk/src/share/classes/sun/management/jmxremote/LocalRMIServerSocketFactory.java

@@ -49,13 +49,34 @@ public final class LocalRMIServerSocketFactory implements RMIServerSocketFactory
         return new ServerSocket(port) {
             @Override
             public Socket accept() throws IOException {
-                Socket socket = super.accept();
-                InetAddress remoteAddr = socket.getInetAddress();
+                final Socket socket = super.accept();
+                final InetAddress remoteAddr = socket.getInetAddress();
                 final String msg = "The server sockets created using the " +
                        "LocalRMIServerSocketFactory only accept connections " +
                        "from clients running on the host where the RMI " +
                        "remote objects have been exported.";
-                if (remoteAddr.isAnyLocalAddress()) {
+
+                if (remoteAddr == null) {
+                    // Though unlikeky, the socket could be already
+                    // closed... Send a more detailed message in
+                    // this case. Also avoid throwing NullPointerExceptiion
+                    //
+                    String details = "";
+                    if (socket.isClosed()) {
+                        details = " Socket is closed.";
+                    } else if (!socket.isConnected()) {
+                        details = " Socket is not connected";
+                    }
+                    try {
+                        socket.close();
+                    } catch (Exception ok) {
+                        // ok - this is just cleanup before throwing detailed
+                        // exception.
+                    }
+                    throw new IOException(msg +
+                            " Couldn't determine client address." +
+                            details);
+                } else if (remoteAddr.isLoopbackAddress()) {
                     // local address: accept the connection.
                     return socket;
                 }

jdk/test/sun/management/jmxremote/LocalRMIServerSocketFactoryTest.java

@@ -0,0 +1,145 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc.  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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/**
+ *  @test LocalRMIServerSocketFactoryTest.java
+ *  @bug 6774170
+ *  @summary Connect to a server socket returned by the LocalRMIServerSocketFactory.
+ *
+ *  @author Daniel Fuchs
+ *
+ *  @run compile -XDignore.symbol.file=true -source 1.6 -g LocalRMIServerSocketFactoryTest.java
+ *  @run main LocalRMIServerSocketFactoryTest
+ */
+
+import sun.management.jmxremote.LocalRMIServerSocketFactory;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.util.concurrent.SynchronousQueue;
+
+public class LocalRMIServerSocketFactoryTest {
+
+    private static final SynchronousQueue<Exception> queue =
+            new SynchronousQueue<Exception>();
+
+    static final class Result extends Exception {
+
+        private Result() {
+            super("SUCCESS: No exception was thrown");
+        }
+        static final Result SUCCESS = new Result();
+    }
+
+    private static void checkError(String message) throws Exception {
+
+        // Wait for the server to set the error field.
+        final Exception x = queue.take();
+
+        if (x == Result.SUCCESS) {
+            return;
+        }
+
+
+        // case of 6674166: this is very unlikely to happen, even if
+        //     both 6674166 and 6774170 aren't fixed. If it happens
+        //     however, it might indicate that neither defects are fixed.
+
+        if (x instanceof NullPointerException) {
+            throw new Exception(message + " - " +
+                    "Congratulations! it seems you have triggered 6674166. " +
+                    "Neither 6674166 nor 6774170 seem to be fixed: " + x, x);
+        } else if (x instanceof IOException) {
+            throw new Exception(message + " - " +
+                    "Unexpected IOException. Maybe you triggered 6674166? " +
+                    x, x);
+        } else if (x != null) {
+            throw new Exception(message + " - " +
+                    "Ouch, that's bad. " +
+                    "This is a new kind of unexpected exception " +
+                    x, x);
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        final LocalRMIServerSocketFactory f =
+                new LocalRMIServerSocketFactory();
+        final ServerSocket s = f.createServerSocket(0);
+        final int port = s.getLocalPort();
+        Thread t = new Thread() {
+
+            public void run() {
+                while (true) {
+                    Exception error = Result.SUCCESS;
+                    try {
+                        System.err.println("Accepting: ");
+                        final Socket ss = s.accept();
+                        System.err.println(ss.getInetAddress() + " accepted");
+                    } catch (Exception x) {
+                        x.printStackTrace();
+                        error = x;
+                    } finally {
+                        try {
+                            // wait for the client to get the exception.
+                            queue.put(error);
+                        } catch (Exception x) {
+                            // too bad!
+                            System.err.println("Could't send result to client!");
+                            x.printStackTrace();
+                            return;
+                        }
+                    }
+                }
+            }
+        };
+        t.setDaemon(true);
+        t.start();
+
+        System.err.println("new Socket((String)null, port)");
+        final Socket s1 = new Socket((String) null, port);
+        checkError("new Socket((String)null, port)");
+        s1.close();
+        System.err.println("new Socket((String)null, port): PASSED");
+
+        System.err.println("new Socket(InetAddress.getByName(null), port)");
+        final Socket s2 = new Socket(InetAddress.getByName(null), port);
+        checkError("new Socket(InetAddress.getByName(null), port)");
+        s2.close();
+        System.err.println("new Socket(InetAddress.getByName(null), port): PASSED");
+
+        System.err.println("new Socket(localhost, port)");
+        final Socket s3 = new Socket("localhost", port);
+        checkError("new Socket(localhost, port)");
+        s3.close();
+        System.err.println("new Socket(localhost, port): PASSED");
+
+        System.err.println("new Socket(127.0.0.1, port)");
+        final Socket s4 = new Socket("127.0.0.1", port);
+        checkError("new Socket(127.0.0.1, port)");
+        s4.close();
+        System.err.println("new Socket(127.0.0.1, port): PASSED");
+
+    }
+}