NetUtil: Netmask '32' is out of bounds #4205
Comments
|
@triller-telekom You implemented this in #3930 could you give me some background? |
|
According to https://askubuntu.com/a/709497, this is no valid netmask for a local subnet. What is your use case here? |
|
Anyhow, the NetworkConfigOptionProvider implementation has just been completely changed by #4189 - so I would suggest to test against the latest master. |
I did not realize any change of the method
The machine is part of a VPN network and that address has been assigned to the client by the VPN server. I didn't realize any other tool that complains about the network configuration. The communication between different VPN clients work as expected. All the time I open Configuration, Services in the Paper UI the log is filled with that stacktrace. |
|
Ah, I should have a deeper look at the stack trace... |
|
Hm, I finally read the JavaDoc for
We don't show the user the interface name though, only the IP address. A way to change this is to extend CidrAddress with an optional |
|
Hmm yes indeed we do not show the user the interface name. The fact that it is (still) in the javadoc is because of the discussions before I implemented that option. The original idea was to let the user select an interface name, but then he doesn't know which IP address this is and also the user does not care which name it is, other operating systems might even create very long names... In the end we decided to let the user select the network address in case its a dynamic one assigned through DHCP, so he does not need to change the configuration if the system gets a different IP assigned. That's why I am wondering why #4189 has changed this behavior and I should now select an IP address and not a network anymore. |
|
I forgot to add the call to the conversion method, because I didn't understand the purpose at first. Why do we only allow IPv4 here? And we should use apache/commons/net/util/SubnetUtils.java. As I understood Guava goes, but the apache library stays, so can and should be used here. |
Maybe I should have left a comment there or paid some attention on your PR, sorry for noticing this late. Will you change it in a follow-up PR please?
It's the first implementation since most people still have IPv4. But our intention was that IPv6 support should be added there too, at some point in time.
I wanted to use that method, but @kaikreuzer complained that we should not add such a dependency to our "core" bundle... :( |
|
We use apache felix :D A lot of Guava code can be replaced by Java8 and soon Java9, but the apache library is really helpful. IPv6 would work differently, because we need the zone index additionally (fe80::3%eth0). Which is the network interface name on linux/macOS and the network interface number on Windows. |
|
I would say, we show the user the CIDR notation (including ipv6 zone index) and store it as value as well. The networkAdressService would find the network interface for IPv4 like at the moment (computing the subnet mask etc) and for IPv6 the zone index is used. |
|
But you still want to have the IPv6 configuration as a separate field, right? So the user can select the subnet to be used for IPv4 and in addition can configure the zone for IPv6. If that is what you suggest, I am fine with that. |
|
Nope, no separate field. The user will be presented with all assigned IP addresses (IPv4+IPv6) in the list. This is because you are using IPv4 or IPv6 to bind your network services, not both. Of course the respective other protocol works as well on the bound network interface.
Hm, the user should not select a subnet. He selects an "IP/prefix%zone" which is unique and can be used to find the corresponding network interface and assigned IP again (even if DHCP/NDPv6 changed addresses in between). |
|
I will create a pr tomorrow |
I disagree! A user might want to use IPv6 on one interface and IPv4 on another, so this should be 2 fields. |
|
This would mean, that every service (all netty, jetty instances and any other service) would need to start two times and bind to two interface addresses if the user selects an IPv4 and IPv6 address? |
|
I have created a PR (#4218). Of course we need to discuss and agree first on what we mean and what use case each of us have in mind. |
|
Hm, seems I should have tested #4218 before merging. at openHAB startup (even if I start with Note that the dashboard calls the (now deprecated) @davidgraeff Could you please check this? |
|
Note that the logged warning is also a regression, this wasn't logged before. |
|
It was the originally implemented behaviour of the pr to first select an ipv4, but was changed in between to remove ipv4/ipv6 distinction. |
|
As this caused a regression, I have reverted the commit/PR for now.
I don't see any substantial discussion on the PR and I acknowledge that I failed to get involved in such - so my apologies for having merged it prematurely. I think that IPv4 should be the prominent setting in any case, because that is what people are using and what they are familiar with. Imho IPv6 should only be an advanced option, possibly only to be turned on, if desired, so that the standard UI options are sensible. |
Fixes eclipse-archived#4205 Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
* Accept 32bit netmask in config for point-to-point connections Fixes #4205 Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
|
I think IPv6 should work as good as IPv4 with ESH. IPv4 can be the preferred protocol, but that setting should be made in a very prominent way and not buried in the implementation (methods with the name ....IPv4only..., ...preferredIPv4... etc). Even in Germany we already have a cable company providing IPv6 addresses only, and IPv4 works via tunneling ("IPv4Lite"). In my opinion, it should be in the developer guidelines for ESH to write protocol agnostic IP utility methods only, if applicable. I think, with the references PR I proved that an implementation is possible and even simpler (fewer lines of code). I would have wished that others test the PR for their IPv4 network, which didn't happen before the merge. Missing features like considering the "java.net.preferIPv4Stack" flag or a GUI option for an easy choice would have been noticed earlier. |
Why is a netmask '32' out of bounds?
The text was updated successfully, but these errors were encountered: