JDK-8224792 : java.net socket types new-style socket option methods - spec and impl mismatch
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.net
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 13
  • Submitted: 2019-05-25
  • Updated: 2019-05-29
  • Resolved: 2019-05-28
Related Reports
CSR :  
Description
Summary
-------

Update the default implementation of the new-style socket options
methods of `java.net.SocketImpl` and `java.net.DatagramSocketImpl`. As
well as the throwing specification and behaviour of the new-style
socket option methods of the four `java.net` socket types.

Problem
-------

The `java.net` TCP-based socket types, `ServerSocket` and `Socket`, have
their implementations backed by `java.net.SocketImpl`. The `java.net`
UDP-based socket types, `DatagramSocket` and `MulticastSocket`, have
their implementations backed by `java.net.DatagramSocketImpl`. The
backing socket _impl_ allows for different implementations to be plugged
into the socket APIs, which amounts to an SPI-like mechanism ( dating
back to the Java 1.0 era ).

When the three new-style methods for socket options, namely `getOption`,
`setOption`, and `supportedOptions`, were added in Java 9, to the four
`java.net` socket types, the supporting `SocketImpl` and
`DatagramSocketImpl` were also retrofitted with similar methods. Since
`SocketImpl` and `DatagramSocketImpl` are abstract classes, a default
implementation of these three new methods was added ( to allow backward
compatibility with pre-existing implementations ).

The default implementation of `SocketImpl::supportedOptions` returns a
set of socket options that represent that of what a typical
`ServerSocket` implementation may choose to support. The default
implementation of `SocketImpl::getOption` and `SocketImpl::setOption`
do some basic forwarding of the new-style options to the old-style
options, otherwise throw (the specified) `UnsupportedOperationException`, indicating that the `SocketImpl` does not support the option. Given
that the default `supportedOptions` support only the small set of
three server-like socket options ( `SO_RCVBUF`, `SO_REUSEADDR`, and
`IP_TOS`), the `getOption`/`setOption` methods only forward these three
socket options. This default implementation is neither specified nor
documented.

Similarly, the default implementation of
`DatagramSocketImpl::supportedOptions` returns a set of socket options
that represent that of what a typical `DatagramSocket` implementation
may choose to support. The default implementation of
`DatagramSocketImpl::getOption` and `DatagramSocketImpl::setOption` do
some basic forwarding of the new-style options to the old-style options,
otherwise throw (the specified) `UnsupportedOperationException` -
indicating that the `DatagramSocketImpl` does not support the option.
Given that the default `supportedOptions` support only the small set
of socket options ( `SO_SNDBUF`,  `SO_RCVBUF`, `SO_REUSEADDR`, and
`IP_TOS`), the `getOption`/`setOption` methods only forward these four
socket options. This default implementation is neither specified nor
documented.

Given the above limited default implementations, it is reasonable to
assume that any concrete implementation of `SocketImpl` or
`DatagramSocketImpl`, will have to override these three new-style socket
option methods, to provide their own implementation tailored to the
specific socket options that they support. With that in mind, it would
be simpler and more straight forward to reduce the default
implementation and add a note suggesting that subclasses provide their
own implementation. Additionally, in some cases an effective non-empty
( no-op ) specification-compliant default implementation is not
practicable, e.g. it is not possible to for `getOption` to throw an
`IOExcepton` after the socket impl is closed, since the socket impl has
no specified state. 

---

The second part of this CSR is concerted with the behavioural ( and
minor specification ) aspects of the exceptions thrown when "bad" values
are given to the following methods:

    Socket::setOption(SocketOption name, T value)
    Socket::getOption(SocketOption name)
    ServerSocket::setOption(SocketOption name, T value)
    ServerSocket::getOption(SocketOption name)
    DatagramSocket::setOption(SocketOption name, T value)
    DatagramSocket::getOption(SocketOption name)
    MulticastSocket::setOption(SocketOption name, T value)
    MulticastSocket::getOption(SocketOption name)
    SocketImpl::setOption(SocketOption name, T value)
    SocketImpl::getOption(SocketOption name)
    DatagramSocketImpl::setOption(SocketOption name, T value)
    DatagramSocketImpl::getOption(SocketOption name)

The aforementioned methods are specified to throw:

  1) `NullPointerException` if given a `null` socket option `name`
   ( they currently may throw `UnsupportedOperationException` )

  2) `IllegalArgumentException` if given a "bad" socket option `value`
   ( they currently may throw `SocketException` )

  3) `IOException` if invoked after the socket has been closed
   ( they currently continue to operate, or may throw `SocketException`)

Additionally, there are some inconsistencies in their respective
specifications across socket and impl types, when it comes to the
documentation of particular exceptions, e.g. `SocketImpl::setValue` is
missing an `@throws IllegalArgumentException`, while `Socket::setValue`
documents it ( `Socket::setValue` cannot actually validate the option
value, validation is performed by the impl ).
`DatagramSocketImpl::setValue` has a similar issue. Also, there are some
`@throws NullPointerException` missing.


