Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'
On 29/04/2020 21:08, Patrick Concannon wrote: webrev: http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.01/ Thanks Patrick. LGTM. Pedantically, this test might temporarily leak instances of DatagramSocket when it detects a bug. That is, for instance - in the case where `new DatagramSocket(-1)` were buggy and would not throw. But as I'm confident that should never happen, and as modifying the test to cater for that would be quite ugly, let's leave it as is. best regards, -- daniel
Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'
Hi Daniel and Mark,
WRT what happens when `laddr` is null: it has been filed as a separate
issue, i.e. JDK-8243999
<https://bugs.openjdk.java.net/browse/JDK-8243999>, and has been
assigned to me.
Thanks for the wording suggestions, Mark. I will take a closer look once
this patch is out of the way.
Daniel:Thanks for your suggestions. Well spotted with regards to the
port. I've incorporated your comments into the patch, and you can find
them in the new webrev below.
webrev: http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.01/
Kind regards,
Patrick
On 29/04/2020 14:57, mark sheppard wrote:
Hi Daniel, Patrick,
I wonder is there an opportunity here to refine the constructor
descriptions a
little, also
The wording associated with wildcard addressing includes
* an IP address chosen by the kernel.
which is not actually correct, and maybe it should be omitted from
the various constructor descriptions.
there is explicit reference to IPv4 INADDR_ANY (0.0.0.0) which may warrant
a rewording to more general wildcard address {@link
InetAddress#isAnyLocalAddress}
I think the following wording was proposed offline
changing
* If the IP address is 0.0.0.0, the socket will be bound to the
* {@link InetAddress#isAnyLocalAddress wildcard} address,
* an IP address chosen by the kernel.
to
* If the IP address is a {@link InetAddress#isAnyLocalAddress
wildcard} address
* (or is {@code null}), the socket will be bound to the wildcard address.
also changing
The socket will be bound to the
{@link InetAddress#isAnyLocalAddress wildcard} address,
an IP address chosen by the kernel.
to
The socket will be bound to the
{@link InetAddress#isAnyLocalAddress wildcard} address.
best regards
Mark
*From:* net-dev on behalf of Daniel
Fuchs
*Sent:* Tuesday 28 April 2020 15:32
*To:* Patrick Concannon ; OpenJDK
Network Dev list
*Subject:* Re: RFR[8243507]: 'DatagramSocket constructors don’t always
specify what happens when passed invalid parameters'
Hi Patrick,
Looks good to me.
I realize we haven't specified what happens if the `laddr` is null.
Are you planning to fix that in a separate changeset or tag it to
this one?
Maybe we should unify the description of the `port` parameter in
the two constructors:
327 * @param port port to use.
357 * @param port local port to use
Maybe:
* @param port local port to use in the bind operation.
Nit: It's customary to break line before :
343 * address. The local port must be
between 0 and
suggest:
343 * address.
* The local port must be
best regards,
-- daniel
On 28/04/2020 10:33, Patrick Concannon wrote:
> Hi,
>
> Could someone please review my fix for JDK-8243507 'DatagramSocket
> constructors don’t always specify what happens when passed invalid
> parameters'?
>
> Currently, the DatagramSocket constructor `DatagramSocket(SocketAddress
> bindaddr)` doesn't specify what happens if passed a SocketAddress
> subclass not supported by this socket.
> Also, there are two DatagramSocket constructors that accept a port
> number, but neither constructor specifies what happens when passed port
> 0, or a port which falls outside of the valid range of port numbers i.e
> between 0 and 65535 inclusive.
>
> This fix updates the spec for each these constructors to inform the
user
> of what happens when passed an invalid argument. For the constructors
> that take a port, the spec will now specify that an
> IllegalArgumentException is thrown when passed a port outside of the
> acceptable range, or, if zero is passed, that the system will choose an
> appropriate port for them. For the constructor that takes a
> SocketAddress, an IllegalArgumentException will be thrown if an invalid
> SocketAddress subclass is passed as a parameter.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8243507
> csr: https://bugs.openjdk.java.net/browse/JDK-8243976
> webrev:
http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.00/
>
>
> Kind regards,
>
> Patrick
>
Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'
Hi Daniel, Patrick,
I wonder is there an opportunity here to refine the constructor
descriptions a
little, also
The wording associated with wildcard addressing includes
* an IP address chosen by the kernel.
which is not actually correct, and maybe it should be omitted from
the various constructor descriptions.
there is explicit reference to IPv4 INADDR_ANY (0.0.0.0) which may warrant
a rewording to more general wildcard address {@link
InetAddress#isAnyLocalAddress}
I think the following wording was proposed offline
changing
* If the IP address is 0.0.0.0, the socket will be bound to the
* {@link InetAddress#isAnyLocalAddress wildcard} address,
* an IP address chosen by the kernel.
to
* If the IP address is a {@link InetAddress#isAnyLocalAddress wildcard} address
* (or is {@code null}), the socket will be bound to the wildcard address.
also changing
The socket will be bound to the
{@link InetAddress#isAnyLocalAddress wildcard} address,
an IP address chosen by the kernel.
to
The socket will be bound to the
{@link InetAddress#isAnyLocalAddress wildcard} address.
best regards
Mark
From: net-dev on behalf of Daniel Fuchs
Sent: Tuesday 28 April 2020 15:32
To: Patrick Concannon ; OpenJDK Network Dev list
Subject: Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify
what happens when passed invalid parameters'
Hi Patrick,
Looks good to me.
I realize we haven't specified what happens if the `laddr` is null.
Are you planning to fix that in a separate changeset or tag it to
this one?
Maybe we should unify the description of the `port` parameter in
the two constructors:
327 * @param port port to use.
357 * @param port local port to use
Maybe:
* @param port local port to use in the bind operation.
Nit: It's customary to break line before :
343 * address. The local port must be
between 0 and
suggest:
343 * address.
* The local port must be
best regards,
-- daniel
On 28/04/2020 10:33, Patrick Concannon wrote:
> Hi,
>
> Could someone please review my fix for JDK-8243507 'DatagramSocket
> constructors don’t always specify what happens when passed invalid
> parameters'?
>
> Currently, the DatagramSocket constructor `DatagramSocket(SocketAddress
> bindaddr)` doesn't specify what happens if passed a SocketAddress
> subclass not supported by this socket.
> Also, there are two DatagramSocket constructors that accept a port
> number, but neither constructor specifies what happens when passed port
> 0, or a port which falls outside of the valid range of port numbers i.e
> between 0 and 65535 inclusive.
>
> This fix updates the spec for each these constructors to inform the user
> of what happens when passed an invalid argument. For the constructors
> that take a port, the spec will now specify that an
> IllegalArgumentException is thrown when passed a port outside of the
> acceptable range, or, if zero is passed, that the system will choose an
> appropriate port for them. For the constructor that takes a
> SocketAddress, an IllegalArgumentException will be thrown if an invalid
> SocketAddress subclass is passed as a parameter.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8243507
> csr: https://bugs.openjdk.java.net/browse/JDK-8243976
> webrev: http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.00/
>
>
> Kind regards,
>
> Patrick
>
Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'
Hi Patrick, Looks good to me. I realize we haven't specified what happens if the `laddr` is null. Are you planning to fix that in a separate changeset or tag it to this one? Maybe we should unify the description of the `port` parameter in the two constructors: 327 * @param port port to use. 357 * @param port local port to use Maybe: * @param port local port to use in the bind operation. Nit: It's customary to break line before : 343 * address. The local port must be between 0 and suggest: 343 * address. * The local port must be best regards, -- daniel On 28/04/2020 10:33, Patrick Concannon wrote: Hi, Could someone please review my fix for JDK-8243507 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'? Currently, the DatagramSocket constructor `DatagramSocket(SocketAddress bindaddr)` doesn't specify what happens if passed a SocketAddress subclass not supported by this socket. Also, there are two DatagramSocket constructors that accept a port number, but neither constructor specifies what happens when passed port 0, or a port which falls outside of the valid range of port numbers i.e between 0 and 65535 inclusive. This fix updates the spec for each these constructors to inform the user of what happens when passed an invalid argument. For the constructors that take a port, the spec will now specify that an IllegalArgumentException is thrown when passed a port outside of the acceptable range, or, if zero is passed, that the system will choose an appropriate port for them. For the constructor that takes a SocketAddress, an IllegalArgumentException will be thrown if an invalid SocketAddress subclass is passed as a parameter. bug: https://bugs.openjdk.java.net/browse/JDK-8243507 csr: https://bugs.openjdk.java.net/browse/JDK-8243976 webrev: http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.00/ Kind regards, Patrick
RFR[8243507]: 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'
Hi, Could someone please review my fix for JDK-8243507 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'? Currently, the DatagramSocket constructor `DatagramSocket(SocketAddress bindaddr)` doesn't specify what happens if passed a SocketAddress subclass not supported by this socket. Also, there are two DatagramSocket constructors that accept a port number, but neither constructor specifies what happens when passed port 0, or a port which falls outside of the valid range of port numbers i.e between 0 and 65535 inclusive. This fix updates the spec for each these constructors to inform the user of what happens when passed an invalid argument. For the constructors that take a port, the spec will now specify that an IllegalArgumentException is thrown when passed a port outside of the acceptable range, or, if zero is passed, that the system will choose an appropriate port for them. For the constructor that takes a SocketAddress, an IllegalArgumentException will be thrown if an invalid SocketAddress subclass is passed as a parameter. bug: https://bugs.openjdk.java.net/browse/JDK-8243507 csr: https://bugs.openjdk.java.net/browse/JDK-8243976 webrev: http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.00/ Kind regards, Patrick
