7021373: DatagramPacket exception conditions are not clear

Specification is clarified by adding or clarifying @throws clauses where required

Reviewed-by: alanb, chegar, darcy, dfuchs

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 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
@@ -35,6 +35,16 @@ package java.net;
  * differently, and might arrive in any order. Packet delivery is
  * not guaranteed.
  *
+ * <p>
+ * Unless otherwise specified, passing a {@code null} argument causes
+ * a {@link NullPointerException NullPointerException} to be thrown.
+ *
+ * <p>
+ * Methods and constructors of {@code DatagramPacket} accept parameters
+ * of type {@link SocketAddress}. {@code DatagramPacket} supports
+ * {@link InetSocketAddress}, and may support additional {@code SocketAddress}
+ * sub-types.
+ *
  * @author  Pavani Diwanji
  * @author  Benjamin Renaud
  * @since   1.0
@@ -72,6 +82,10 @@ class DatagramPacket {
      * @param   offset  the offset for the buffer
      * @param   length  the number of bytes to read.
      *
+     * @throws  IllegalArgumentException    if the length or offset
+     *          is negative, or if the length plus the offset is
+     *          greater than the length of the packet's given buffer.
+     *
      * @since   1.2
      */
     public DatagramPacket(byte buf[], int offset, int length) {
@@ -89,6 +103,10 @@ class DatagramPacket {
      *
      * @param   buf     buffer for holding the incoming datagram.
      * @param   length  the number of bytes to read.
+     *
+     * @throws  IllegalArgumentException    if the length is negative
+     *          or if the length is greater than the length of the
+     *          packet's given buffer.
      */
     public DatagramPacket(byte buf[], int length) {
         this (buf, 0, length);
@@ -104,8 +122,14 @@ class DatagramPacket {
      * @param   buf     the packet data.
      * @param   offset  the packet data offset.
      * @param   length  the packet data length.
-     * @param   address  the destination address.
+     * @param   address the destination address, or {@code null}.
      * @param   port    the destination port number.
+     *
+     * @throws  IllegalArgumentException    if the length or offset
+     *          is negative, or if the length plus the offset is
+     *          greater than the length of the packet's given buffer,
+     *          or if the port is out of range.
+     *
      * @see     java.net.InetAddress
      *
      * @since   1.2
@@ -128,7 +152,12 @@ class DatagramPacket {
      * @param   offset  the packet data offset.
      * @param   length  the packet data length.
      * @param   address the destination socket address.
-     * @throws  IllegalArgumentException if address type is not supported
+     *
+     * @throws  IllegalArgumentException    if address is null or its
+     *          type is not supported, or if the length or offset is
+     *          negative, or if the length plus the offset is greater
+     *          than the length of the packet's given buffer.
+     *
      * @see     java.net.InetAddress
      *
      * @since   1.4
@@ -146,8 +175,13 @@ class DatagramPacket {
      *
      * @param   buf     the packet data.
      * @param   length  the packet length.
-     * @param   address  the destination address.
+     * @param   address the destination address, or {@code null}.
      * @param   port    the destination port number.
+     *
+     * @throws  IllegalArgumentException    if the length is negative,
+     *          or if the length is greater than the length of the
+     *          packet's given buffer, or if the port is out of range.
+     *
      * @see     java.net.InetAddress
      */
     public DatagramPacket(byte buf[], int length,
@@ -164,9 +198,15 @@ class DatagramPacket {
      * @param   buf     the packet data.
      * @param   length  the packet length.
      * @param   address the destination address.
-     * @throws  IllegalArgumentException if address type is not supported
+     *
-     * @since 1.4
+     * @throws  IllegalArgumentException    if address is null or its type is
+     *          not supported, or if the length is negative, or if the length
+     *          is greater than the length of the packet's given buffer, or
+     *          if the port is out of range.
+     *
      * @see     java.net.InetAddress
+     *
+     * @since   1.4
      */
     public DatagramPacket(byte buf[], int length, SocketAddress address) {
         this(buf, 0, length, address);
@@ -239,13 +279,13 @@ class DatagramPacket {
      * data, length and offset of the packet.
      *
      * @param   buf     the buffer to set for this packet
-     *
      * @param   offset  the offset into the data
-     *
      * @param   length  the length of the data
      *          and/or the length of the buffer used to receive data
      *
-     * @throws    NullPointerException if the argument is null
+     * @throws  IllegalArgumentException    if the length or offset
+     *          is negative, or if the length plus the offset is
+     *          greater than the length of the packet's given buffer.
      *
      * @see     #getData
      * @see     #getOffset
@@ -269,9 +309,12 @@ class DatagramPacket {
     /**
      * Sets the IP address of the machine to which this datagram
      * is being sent.
-     * @param iaddr the {@code InetAddress}
+     *
-     * @since   1.1
+     * @param   iaddr   the {@code InetAddress}, or {@code null}.
+     *
      * @see     #getAddress()
+     *
+     * @since   1.1
      */
     public synchronized void setAddress(InetAddress iaddr) {
         address = iaddr;
@@ -280,9 +323,14 @@ class DatagramPacket {
     /**
      * Sets the port number on the remote host to which this datagram
      * is being sent.
+     *
      * @param   iport   the port number
-     * @since   1.1
+     *
+     * @throws  IllegalArgumentException    if the port is out of range
+     *
      * @see     #getPort()
+     *
+     * @since   1.1
      */
     public synchronized void setPort(int iport) {
         if (iport < 0 || iport > 0xFFFF) {
@@ -296,11 +344,13 @@ class DatagramPacket {
      * host to which this datagram is being sent.
      *
      * @param   address the {@code SocketAddress}
+     *
      * @throws  IllegalArgumentException    if address is null or is a
-     *          SocketAddress subclass not supported by this socket
+     *          SocketAddress subclass not supported.
+     *
+     * @see     #getSocketAddress
      *
      * @since   1.4
-     * @see #getSocketAddress
      */
     public synchronized void setSocketAddress(SocketAddress address) {
         if (address == null || !(address instanceof InetSocketAddress))
@@ -317,8 +367,10 @@ class DatagramPacket {
      * host that this packet is being sent to or is coming from.
      *
      * @return  the {@code SocketAddress}
-     * @since 1.4
+     *
      * @see     #setSocketAddress
+     *
+     * @since   1.4
      */
     public synchronized SocketAddress getSocketAddress() {
         return new InetSocketAddress(getAddress(), getPort());
@@ -331,8 +383,6 @@ class DatagramPacket {
      *
      * @param   buf     the buffer to set for this packet.
      *
-     * @throws    NullPointerException if the argument is null.
-     *
      * @see     #getLength
      * @see     #getData
      *
@@ -357,9 +407,9 @@ class DatagramPacket {
      *
      * @param   length      the length to set for this packet.
      *
-     * @throws    IllegalArgumentException if the length is negative
+     * @throws  IllegalArgumentException    if the length is negative,
-     * of if the length is greater than the packet's data buffer
+     *          or if the length plus the offset is greater than the
-     * length.
+     *          length of the packet's data buffer.
      *
      * @see     #getLength
      * @see     #setData

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 1998, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -22,130 +22,98 @@
  */
 
 /* @test
- *
+ * @bug 4091803 7021373
- * @bug 4091803
- *
  * @summary this tests that the constructor of DatagramPacket rejects
  *          bogus arguments properly.
- *
+ * @run testng Constructor
- * @author Benjamin Renaud
  */
-import java.io.*;
+
-import java.net.*;
+import java.net.DatagramPacket;
-import java.util.*;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.expectThrows;
 
 public class Constructor {
 
-    public static void main(String[] args) throws Exception {
+    private static final byte[] buf = new byte[128];
-        testNullPacket();
+
-        testNegativeBufferLength();
+    private static final InetAddress LOOPBACK = InetAddress.getLoopbackAddress();
-        testPacketLengthTooLarge();
+    private static final Class<NullPointerException> NPE = NullPointerException.class;
-        testNegativePortValue();
+    private static final Class<IllegalArgumentException> IAE = IllegalArgumentException.class;
-        testPortValueTooLarge();
+
-        testSimpleConstructor();
+    @Test
-        testFullConstructor();
+    public void testNullPacket() {
-        System.err.println("all passed!");
+        expectThrows(NPE,
+                () -> new DatagramPacket(null, 100));
+    }
+    @Test
+    public void testNull() throws Exception {
+        expectThrows(NPE, () -> new DatagramPacket(null, 100));
+        expectThrows(NPE, () -> new DatagramPacket(null, 0, 10));
+        expectThrows(NPE, () -> new DatagramPacket(null, 0, 10, LOOPBACK, 80));
+        expectThrows(NPE, () -> new DatagramPacket(null, 10, LOOPBACK, 80));
+        expectThrows(NPE, () -> new DatagramPacket(null, 0, 10, new InetSocketAddress(80)));
+        expectThrows(NPE, () -> new DatagramPacket(null, 10, new InetSocketAddress(80)));
+
+        // no Exception expected for null addresses
+        new DatagramPacket(buf, 10, null, 0);
+        new DatagramPacket(buf, 10, 10, null, 0);
     }
 
-    static void testNullPacket() throws Exception {
+    @Test
-        boolean error = true;
+    public void testNegativeBufferLength() {
-        try {
-            new DatagramPacket(null, 100);
-        } catch (NullPointerException e) {
-            /* correct exception */
-            error = false;
-        }
-        if (error) {
-            throw new RuntimeException("test 1 failed.");
-        }
-    }
-
-    static void testNegativeBufferLength() throws Exception {
-        boolean error = true;
-        byte[] buf = new byte[128];
-        try {
         /* length lesser than buffer length */
-            new DatagramPacket(buf, -128);
+        expectThrows(IAE,
-        } catch (IllegalArgumentException e) {
+                () -> new DatagramPacket(buf, -128));
-            /* correct exception */
-            error = false;
-        }
-        if (error) {
-            throw new RuntimeException("test 2 failed.");
-        }
     }
 
-    static void testPacketLengthTooLarge() throws Exception {
+    @Test
-        boolean error = true;
+    public void testPacketLengthTooLarge() {
-        byte[] buf = new byte[128];
-        try {
         /* length greater than buffer length */
-            new DatagramPacket(buf, 256);
+        expectThrows(IAE,
-        } catch (IllegalArgumentException e) {
+                () -> new DatagramPacket(buf, 256));
-            /* correct exception */
-            error = false;
-        }
-        if (error) {
-            throw new RuntimeException("test 3 failed.");
-        }
     }
 
-    static void testNegativePortValue() throws Exception {
+    @Test
-        boolean error = true;
+    public void testNegativePortValue() throws Exception {
-        byte[] buf = new byte[128];
-        InetAddress host = InetAddress.getLocalHost();
-        try {
         /* negative port */
-            new DatagramPacket(buf, 100, host, -1);
+        InetAddress addr = InetAddress.getLocalHost();
-        } catch (IllegalArgumentException e) {
+
-            /* correct exception */
+        expectThrows(IAE,
-            error = false;
+                () -> new DatagramPacket(buf, 100, addr, -1));
-        }
-        if (error) {
-            throw new RuntimeException("test 5 failed.");
-        }
     }
 
-    static void testPortValueTooLarge() throws Exception {
+    @Test
-        boolean error = true;
+    public void testPortValueTooLarge() {
-        byte[] buf = new byte[256];
-        InetAddress address = InetAddress.getLocalHost();
-        try {
         /* invalid port value */
-            new DatagramPacket(buf, 256, address, Integer.MAX_VALUE);
+        expectThrows(IAE,
-        } catch (IllegalArgumentException e) {
+                () -> new DatagramPacket(buf, 128, LOOPBACK, Integer.MAX_VALUE));
-            /* correct exception */
-            error = false;
-        }
-        if (error) {
-            throw new RuntimeException("test 6 failed.");
-        }
     }
 
-    static void testSimpleConstructor() {
+    @Test
-        byte[] buf = new byte[128];
+    public void testSimpleConstructor() {
         int offset = 10;
         int length = 50;
-        DatagramPacket packet = new DatagramPacket(buf, offset, length);
+        DatagramPacket pkt = new DatagramPacket(buf, offset, length);
-        if (packet.getData() != buf || packet.getOffset() != offset ||
+
-               packet.getLength() != length) {
+        assertFalse((pkt.getData() != buf || pkt.getOffset() != offset ||
-            throw new RuntimeException("simple constructor failed");
+                pkt.getLength() != length), "simple constructor failed");
-        }
     }
 
-    static void testFullConstructor() throws Exception {
+    @Test
-        byte[] buf = new byte[128];
+    public void testFullConstructor() {
         int offset = 10;
         int length = 50;
-        InetAddress address = InetAddress.getLocalHost();
         int port = 8080;
-        DatagramPacket packet = new DatagramPacket(buf, offset, length,
+        DatagramPacket packet = new DatagramPacket(buf, offset, length, LOOPBACK, port);
-                                                   address, port);
+
-        if (packet.getData() != buf || packet.getOffset() != offset ||
+        assertFalse((packet.getData() != buf || packet.getOffset() != offset ||
                 packet.getLength() != length ||
-            packet.getAddress() != address ||
+                packet.getAddress() != LOOPBACK ||
-            packet.getPort() != port) {
+                packet.getPort() != port), "full constructor failed");
-            throw new RuntimeException("full constructor failed");
-        }
     }
 }

test/jdk/java/net/DatagramPacket/Setters.java

@@ -0,0 +1,135 @@
+/*
+ * 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 7021373
+ * @summary check that the DatagramPacket setter methods
+ *          throw the correct exceptions
+ * @run testng Setters
+ */
+
+
+import java.net.DatagramPacket;
+
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+
+public class Setters {
+
+    private static final byte[] buf = new byte[128];
+
+    private static final Class<IllegalArgumentException> IAE = IllegalArgumentException.class;
+    private static final Class<NullPointerException> NPE = NullPointerException.class;
+
+    @Test
+    public void testSetAddress() throws Exception {
+        DatagramPacket pkt = new DatagramPacket(buf, 8);
+
+        // No Exception expected for null addresses
+        pkt.setAddress(null);
+        assertTrue(pkt.getAddress() == null);
+    }
+
+    @DataProvider
+    Object[][] data() { // add checks for setAddress with null - add getAddress to verify
+        return new Object[][]{
+                { buf,          0,      -1,     IAE  },
+                { buf,          -1,     1,      IAE  },
+                { buf,          128,    128,    IAE  },
+                { null,         0,      0,      NPE  },
+                { new byte[8],  2,      2,      null },
+                { buf,          2,      2,      null },
+                { buf,          20,     20,     null },
+        };
+    }
+
+    @Test(dataProvider = "data")
+    public void testSetData(byte[] buf,
+                            int offset,
+                            int length,
+                            Class<? extends Exception> exception) {
+        DatagramPacket pkt = new DatagramPacket(new byte[8], 8);
+
+        if (exception != null) {
+            expectThrows(exception, () -> pkt.setData(buf, offset, length));
+        } else if (buf == null) {
+            expectThrows(exception, () -> pkt.setData(buf));
+        } else {
+            pkt.setData(buf, offset, length);
+        }
+    }
+
+    @DataProvider
+    Object[][] lengths() {
+        return new Object[][]{
+                { 0,     -1,     IAE  },
+                { 8,     1,      IAE  },
+                { 4,     -2,     IAE  },
+                { 0,     9,      IAE  },
+                { 2,     2,      null },
+                { 4,     4,      null }
+        };
+    }
+
+    @Test(dataProvider = "lengths")
+    public void testSetLength(int offset, int length,
+                              Class<? extends Exception> exception) {
+        DatagramPacket pkt = new DatagramPacket(new byte[8], offset, 0);
+
+        if (exception != null) {
+            expectThrows(exception, () -> pkt.setLength(length));
+        } else {
+            pkt.setLength(length);
+        }
+    }
+
+    @DataProvider
+    Object[][] ports() {
+        return new Object[][]{
+                { -1,                IAE  },
+                { -666,              IAE  },
+                { Integer.MAX_VALUE, IAE  },
+                { 0xFFFF + 1,        IAE  },
+                { 0xFFFFFF,          IAE  },
+                { 0,                 null },
+                { 1,                 null },
+                { 666,               null },
+                { 0xFFFE,            null },
+                { 0xFFFF,            null },
+        };
+    }
+
+    @Test(dataProvider = "ports")
+    public void testSetPort(int port, Class<? extends Exception> exception) {
+        DatagramPacket pkt = new DatagramPacket(new byte[8], 0);
+
+        if (exception != null) {
+            expectThrows(exception, () -> pkt.setPort(port));
+        } else {
+            pkt.setPort(port);
+        }
+    }
+}