8233141: DatagramSocket.send doesn't specify exception thrown when no target address

DatagramSocket and MulticastSocket send methods are changed to throw IllegalArgumentException if no target address can be determined.

Reviewed-by: alanb

src/java.base/share/classes/java/net/DatagramSocket.java

@@ -646,7 +646,9 @@ class DatagramSocket implements java.io.Closeable {
      *             if this socket has an associated channel,
      *             and the channel is in non-blocking mode.
      * @throws     IllegalArgumentException if the socket is connected,
-     *             and connected address and packet address differ.
+     *             and connected address and packet address differ, or
+     *             if the socket is not connected and the packet address
+     *             is not set.
      *
      * @see        java.net.DatagramPacket
      * @see        SecurityManager#checkMulticast(InetAddress)
@@ -655,12 +657,15 @@ class DatagramSocket implements java.io.Closeable {
      * @spec JSR-51
      */
     public void send(DatagramPacket p) throws IOException  {
-        InetAddress packetAddress = null;
         synchronized (p) {
             if (isClosed())
                 throw new SocketException("Socket is closed");
-            checkAddress (p.getAddress(), "send");
+            InetAddress packetAddress = p.getAddress();
+            checkAddress (packetAddress, "send");
             if (connectState == ST_NOT_CONNECTED) {
+                if (packetAddress == null) {
+                    throw new IllegalArgumentException("Address not set");
+                }
                 // check the address is ok with the security manager on every send.
                 SecurityManager security = System.getSecurityManager();
 
@@ -669,16 +674,15 @@ class DatagramSocket implements java.io.Closeable {
                 // while you are trying to send the packet for example
                 // after the security check but before the send.
                 if (security != null) {
-                    if (p.getAddress().isMulticastAddress()) {
-                        security.checkMulticast(p.getAddress());
+                    if (packetAddress.isMulticastAddress()) {
+                        security.checkMulticast(packetAddress);
                     } else {
-                        security.checkConnect(p.getAddress().getHostAddress(),
+                        security.checkConnect(packetAddress.getHostAddress(),
                                               p.getPort());
                     }
                 }
             } else {
                 // we're connected
-                packetAddress = p.getAddress();
                 if (packetAddress == null) {
                     p.setAddress(connectedAddress);
                     p.setPort(connectedPort);

src/java.base/share/classes/java/net/MulticastSocket.java

@@ -29,6 +29,7 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Set;
+import java.net.PortUnreachableException;
 
 /**
  * The multicast datagram socket class is useful for sending
@@ -648,6 +649,14 @@ class MulticastSocket extends DatagramSocket {
      * @throws     SecurityException  if a security manager exists and its
      *             {@code checkMulticast} or {@code checkConnect}
      *             method doesn't allow the send.
+     * @throws     PortUnreachableException may be thrown if the socket is connected
+     *             to a currently unreachable destination. Note, there is no
+     *             guarantee that the exception will be thrown.
+     * @throws     IllegalArgumentException if the socket is connected,
+     *             and connected address and packet address differ, or
+     *             if the socket is not connected and the packet address
+     *             is not set.
+     *
      *
      * @deprecated Use the following code or its equivalent instead:
      *  ......
@@ -667,32 +676,34 @@ class MulticastSocket extends DatagramSocket {
         throws IOException {
             if (isClosed())
                 throw new SocketException("Socket is closed");
-            checkAddress(p.getAddress(), "send");
             synchronized(ttlLock) {
                 synchronized(p) {
+                    InetAddress packetAddress = p.getAddress();
+                    checkAddress(packetAddress, "send");
                     if (connectState == ST_NOT_CONNECTED) {
+                        if (packetAddress == null) {
+                            throw new IllegalArgumentException("Address not set");
+                        }
                         // Security manager makes sure that the multicast address
                         // is allowed one and that the ttl used is less
                         // than the allowed maxttl.
                         SecurityManager security = System.getSecurityManager();
                         if (security != null) {
-                            if (p.getAddress().isMulticastAddress()) {
-                                security.checkMulticast(p.getAddress(), ttl);
+                            if (packetAddress.isMulticastAddress()) {
+                                security.checkMulticast(packetAddress, ttl);
                             } else {
-                                security.checkConnect(p.getAddress().getHostAddress(),
+                                security.checkConnect(packetAddress.getHostAddress(),
                                                       p.getPort());
                             }
                         }
                     } else {
                         // we're connected
-                        InetAddress packetAddress = null;
-                        packetAddress = p.getAddress();
                         if (packetAddress == null) {
                             p.setAddress(connectedAddress);
                             p.setPort(connectedPort);
                         } else if ((!packetAddress.equals(connectedAddress)) ||
                                    p.getPort() != connectedPort) {
-                            throw new SecurityException("connected address and packet address" +
+                            throw new IllegalArgumentException("connected address and packet address" +
                                                         " differ");
                         }
                     }

test/jdk/java/net/DatagramSocket/AddressNotSet.java

@@ -0,0 +1,150 @@
+/*
+ * 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.
+ */
+
+/*
+ * @test
+ * @bug 8233141
+ * @summary DatagramSocket.send should throw IllegalArgumentException
+ *          when the packet address is not correctly set.
+ * @run main AddressNotSet
+ */
+
+import java.net.DatagramPacket;
+import java.net.DatagramSocket;
+import java.net.InetAddress;
+import java.net.MulticastSocket;
+import java.net.SocketAddress;
+import java.nio.channels.DatagramChannel;
+
+import static java.lang.System.out;
+
+public class AddressNotSet {
+
+    final InetAddress loopbackAddress = InetAddress.getLoopbackAddress();
+    final DatagramSocket serversock;
+    int i;
+    AddressNotSet() throws Exception {
+        serversock = new DatagramSocket(0, loopbackAddress);
+    }
+
+    public static void main (String args[]) throws Exception {
+        new AddressNotSet().run();
+    }
+
+    public void run() throws Exception {
+        try (var ss = serversock) {
+            try (DatagramSocket sock = new DatagramSocket()) {
+                test(sock);
+            }
+            try (DatagramSocket sock = new MulticastSocket()) {
+                test(sock);
+            }
+            try (DatagramSocket sock = DatagramChannel.open().socket()) {
+                test(sock);
+            }
+        }
+    }
+
+    private void test(DatagramSocket sock) throws Exception {
+        out.println("Testing with " + sock.getClass());
+        InetAddress addr = loopbackAddress;
+        byte[] buf;
+        DatagramPacket p;
+        int port = serversock.getLocalPort();
+        SocketAddress connectedAddress = serversock.getLocalSocketAddress();
+
+        out.println("Checking send to non-connected address ...");
+        try {
+            out.println("Checking send with no packet address");
+            buf = ("Hello, server"+(++i)).getBytes();
+            p = new DatagramPacket(buf, buf.length);
+            sock.send(p);
+            throw new AssertionError("Expected IllegalArgumentException not received");
+        } catch (IllegalArgumentException x) {
+            out.println("Got expected exception: " + x);
+        }
+
+        out.println("Checking send to valid address");
+        buf = ("Hello, server"+(++i)).getBytes();
+        p = new DatagramPacket(buf, buf.length, addr, port);
+        sock.send(p);
+        serversock.receive(p);
+
+        out.println("Connecting to server address: " + connectedAddress);
+        sock.connect(connectedAddress);
+
+        try {
+            out.println("Checking send with different address than connected");
+            buf = ("Hello, server"+(++i)).getBytes();
+            p = new DatagramPacket(buf, buf.length, addr, port+1);
+            sock.send(p);
+            throw new AssertionError("Expected IllegalArgumentException not received");
+        } catch (IllegalArgumentException x) {
+            out.println("Got expected exception: " + x);
+        }
+
+        out.println("Checking send to valid address");
+        buf = ("Hello, server"+(++i)).getBytes();
+        p = new DatagramPacket(buf, buf.length, addr, port);
+        sock.send(p);
+        serversock.receive(p);
+
+        if (sock instanceof MulticastSocket) {
+            sock.disconnect();
+            testTTL((MulticastSocket)sock);
+        }
+    }
+
+    private void testTTL(MulticastSocket sock) throws Exception {
+        out.println("Testing deprecated send TTL with " + sock.getClass());
+        final byte ttl = 100;
+        InetAddress addr = loopbackAddress;
+        byte[] buf;
+        DatagramPacket p;
+        int port = serversock.getLocalPort();
+
+        out.println("Checking send to non-connected address ...");
+        try {
+            out.println("Checking send with no packet address");
+            buf = ("Hello, server"+(++i)).getBytes();
+            p = new DatagramPacket(buf, buf.length);
+            sock.send(p,ttl);
+            throw new AssertionError("Expected IllegalArgumentException not received");
+        } catch (IllegalArgumentException x) {
+            out.println("Got expected exception: " + x);
+        }
+
+        out.println("Connecting to connected address: " + sock);
+        sock.connect(addr, port);
+
+        try {
+            out.println("Checking send with different address than connected");
+            buf = ("Hello, server"+(++i)).getBytes();
+            p = new DatagramPacket(buf, buf.length, addr, port+1);
+            sock.send(p, ttl);
+            throw new AssertionError("Expected IllegalArgumentException not received");
+        } catch (IllegalArgumentException x) {
+            out.println("Got expected exception: " + x);
+        }
+    }
+}

test/jdk/java/net/DatagramSocket/SendDatagramToBadAddress.java

@@ -110,12 +110,17 @@ public class SendDatagramToBadAddress {
     }
 
     public void run() throws Exception {
-
         if (OSsupportsFeature()) {
             print ("running on OS that supports ICMP port unreachable");
         }
+        try (DatagramSocket sock = new DatagramSocket()) {
+            test(sock);
+        }
+    }
+
+    private void test(DatagramSocket sock) throws Exception {
+        print("Testing with " + sock.getClass());
         InetAddress addr = InetAddress.getLoopbackAddress();
-        DatagramSocket sock = new DatagramSocket();
         DatagramSocket serversock = new DatagramSocket(0);
         DatagramPacket p;
         byte[] buf;