Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
Make failing to import netifaces not a load-time error
Browse files Browse the repository at this point in the history
Only raise an exception if we try to execute a validator that would
require `netifaces` (the default.)
  • Loading branch information
JordanMilne committed Aug 15, 2022
1 parent 9606288 commit 5aafc5f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
14 changes: 12 additions & 2 deletions advocate/addrvalidator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
import ipaddress
import re

import netifaces
try:
import netifaces
HAVE_NETIFACES = True
except ImportError:
netifaces = None
HAVE_NETIFACES = False

from .exceptions import NameserverException
from .exceptions import NameserverException, ConfigException


def canonicalize_hostname(hostname):
Expand All @@ -19,6 +24,9 @@ def canonicalize_hostname(hostname):

def determine_local_addresses():
"""Get all IPs that refer to this machine according to netifaces"""
if not HAVE_NETIFACES:
raise ConfigException("Tried to determine local addresses, "
"but netifaces module was not importable")
ips = []
for interface in netifaces.interfaces():
if_families = netifaces.ifaddresses(interface)
Expand Down Expand Up @@ -68,6 +76,8 @@ def __init__(
allow_teredo=False,
allow_6to4=False,
allow_dns64=False,
# Must be explicitly set to "False" if you don't want to try
# detecting local interface addresses with netifaces.
autodetect_local_addresses=True,
):
if not port_blacklist and not port_whitelist:
Expand Down
4 changes: 4 additions & 0 deletions advocate/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@ class MountDisabledException(AdvocateException):

class ProxyDisabledException(NotImplementedError, AdvocateException):
pass


class ConfigException(AdvocateException):
pass
22 changes: 21 additions & 1 deletion test/test_advocate.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from advocate.api import RequestsAPIWrapper
from advocate.connection import advocate_getaddrinfo
from advocate.exceptions import (
ConfigException,
MountDisabledException,
NameserverException,
UnacceptableAddressException,
Expand Down Expand Up @@ -311,6 +312,25 @@ def test_local_address_handling(self, mock_determine_local_addresses):
))
mock_determine_local_addresses.assert_not_called()

def test_netifaces_presence_optional(self):
# Advocate should still work without netifaces, but only if you've specifically
# said you don't care about checking against local interface addresses.
with patch("advocate.addrvalidator.HAVE_NETIFACES", False):
with self.assertRaises(ConfigException):
self._is_addrinfo_allowed("200.1.1.1", 80, autodetect_local_addresses=True)
with self.assertRaises(ConfigException):
advocate.addrvalidator.determine_local_addresses()
# Should be fine if you specifically asked to not look at the local addrs
self.assertTrue(self._is_addrinfo_allowed(
"200.1.1.1",
80,
autodetect_local_addresses=False,
))

# These shouldn't `raise`
self._is_addrinfo_allowed("200.1.1.1", 80, autodetect_local_addresses=True)
advocate.addrvalidator.determine_local_addresses()


@unittest.skipIf(
not canonname_supported(),
Expand Down Expand Up @@ -341,7 +361,7 @@ def test_idn(self):
fake_lookup=True,
hostname_blacklist={"*.org"}
))
# case insensitive, please
# case-insensitive, please
self.assertFalse(self._is_hostname_allowed(
u"中国.example.oRg",
fake_lookup=True,
Expand Down

0 comments on commit 5aafc5f

Please sign in to comment.