-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Description
MulticastOption's constructor doesn't allow the IPAddress group parameter to be null.
runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MulticastOption.cs
Lines 14 to 59 in 5ee7f88
| // Creates a new instance of the MulticastOption class with the specified IP address | |
| // group and local address. | |
| public MulticastOption(IPAddress group, IPAddress mcint) | |
| { | |
| if (group == null) | |
| { | |
| throw new ArgumentNullException(nameof(group)); | |
| } | |
| if (mcint == null) | |
| { | |
| throw new ArgumentNullException(nameof(mcint)); | |
| } | |
| Group = group; | |
| LocalAddress = mcint; | |
| } | |
| public MulticastOption(IPAddress group, int interfaceIndex) | |
| { | |
| if (group == null) | |
| { | |
| throw new ArgumentNullException(nameof(group)); | |
| } | |
| if (interfaceIndex < 0 || interfaceIndex > 0x00FFFFFF) | |
| { | |
| throw new ArgumentOutOfRangeException(nameof(interfaceIndex)); | |
| } | |
| Group = group; | |
| _ifIndex = interfaceIndex; | |
| } | |
| // Creates a new version of the MulticastOption class for the specified group. | |
| public MulticastOption(IPAddress group) | |
| { | |
| if (group == null) | |
| { | |
| throw new ArgumentNullException(nameof(group)); | |
| } | |
| Group = group; | |
| LocalAddress = IPAddress.Any; | |
| } |
However, the public property for Group allows it to be set to null:
runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MulticastOption.cs
Lines 61 to 72 in 5ee7f88
| // Sets the IP address of a multicast group. | |
| public IPAddress Group | |
| { | |
| get | |
| { | |
| return _group; | |
| } | |
| set | |
| { | |
| _group = value; | |
| } | |
| } |
This is inconsistent. And when trying to add nullable annotations to this class, this setter forces the getter to allow returning null. Which means all consuming code will need to check for null, when it really isn't an acceptable state for this class to be in.
The IPv6 version of this class fixes this issue by throwing an ArgumentNullException in the setter:
runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MulticastOption.cs
Lines 144 to 160 in 5ee7f88
| // Sets the IP address of a multicast group. | |
| public IPAddress Group | |
| { | |
| get | |
| { | |
| return _group; | |
| } | |
| set | |
| { | |
| if (value == null) | |
| { | |
| throw new ArgumentNullException(nameof(value)); | |
| } | |
| _group = value; | |
| } | |
| } |
We should consider a small breaking change here and throw an ArgumentNullException from MulticastOption.Group's setter.
/cc @stephentoub @davidsh @dotnet/ncl
NOTE: If we don't change the setter to reject null, we will need to decide what to do when it is null. Currently, we NRE:
