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 <[email protected]> on behalf of Patrick Concannon
<[email protected]>
Sent: Friday 10 April 2020 10:16
To: OpenJDK Network Dev list <[email protected]>
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
