8237890: DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set

This fix changes the default port of a DatagramPacket from -1 to 0, which changes the behaviour of calling getSocketAddress with no port set from throwing an IAE to returning an InetSocketAddress representing any local address with port 0.

Reviewed-by: alanb, chegar, dfuchs

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

@@ -90,8 +90,6 @@ class DatagramPacket {
      */
     public DatagramPacket(byte buf[], int offset, int length) {
         setData(buf, offset, length);
-        this.address = null;
-        this.port = -1;
     }
 
     /**
@@ -214,7 +212,8 @@ class DatagramPacket {
 
     /**
      * Returns the IP address of the machine to which this datagram is being
-     * sent or from which the datagram was received.
+     * sent or from which the datagram was received, or {@code null} if not
+     * set.
      *
      * @return  the IP address of the machine to which this datagram is being
      *          sent or from which the datagram was received.
@@ -227,7 +226,7 @@ class DatagramPacket {
 
     /**
      * Returns the port number on the remote host to which this datagram is
-     * being sent or from which the datagram was received.
+     * being sent or from which the datagram was received, or 0 if not set.
      *
      * @return  the port number on the remote host to which this datagram is
      *          being sent or from which the datagram was received.
@@ -363,8 +362,10 @@ class DatagramPacket {
     }
 
     /**
-     * Gets the SocketAddress (usually IP address + port number) of the remote
+     * Returns the {@link InetSocketAddress#InetSocketAddress(InetAddress, int)
-     * host that this packet is being sent to or is coming from.
+     * SocketAddress} (usually {@linkplain #getAddress() IP address} +
+     * {@linkplain #getPort() port number}) of the remote host that this packet
+     * is being sent to or is coming from.
      *
      * @return  the {@code SocketAddress}
      *

src/java.base/share/classes/sun/nio/ch/DatagramSocketAdaptor.java

@@ -209,7 +209,6 @@ public class DatagramSocketAdaptor
                     p.setPort(remote.getPort());
                     target = remote;
                 } else {
-                    // throws IllegalArgumentException if port not set
                     target = (InetSocketAddress) p.getSocketAddress();
                 }
             }

test/jdk/java/net/DatagramPacket/Constructor.java

@@ -35,6 +35,7 @@ import java.net.InetSocketAddress;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.expectThrows;
 
 public class Constructor {
@@ -116,4 +117,15 @@ public class Constructor {
                 packet.getAddress() != LOOPBACK ||
                 packet.getPort() != port), "full constructor failed");
     }
+
+    @Test
+    public void testDefaultValues() {
+        DatagramPacket packet = new DatagramPacket(buf, 0);
+        assertTrue(packet.getAddress() == null);
+        assertTrue(packet.getPort() == 0);
+
+        DatagramPacket packet1 = new DatagramPacket(buf, 0, 0);
+        assertTrue(packet1.getAddress() == null);
+        assertTrue(packet1.getPort() == 0);
+    }
 }

test/jdk/java/net/DatagramPacket/Getters.java

@@ -0,0 +1,47 @@
+/*
+ * 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 8237890
+ * @summary Check that DatagramPacket's get methods perform as expected
+ * @run testng Getters
+ */
+
+import org.testng.annotations.Test;
+
+import java.net.DatagramPacket;
+import java.net.InetSocketAddress;
+
+import static org.testng.Assert.assertTrue;
+
+public class Getters {
+
+    @Test
+    public void testDefaultGetSocketAddress() {
+        DatagramPacket packet = new DatagramPacket(new byte[128], 0);
+        InetSocketAddress addr = (InetSocketAddress)packet.getSocketAddress();
+
+        assertTrue(addr.getAddress().isAnyLocalAddress());
+        assertTrue(addr.getPort() == 0);
+    }
+}

test/jdk/java/net/DatagramSocket/SendCheck.java

@@ -27,6 +27,7 @@ import java.net.DatagramSocket;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.MulticastSocket;
+import java.net.SocketException;
 import java.nio.ByteBuffer;
 import java.nio.channels.DatagramChannel;
 import java.util.ArrayList;
@@ -39,11 +40,10 @@ import org.testng.annotations.DataProvider;
 
 import static org.testng.Assert.expectThrows;
 
-
 /*
  * @test
  * @bug 8236105
- * @summary check that DatagramSocket, MulticastSocket,
+ * @summary Check that DatagramSocket, MulticastSocket,
  *          DatagramSocketAdaptor and DatagramChannel all
  *          throw expected Execption when passed a DatagramPacket
  *          with invalid details
@@ -51,8 +51,9 @@ import static org.testng.Assert.expectThrows;
  */
 
 public class SendCheck {
+    private InetAddress loopbackAddr, wildcardAddr;
     static final Class<IOException> IOE = IOException.class;
-    static final Class<IllegalArgumentException> IAE = IllegalArgumentException.class;
+    static final Class<SocketException> SE = SocketException.class;
 
     static final byte[] buf = {0, 1, 2};
     static DatagramSocket socket;
@@ -100,85 +101,69 @@ public class SendCheck {
     }
 
     @DataProvider(name = "packets")
-    static Object[][] providerIO() throws IOException {
+    Object[][] providerIO() throws IOException {
-        var wildcard = new InetSocketAddress(0);
+        loopbackAddr = InetAddress.getLoopbackAddress();
+        wildcardAddr = new InetSocketAddress(0).getAddress();
 
-        /*
+        // loopback addr with no port set
-        Commented until JDK-8236852 is fixed
-
-        // loopback w/port 0 -- DC, DSA, MS, DS throws IO
         var pkt1 = new DatagramPacket(buf, 0, buf.length);
-        pkt1.setAddress(InetAddress.getLoopbackAddress());
+        pkt1.setAddress(loopbackAddr);
-        pkt1.setPort(0);
-         */
 
-        /*
+        // wildcard addr with no port set
-        Commented until JDK-8236852 is fixed
-
-        // wildcard w/port 0 -- DC, DSA, MS, DS throws IO
         var pkt2 = new DatagramPacket(buf, 0, buf.length);
-        pkt2.setAddress(wildcard.getAddress());
+        pkt2.setAddress(wildcardAddr);
-        pkt2.setPort(0);
-        */
-
-        // loopback w/port -1 -- DC, DSA, MS, DS throws IAE
-        var pkt3 = new DatagramPacket(buf, 0, buf.length);
-        pkt3.setAddress(InetAddress.getLoopbackAddress());
-
-        // wildcard w/port -1 -- DC, DSA, MS, DS throws IAE
-        var pkt4 = new DatagramPacket(buf, 0, buf.length);
-        pkt4.setAddress(wildcard.getAddress());
 
         /*
         Commented until JDK-8236807 is fixed
 
-        // wildcard addr w/valid port -- DS, MS throws IO ; DC, DSA doesn't throw
+        // wildcard addr w/valid port
-        var pkt5 = new DatagramPacket(buf, 0, buf.length);
+        var pkt3 = new DatagramPacket(buf, 0, buf.length);
-        var addr1 = wildcard.getAddress();
+        pkt3.setAddress(wildcardAddr);
-        pkt5.setAddress(addr1);
+        pkt3.setPort(socket.getLocalPort());
-        pkt5.setPort(socket.getLocalPort());
         */
 
-        // PKTS 3 & 4: invalid port -1
+        List<Packet> Packets = List.of(Packet.of(pkt1), Packet.of(pkt2));
-        List<Packet> iaePackets = List.of(Packet.of(pkt3), Packet.of(pkt4));
 
         List<Sender> senders = List.of(
                 Sender.of(new DatagramSocket(null)),
                 Sender.of(new MulticastSocket(null), (byte) 0),
                 Sender.of(DatagramChannel.open()),
-                Sender.of(DatagramChannel.open().socket())
+                Sender.of(DatagramChannel.open().socket()),
+                Sender.of((MulticastSocket)
+                        DatagramChannel.open().socket(), (byte) 0)
         );
 
         List<Object[]> testcases = new ArrayList<>();
-        for (var p : iaePackets) {
+        for (var packet : Packets) {
-            addTestCaseFor(testcases, senders, p, IAE);
+            addTestCaseFor(testcases, senders, packet);
         }
 
         return testcases.toArray(new Object[0][0]);
     }
 
     static void addTestCaseFor(List<Object[]> testcases,
-                               List<Sender> senders, Packet p,
+                               List<Sender> senders, Packet p) {
-                               Class<? extends Throwable> exception) {
         for (var s : senders) {
-            Object[] testcase = new Object[]{s, p, exception};
+            Object[] testcase = new Object[]{s, p, s.expectedException()};
             testcases.add(testcase);
         }
     }
 
     @Test(dataProvider = "packets")
-    public static void channelSendCheck(Sender<IOException> sender,
+    public static void sendCheck(Sender<IOException> sender,
                                         Packet packet,
                                         Class<? extends Throwable> exception) {
         DatagramPacket pkt = packet.packet;
         if (exception != null) {
             Throwable t = expectThrows(exception, () -> sender.send(pkt));
-            System.out.printf("%s got expected exception %s%n", packet.toString(), t);
+            System.out.printf("%s got expected exception %s%n",
+                    packet.toString(), t);
         } else {
             try {
                 sender.send(pkt);
-            } catch (IOException x) {
+            } catch (IOException e) {
-                throw new AssertionError("Unexpected exception for " + sender + " / " + packet, x);
+                throw new AssertionError("Unexpected exception for "
+                        + sender + " / " + packet, e);
             }
         }
     }
@@ -188,21 +173,23 @@ public class SendCheck {
 
         void close() throws E;
 
+        Class<? extends E> expectedException();
+
         static Sender<IOException> of(DatagramSocket socket) {
-            return new SenderImpl<>(socket, socket::send, socket::close);
+            return new SenderImpl<>(socket, socket::send, socket::close, SE);
         }
 
         static Sender<IOException> of(MulticastSocket socket, byte ttl) {
             SenderImpl.Send<IOException> send =
                     (pkt) -> socket.send(pkt, ttl);
-            return new SenderImpl<>(socket, send, socket::close);
+            return new SenderImpl<>(socket, send, socket::close, IOE);
         }
 
         static Sender<IOException> of(DatagramChannel socket) {
             SenderImpl.Send<IOException> send =
                     (pkt) -> socket.send(ByteBuffer.wrap(pkt.getData()),
                             pkt.getSocketAddress());
-            return new SenderImpl<>(socket, send, socket::close);
+            return new SenderImpl<>(socket, send, socket::close, SE);
         }
     }
 
@@ -220,11 +207,14 @@ public class SendCheck {
         private final Send<E> send;
         private final Closer<E> closer;
         private final Object socket;
+        private final Class<? extends E> expectedException;
 
-        public SenderImpl(Object socket, Send<E> send, Closer<E> closer) {
+        public SenderImpl(Object socket, Send<E> send, Closer<E> closer,
+                          Class<? extends E> expectedException) {
             this.socket = socket;
             this.send = send;
             this.closer = closer;
+            this.expectedException = expectedException;
         }
 
         @Override
@@ -237,6 +227,11 @@ public class SendCheck {
             closer.close();
         }
 
+        @Override
+        public Class<? extends E> expectedException() {
+            return expectedException;
+        }
+
         @Override
         public String toString() {
             return socket.getClass().getSimpleName();