JDK-8234582 : MulticastSocket getOption/setOption inverts the value of IP_MULTICAST_LOOP
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.net
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 14
  • Submitted: 2019-11-21
  • Updated: 2019-11-21
  • Resolved: 2019-11-21
Related Reports
CSR :  
JDK-8233296 - MulticastSocket getOption/setOption inverts the value of IP_MULTICAST_LOOP
Description
Summary
-------

`MulticastSocket::getOption(StandardSocketOption.IP_LOOPBACK_MODE)` and `MulticastSocket::setOption(StandardSocketOption.IP_LOOPBACK_MODE, Boolean value)`  invert the value of IP_MULTICAST_LOOP

Problem
-------

`MulticastSocket` delegates getting and setting of options to a wrapped `DatagramSocketImpl` implementing `SocketOptions`; This implementation is not directly accessible to the code calling of `MulticastSocket`.

Before JDK-8036979 was implemented, options could only be set by calling the legacy and ad-hoc option-specific getters and setters defined by `MulticastSocket`/`DatagramSocket`. For getting and setting the  `IP_MULTICAST_LOOP` option, `MulticastSocket` defined an ad-hoc `getLoopbackMode`/`setLoopbackMode` getter/setter pair.

However the parameter passed to `setLoopbackMode` is a boolean named `disable` which when `true` is specified to disable loopback mode. Similarly, `getLoopbackMode` is specified to return true when loopback mode is disabled. This was implemented by directly passing/returning that value to the legacy `DatagramSocketImpl::setOption`/`getOption` methods inherited from the `SocketOptions` interface. These legacy methods defined by the `SocketOptions` interface take an `int` constant to identify which option is get/set - (and not a SocketOption object).

When JDK-8036979 was implemented in JDK 9 to add public `getOption`/`setOption` methods to `DatagramSocket` (and by extension to `MulticastSocket`), a bridge was implemented in the internal AbstractPlainDatagramSocketImpl class to transform a call from the new `DatagramSocket/MulticastSocket` `getOption`/`setOption` into a call to `DatagramSocketImpl::getOption`/`setOption`. However - that fact that `true` had always been passed to disable loopback mode instead of enabling it as is defined by the newer `StandardSocketOptions.IP_MULTICAST_LOOP` `SocketOption` was overlooked.

In summary we ended up with:

    (1) MulticastSocket::setLoopbackMode(true)  => calls (2) with true => disable loopback
    (2) DatagramSocketImpl::setOption(SocketOptions.IP_MULTICAST_LOOP, true) => disable loopback
    (3) MulticastSocket::setOption(StandardSocketOptions.IP_MULTICAST_LOOP, true) => calls(2) with true  => disable loopback
    
This is this last behavior (3) which is problematic, as `StandardSocketOptions.IP_MULTICAST_LOOP` specifies that `true` means the option is enabled.

Solution
--------

The solution is to change the behavior of `MulticastSocket::setOption(StandardSocketOptions.IP_MULTICAST_LOOP, value)` and `MulticastSocket::getOption(StandardSocketOptions.IP_MULTICAST_LOOP)` to conform to the specification of  `StandardSocketOptions.IP_MULTICAST_LOOP`.
The behavior of the other methods (`MulticastSocket::set(get)LoopbackMode` and `DatagramSocketImpl` implementation of the legacy `SocketOptions` interface) are left unchanged.

In summary we want:

    (1) MulticastSocket::setLoopbackMode(true)  => calls (2) with true => disable loopback
    (2) DatagramSocketImpl::setOption(SocketOptions.IP_MULTICAST_LOOP, true) => disable loopback
    (3) MulticastSocket::setOption(StandardSocketOptions.IP_MULTICAST_LOOP, true) => calls(2) with false  => enable loopback

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

There is no specification change - only behavioral. 

For convenience a webrev can be seen there: http://cr.openjdk.java.net/~dfuchs/webrev_8233296/webrev.00/
Comments
I see a release note is already planned and I concur with Alan's recommendation with respect to 11 updates. Moving to Approved.
21-11-2019
MulticastSocket does not document that it supports reading or setting the IP_MULTICAST_LOOP socket option with this API so it is unlikely to be used. In addition, setting IP_MULTICAST_LOOP isn't effective everywhere when the socket is IPv6 and used to join IPv4 multicast groups. Finally, the API is new since Java SE 9 so unlikely to be widely used. So I think the compatibility risk is low. It might even be worthwhie to back-port this to a JDK 11 update to avoid incompatibles for code written in the future that starts to use this API to change the loopback mode.
21-11-2019