Discoverer: Fix NPE when NetIf is not specified

[holgerfriedrich]

The following code should work - if I read the javadoc correctly:

Discoverer discovererUdp = new Discoverer(0, false);
discovererUdp.startSearch(null, 5, true);
List<Result<SearchResponse>> responses = discovererUdp.getSearchResponses();
logger.warn("discovery {}", responses.toString());

However, toString() fails with

java.lang.NullPointerException: Cannot invoke "java.net.NetworkInterface.getName()" because "this.ni" is null

and during detection I see

java.lang.NullPointerException: Cannot invoke "java.net.NetworkInterface.equals(Object)" because the return value of "tuwien.auto.calimero.knxnetip.Discoverer$Result.getNetworkInterface()" is null at tuwien.auto.calimero.knxnetip.Discoverer$Result.equals(Discoverer.java:225) ~[bundleFile:?] at java.util.ArrayList.indexOfRange(ArrayList.java:299) ~[?:?] at java.util.ArrayList.indexOf(ArrayList.java:286) ~[?:?] at java.util.ArrayList.contains(ArrayList.java:275) ~[?:?] at java.util.Collections$SynchronizedCollection.contains(Collections.java:2087) ~[?:?] at tuwien.auto.calimero.knxnetip.Discoverer$ReceiverLoop.onReceive(Discoverer.java:868) [bundleFile:?] at tuwien.auto.calimero.knxnetip.Discoverer$ReceiverLoop.receive(Discoverer.java:922) [bundleFile:?] at tuwien.auto.calimero.internal.UdpSocketLooper.loop(UdpSocketLooper.java:134) [bundleFile:?] at tuwien.auto.calimero.knxnetip.Discoverer$ReceiverLoop.run(Discoverer.java:840) [bundleFile:?]

Both is fixed by more restrictive null checks.

Or did I get the usage instructions wrong and a network interface should be supplied?

[bmalinowsky]

It's a regression, it was implemented with MulticastSocket which always returns an interface; now with using a datagram channel it doesn't if not specifically set.

One could also assign Net.defaultNetif in ReceiverLoop ctor if the netif is null. I am not sure how important the old behavior is; one advantage might be that a user doesn't have to null check netif in Result.

[bmalinowsky]

Looking at the code, I converted Result to a record and fixed this issue at the same time.

As a side note, using startSearch(netif, timeout, wait) with netif null is mainly either for a host with a (stable) single interface (i.e., the default netif and routing table is straight forward), or if you specifically rely on the routing table. In case of discovery with unicast responses, this also requires an appropriate config of the localhost setting. So I would in general use startSearch(timeout, wait) which checks all usable interfaces.

[holgerfriedrich]

@bmalinowsky thanks for the hint. I had implemented iterating over all network interfaces in the meantime to get it running with 2.5.1 release. Somehow I have overlooked the method startSearch with only 2 parameters. That's much cleaner now.

Regarding the PR - as you have fixed the root cause, this PR seems no longer necessary. Closing. And thanks for looking into it.