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

[mark sheppard]

Hi Patrick,
    if I understand the change correctly, you wish to eliminate the IllegalArgumentException and return an InetSocketAddress
based on the current set values for address and port. Because you have changed the default value of the port to 0, the getSocketAddress
will now return a SocketAddress with a wildcard address (assuming that address has not yet been set) and an ephemeral port, rather than the Exception caused by the port == -1.  Calling new InetSocketAddress(null, 0) will mean a wildcard address and an ephemeral port.
Port 0 is reserved for system selection of ephemeral port.  Thus, I think this change is now introducing some dubious semantics ?
Additionally if you were to call getSocketAddress multiple times you would get different non equal SocketAdress objects i.e. each one would have a different ephemeral port.

A possible approach is that an invocation of getSocketAddress should return null if the address or the port is not set, OR alternatively throw an IllegalStateException with either "port not set" or "address not set", as the DatagramPacket is in an unusable state for sending and the remote address is not set?

If we look at DatagramSocket::send() method , I suspect that there is another side effect to your change the check


                if (packetPort < 0 || packetPort > 0xFFFF)<https://hg.openjdk.java.net/jdk/jdk/rev/67e7f7e8284a#l1.22>
                    throw new IllegalArgumentException("port out of range:" + packetPort);

will allow a DatagramPacket with port == 0  to continue its porcessing. But sending to port 0 is not allowed afaik
so should this be:
      if (packetPort <= 0 || packetPort > 0xFFFF)
          throw new IllegalArgumentException("port out of range: " + packetPort);

regards
Mark


________________________________
From: net-dev <net-dev-bounces at openjdk.java.net> on behalf of Patrick Concannon <patrick.concannon at oracle.com>
Sent: Friday 10 April 2020 10:16
To: OpenJDK Network Dev list <net-dev at openjdk.java.net>
Subject: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'


Hi,

Could someone please review my webrev and CSR for JDK-8237890 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set' ?

DatagramPacket::getSocketAddress is misleading in that it can throw an IllegalArgumentException even though it doesn't take any arguments. This can occur if the port of a DatagramPacket is not set before a call to getSocketAddress is made.

In the recent fix JDK-8236940<https://bugs.openjdk.java.net/browse/JDK-8236940>, additional checks were added to ensure DatagramPacket cannot be sent to port 0. Following on from this update, the current fix changes the default port of a DatagramPacket from -1 to 0. An IllegalArgumentException will therefore no longer be thrown when getSocketAddress with no port set. Instead, an InetSocketAddress representing any local address with port 0 is returned.

bug: https://bugs.openjdk.java.net/browse/JDK-8237890
csr: https://bugs.openjdk.java.net/browse/JDK-8242483
webrev: http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.00/



Kind regards,

Patrick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200410/9d7d08c7/attachment.htm>