Skip to content

Commit

Permalink
馃悰 Fix netmask & external address detection
Browse files Browse the repository at this point in the history
Closes #5005

To be resilient against netifaces vs netifaces2 lib fights or future
fixes on netifaces2, we are going with an either/or approach with
regards to mask & netmask. The unit tests have been adjusted
accordingly as well.
  • Loading branch information
foosel committed May 6, 2024
1 parent 32eca5b commit 7d80170
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 11 deletions.
16 changes: 15 additions & 1 deletion src/octoprint/util/net.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,28 @@ def f():
HAS_V6 = False


def get_netmask(address):
# netifaces2 - see #5005
netmask = address.get("mask")
if netmask:
return netmask

# netifaces
netmask = address.get("netmask")
if netmask:
return netmask

raise ValueError(f"No netmask found in address: {address!r}")


def get_lan_ranges(additional_private=None):
logger = logging.getLogger(__name__)

if additional_private is None or not isinstance(additional_private, (list, tuple)):
additional_private = []

def to_ipnetwork(address):
prefix = address["netmask"]
prefix = get_netmask(address)
if "/" in prefix:
# v6 notation in netifaces output, e.g. "ffff:ffff:ffff:ffff::/64"
_, prefix = prefix.split("/")
Expand Down
45 changes: 35 additions & 10 deletions tests/util/test_net.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,29 @@ def patched_interfaces():
return ["eth0"]


def patched_ifaddresses(addr):
def patched_ifaddresses(addr, netmask):
if addr == "eth0":
return {
socket.AF_INET: [
{"addr": "192.168.123.10", "netmask": "255.255.255.0"},
{"addr": "12.1.1.10", "netmask": "255.0.0.0"},
{"addr": "192.168.123.10", netmask: "255.255.255.0"},
{"addr": "12.1.1.10", netmask: "255.0.0.0"},
],
socket.AF_INET6: [
{"addr": "2a01:4f8:1c0c:6958::1", "netmask": "ffff:ffff:ffff:ffff::/64"}
{"addr": "2a01:4f8:1c0c:6958::1", netmask: "ffff:ffff:ffff:ffff::/64"}
],
}

return {}


def patched_ifaddresses_mask(addr):
return patched_ifaddresses(addr, "mask")


def patched_ifaddresses_netmask(addr):
return patched_ifaddresses(addr, "netmask")


@ddt.ddt
class UtilNetTest(unittest.TestCase):
@ddt.data(
Expand All @@ -49,13 +57,14 @@ class UtilNetTest(unittest.TestCase):
)
@ddt.unpack
@mock.patch("netifaces.interfaces", side_effect=patched_interfaces)
@mock.patch("netifaces.ifaddresses", side_effect=patched_ifaddresses)
@mock.patch.object(octoprint.util.net, "HAS_V6", True)
def test_is_lan_address(self, input_address, input_additional, expected, nifa, nifs):
actual = octoprint.util.net.is_lan_address(
input_address, additional_private=input_additional
)
self.assertEqual(expected, actual)
def test_is_lan_address(self, input_address, input_additional, expected, nifs):
for side_effect in (patched_ifaddresses_mask, patched_ifaddresses_netmask):
with mock.patch("netifaces.ifaddresses", side_effect=side_effect):
actual = octoprint.util.net.is_lan_address(
input_address, additional_private=input_additional
)
self.assertEqual(expected, actual)

@ddt.data(
("fe80::89f3:31bb:ced0:2093%wlan0", "fe80::89f3:31bb:ced0:2093"),
Expand All @@ -77,3 +86,19 @@ def test_strip_interface_tag(self, address, expected):
def test_unmap_v4_in_v6(self, address, expected):
actual = octoprint.util.net.unmap_v4_as_v6(address)
self.assertEqual(expected, actual)

@ddt.data(
({"mask": "192.168.0.0/24"}, "192.168.0.0/24"),
({"netmask": "192.168.0.0/24"}, "192.168.0.0/24"),
)
@ddt.unpack
def test_get_netmask(self, address, expected):
actual = octoprint.util.net.get_netmask(address)
self.assertEqual(expected, actual)

def test_get_netmask_broken_address(self):
try:
octoprint.util.net.get_netmask({"nm": "192.168.0.0/24"})
self.fail("Expected ValueError")
except Exception as e:
self.assertIsInstance(e, ValueError)

0 comments on commit 7d80170

Please sign in to comment.