Solution
--------

  1) Update ( simplify ) and specify the default implementation of the
     three new-style socket option methods of `SocketImpl` and
     `DatagramSocketImpl`. More specifically, change the default
     implementation of `supportedOptions` to return an empty set -
     indicating that no socket options are supported. The default
     implementation of `getOption`/`setOption` can then be updated to
     always throw `UnsupportedOperationException`, as logically no
     socket options are supported.

  2) Update the implementation of the four `java.net` socket types, and
     the two impl types, to match their specification. That is, to
     throw the specified exception when "bad" input is given.

  3) Make the specification of all three new-style socket option
     methods consistent, with respect to their specified exceptions,
     across all four socket types and two socket impls.

Specification
-------------

`java.net.SocketImpl`

       /**
        * Returns a set of SocketOptions supported by this impl
        * and by this impl's socket (Socket or ServerSocket)
        *
    +   * @implSpec
    +   * The default implementation of this method returns an empty set.
    +   * Subclasses should override this method with an appropriate implementation.
    +   *
        * @return a Set of SocketOptions
        *
        * @since 9
        */
       protected Set<SocketOption<?>> supportedOptions()

       /**
        * Called to set a socket option.
        *
    +   * @implSpec
    +   * The default implementation of this method first checks that the given
    +   * socket option {code name} is not null, then throws {@code
    +   * UnsupportedOperationException}. Subclasses should override this method
    +   * with an appropriate implementation.
    +   *
        * @param <T> The type of the socket option value
        * @param name The socket option
        * @param value The value of the socket option. A value of {@code null}
        *              may be valid for some options.
        *
        * @throws UnsupportedOperationException if the SocketImpl does not
        *         support the option
    +   * @throws IllegalArgumentException if the value is not valid for
    +   *         the option
        * @throws IOException if an I/O error occurs, or if the socket is closed
    +   * @throws NullPointerException if name is {@code null}
        *
        * @since 9
        */
       protected <T> void setOption(SocketOption<T> name, T value) throws IOException

       /**
        * Called to get a socket option.
        *
    +   * @implSpec
    +   * The default implementation of this method first checks that the given
    +   * socket option {code name} is not null, then throws {@code
    +   * UnsupportedOperationException}. Subclasses should override this method
    +   * with an appropriate implementation.
    +   *
        * @param <T> The type of the socket option value
        * @param name The socket option
        * @return the value of the named option
        *
        * @throws UnsupportedOperationException if the SocketImpl does not
        *         support the option
        * @throws IOException if an I/O error occurs, or if the socket is closed
    +   * @throws NullPointerException if name is {@code null}
        *
        * @since 9
        */
        protected <T> T getOption(SocketOption<T> name) throws IOException


`java.net.DatagramSocketImpl`

       /**
        * Returns a set of SocketOptions supported by this impl
        * and by this impl's socket (DatagramSocket or MulticastSocket)
        *
    +   * @implSpec
    +   * The default implementation of this method returns an empty set.
    +   * Subclasses should override this method with an appropriate implementation.
    +   *
        * @return a Set of SocketOptions
        *
        * @since 9
        */
       protected Set<SocketOption<?>> supportedOptions()

       /**
        * Called to set a socket option.
        *
    +   * @implSpec
    +   * The default implementation of this method first checks that the given
    +   * socket option {code name} is not null, then throws {@code
    +   * UnsupportedOperationException}. Subclasses should override this method
    +   * with an appropriate implementation.
    +   *
        * @param <T> The type of the socket option value
        * @param name The socket option
        * @param value The value of the socket option. A value of {@code null}
        *              may be valid for some options.
        *
        * @throws UnsupportedOperationException if the DatagramSocketImpl does not
        *         support the option
    +   * @throws IllegalArgumentException if the value is not valid for
    +   *         the option
    -   * @throws IOException if an I/O problem occurs while attempting to set the option
    +   * @throws IOException if an I/O error occurs, or if the socket is closed
        * @throws NullPointerException if name is {@code null}
        *
        * @since 9
        */
       protected <T> void setOption(SocketOption<T> name, T value) throws IOException

       /**
        * Called to get a socket option.
        *
    +   * @implSpec
    +   * The default implementation of this method first checks that the given
    +   * socket option {code name} is not null, then throws {@code
    +   * UnsupportedOperationException}. Subclasses should override this method
    +   * with an appropriate implementation.
    +   *
        * @param <T> The type of the socket option value
        * @param name The socket option
        * @return the socket option
        *
        * @throws UnsupportedOperationException if the DatagramSocketImpl does not
        *         support the option
    -   * @throws IOException if an I/O problem occurs while attempting to set the option
    +   * @throws IOException if an I/O error occurs, or if the socket is closed
        * @throws NullPointerException if name is {@code null}
        *
        * @since 9
        */
       protected <T> T getOption(SocketOption<T> name) throws IOException

Comments
The compatibility impact of the changes proposed seem minor. The set/getOption methods were added in Java SE 9 and unlikely there is a lot of code in the wild invoking these with bad values and relying on an unspecified exception. There is little evidence that the SocketImpl SPI mechanism is widely used. Many of the motivations for using this mechanism went away in JDK 1.4. If they exist then it would be prudent for the maintainers to override supportedOptions so that users of the socket API have an accurate picture of the socket options that are supported. The changes are release note worthy.
29-05-2019

I agree with Alan that a release note is warranted and I've added the label to the parent issue. Moving to Approved.
28-05-2019

This CSR is mainly motivated by the creation of new JCK tests in this area.
27-05-2019