Explorar el Código

Resolver-related code cleanup

This commit introduces a new exit code (4) for when no system resolver can be found, instead of just crashing.

It also introduces ResolverType, which is similar to code that used to be present. While the previous code passed all its unit tests, there was system-level code (obtaining the resolvers) being called from those unit tests, which isn't something unit tests are meant to be doing — there's no guarantee that they'll continue to work on systems with different network conifgurations. Because ResolverType is just a type, it can be used in tests. (Resolver has had its PartialEq impl stripped, and there's now a warning, to stop this from happening again.)

The refactor lets us remove some Clippy allowances, which is always nice.
Benjamin Sago hace 3 años
padre
commit
cf051a9f6f
Se han modificado 5 ficheros con 179 adiciones y 127 borrados
  1. 3 0
      man/dog.1.md
  2. 13 4
      src/main.rs
  3. 93 89
      src/options.rs
  4. 14 7
      src/requests.rs
  5. 56 27
      src/resolve.rs

+ 3 - 0
man/dog.1.md

@@ -232,6 +232,9 @@ EXIT STATUSES
 3
 : If there was a problem with the command-line arguments.
 
+4
+: If there was a problem obtaining the system nameserver information.
+
 
 AUTHOR
 ======

+ 13 - 4
src/main.rs

@@ -16,10 +16,7 @@
 #![allow(clippy::module_name_repetitions)]
 #![allow(clippy::option_if_let_else)]
 #![allow(clippy::too_many_lines)]
-#![allow(clippy::unit_arg)]
-#![allow(clippy::unused_self)]
 #![allow(clippy::upper_case_acronyms)]
-#![allow(clippy::useless_let_if_seq)]
 #![allow(clippy::wildcard_imports)]
 
 #![deny(unsafe_code)]
@@ -109,7 +106,16 @@ fn run(Options { requests, format, measure_time }: Options) -> i32 {
     let timer = if measure_time { Some(Instant::now()) } else { None };
 
     let mut errored = false;
-    for (request_list, transport) in requests.generate() {
+
+    let request_tuples = match requests.generate() {
+        Ok(rt) => rt,
+        Err(e) => {
+            eprintln!("Unable to obtain resolver: {}", e);
+            return exits::SYSTEM_ERROR;
+        }
+    };
+
+    for (request_list, transport) in request_tuples {
         let request_list_len = request_list.len();
         for (i, request) in request_list.into_iter().enumerate() {
             let result = transport.send(&request);
@@ -189,4 +195,7 @@ mod exits {
 
     /// Exit code for when the command-line options are invalid.
     pub const OPTIONS_ERROR: i32 = 3;
+
+    /// Exit code for when the system network configuration could not be determined.
+    pub const SYSTEM_ERROR: i32 = 4;
 }

+ 93 - 89
src/options.rs

@@ -11,7 +11,7 @@ use dns::record::RecordType;
 use crate::connect::TransportType;
 use crate::output::{OutputFormat, UseColours, TextFormat};
 use crate::requests::{RequestGenerator, Inputs, ProtocolTweaks, UseEDNS};
-use crate::resolve::Resolver;
+use crate::resolve::ResolverType;
 use crate::txid::TxidGenerator;
 
 
@@ -168,7 +168,7 @@ impl Inputs {
         }
 
         for ns in matches.opt_strs("nameserver") {
-            self.add_nameserver(&ns)?;
+            self.add_nameserver(&ns);
         }
 
         for qclass in matches.opt_strs("class") {
@@ -185,11 +185,11 @@ impl Inputs {
 
         let input_uppercase = input.to_ascii_uppercase();
         if let Some(rt) = RecordType::from_type_name(&input_uppercase) {
-            self.types.push(rt);
+            self.record_types.push(rt);
             Ok(())
         }
         else if let Ok(type_number) = input.parse::<u16>() {
-            self.types.push(RecordType::from(type_number));
+            self.record_types.push(RecordType::from(type_number));
             Ok(())
         }
         else {
@@ -197,33 +197,22 @@ impl Inputs {
         }
     }
 
-    fn add_nameserver(&mut self, input: &str) -> Result<(), OptionsError> {
-        self.resolvers.push(Resolver::specified(input.into()));
-        Ok(())
-    }
-
-    fn parse_class_name(&self, input: &str) -> Option<QClass> {
-        if input.eq_ignore_ascii_case("IN") {
-            Some(QClass::IN)
-        }
-        else if input.eq_ignore_ascii_case("CH") {
-            Some(QClass::CH)
-        }
-        else if input.eq_ignore_ascii_case("HS") {
-            Some(QClass::HS)
-        }
-        else {
-            None
-        }
+    fn add_nameserver(&mut self, input: &str) {
+        self.resolver_types.push(ResolverType::Specific(input.into()));
     }
 
     fn add_class(&mut self, input: &str) -> Result<(), OptionsError> {
-        let qclass = self.parse_class_name(input)
+        let qclass = parse_class_name(input)
             .or_else(|| input.parse().ok().map(QClass::Other));
 
         match qclass {
-            Some(c)  => Ok(self.classes.push(c)),
-            None     => Err(OptionsError::InvalidQueryClass(input.into())),
+            Some(c) => {
+                self.classes.push(c);
+                Ok(())
+            }
+            None => {
+                Err(OptionsError::InvalidQueryClass(input.into()))
+            }
         }
     }
 
@@ -231,10 +220,10 @@ impl Inputs {
         for argument in matches.free {
             if let Some(nameserver) = argument.strip_prefix('@') {
                 trace!("Got nameserver -> {:?}", nameserver);
-                self.add_nameserver(nameserver)?;
+                self.add_nameserver(nameserver);
             }
-            else if Self::is_constant_name(&argument) {
-                if let Some(class) = self.parse_class_name(&argument) {
+            else if is_constant_name(&argument) {
+                if let Some(class) = parse_class_name(&argument) {
                     trace!("Got qclass -> {:?}", &argument);
                     self.classes.push(class);
                 }
@@ -255,30 +244,17 @@ impl Inputs {
         Ok(())
     }
 
-    fn is_constant_name(argument: &str) -> bool {
-        let first_char = match argument.chars().next() {
-            Some(c)  => c,
-            None     => return false,
-        };
-
-        if ! first_char.is_ascii_alphabetic() {
-            return false;
-        }
-
-        argument.chars().all(|c| c.is_ascii_alphanumeric())
-    }
-
     fn load_fallbacks(&mut self) {
-        if self.types.is_empty() {
-            self.types.push(RecordType::A);
+        if self.record_types.is_empty() {
+            self.record_types.push(RecordType::A);
         }
 
         if self.classes.is_empty() {
             self.classes.push(QClass::IN);
         }
 
-        if self.resolvers.is_empty() {
-            self.resolvers.push(Resolver::system_default());
+        if self.resolver_types.is_empty() {
+            self.resolver_types.push(ResolverType::SystemDefault);
         }
 
         if self.transport_types.is_empty() {
@@ -287,7 +263,7 @@ impl Inputs {
     }
 
     fn check_for_missing_nameserver(&self) -> Result<(), OptionsError> {
-        if self.resolvers.is_empty() && self.transport_types == [TransportType::HTTPS] {
+        if self.resolver_types.is_empty() && self.transport_types == [TransportType::HTTPS] {
             Err(OptionsError::MissingHttpsUrl)
         }
         else {
@@ -296,6 +272,34 @@ impl Inputs {
     }
 }
 
+fn is_constant_name(argument: &str) -> bool {
+    let first_char = match argument.chars().next() {
+        Some(c)  => c,
+        None     => return false,
+    };
+
+    if ! first_char.is_ascii_alphabetic() {
+        return false;
+    }
+
+    argument.chars().all(|c| c.is_ascii_alphanumeric())
+}
+
+fn parse_class_name(input: &str) -> Option<QClass> {
+    if input.eq_ignore_ascii_case("IN") {
+        Some(QClass::IN)
+    }
+    else if input.eq_ignore_ascii_case("CH") {
+        Some(QClass::CH)
+    }
+    else if input.eq_ignore_ascii_case("HS") {
+        Some(QClass::HS)
+    }
+    else {
+        None
+    }
+}
+
 
 impl TxidGenerator {
     fn deduce(matches: &getopts::Matches) -> Result<Self, OptionsError> {
@@ -506,9 +510,9 @@ mod test {
         fn fallbacks() -> Self {
             Inputs {
                 domains:         vec![ /* No domains by default */ ],
-                types:           vec![ RecordType::A ],
+                record_types:    vec![ RecordType::A ],
                 classes:         vec![ QClass::IN ],
-                resolvers:       vec![ Resolver::system_default() ],
+                resolver_types:  vec![ ResolverType::SystemDefault ],
                 transport_types: vec![ TransportType::Automatic ],
             }
         }
@@ -574,7 +578,7 @@ mod test {
     fn just_domain() {
         let options = Options::getopts(&[ "lookup.dog" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
+            domains: vec![ Labels::encode("lookup.dog").unwrap() ],
             .. Inputs::fallbacks()
         });
     }
@@ -583,7 +587,7 @@ mod test {
     fn just_named_domain() {
         let options = Options::getopts(&[ "-q", "lookup.dog" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
+            domains: vec![ Labels::encode("lookup.dog").unwrap() ],
             .. Inputs::fallbacks()
         });
     }
@@ -592,8 +596,8 @@ mod test {
     fn domain_and_type() {
         let options = Options::getopts(&[ "lookup.dog", "SOA" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            types:      vec![ RecordType::SOA ],
+            domains:      vec![ Labels::encode("lookup.dog").unwrap() ],
+            record_types: vec![ RecordType::SOA ],
             .. Inputs::fallbacks()
         });
     }
@@ -602,8 +606,8 @@ mod test {
     fn domain_and_type_lowercase() {
         let options = Options::getopts(&[ "lookup.dog", "soa" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            types:      vec![ RecordType::SOA ],
+            domains:      vec![ Labels::encode("lookup.dog").unwrap() ],
+            record_types: vec![ RecordType::SOA ],
             .. Inputs::fallbacks()
         });
     }
@@ -612,8 +616,8 @@ mod test {
     fn domain_and_nameserver() {
         let options = Options::getopts(&[ "lookup.dog", "@1.1.1.1" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            resolvers:  vec![ Resolver::specified("1.1.1.1".into()) ],
+            domains:        vec![ Labels::encode("lookup.dog").unwrap() ],
+            resolver_types: vec![ ResolverType::Specific("1.1.1.1".into()) ],
             .. Inputs::fallbacks()
         });
     }
@@ -622,8 +626,8 @@ mod test {
     fn domain_and_class() {
         let options = Options::getopts(&[ "lookup.dog", "CH" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::CH ],
+            domains: vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes: vec![ QClass::CH ],
             .. Inputs::fallbacks()
         });
     }
@@ -632,8 +636,8 @@ mod test {
     fn domain_and_class_lowercase() {
         let options = Options::getopts(&[ "lookup.dog", "ch" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::CH ],
+            domains: vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes: vec![ QClass::CH ],
             .. Inputs::fallbacks()
         });
     }
@@ -642,10 +646,10 @@ mod test {
     fn all_free() {
         let options = Options::getopts(&[ "lookup.dog", "CH", "NS", "@1.1.1.1" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::CH ],
-            types:      vec![ RecordType::NS ],
-            resolvers:  vec![ Resolver::specified("1.1.1.1".into()) ],
+            domains:        vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes:        vec![ QClass::CH ],
+            record_types:   vec![ RecordType::NS ],
+            resolver_types: vec![ ResolverType::Specific("1.1.1.1".into()) ],
             .. Inputs::fallbacks()
         });
     }
@@ -654,10 +658,10 @@ mod test {
     fn all_parameters() {
         let options = Options::getopts(&[ "-q", "lookup.dog", "--class", "CH", "--type", "SOA", "--nameserver", "1.1.1.1" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::CH ],
-            types:      vec![ RecordType::SOA ],
-            resolvers:  vec![ Resolver::specified("1.1.1.1".into()) ],
+            domains:        vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes:        vec![ QClass::CH ],
+            record_types:   vec![ RecordType::SOA ],
+            resolver_types: vec![ ResolverType::Specific("1.1.1.1".into()) ],
             .. Inputs::fallbacks()
         });
     }
@@ -666,10 +670,10 @@ mod test {
     fn all_parameters_lowercase() {
         let options = Options::getopts(&[ "-q", "lookup.dog", "--class", "ch", "--type", "soa", "--nameserver", "1.1.1.1" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::CH ],
-            types:      vec![ RecordType::SOA ],
-            resolvers:  vec![ Resolver::specified("1.1.1.1".into()) ],
+            domains:        vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes:        vec![ QClass::CH ],
+            record_types:   vec![ RecordType::SOA ],
+            resolver_types: vec![ ResolverType::Specific("1.1.1.1".into()) ],
             .. Inputs::fallbacks()
         });
     }
@@ -678,8 +682,8 @@ mod test {
     fn two_types() {
         let options = Options::getopts(&[ "-q", "lookup.dog", "--type", "SRV", "--type", "AAAA" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            types:      vec![ RecordType::SRV, RecordType::AAAA ],
+            domains:      vec![ Labels::encode("lookup.dog").unwrap() ],
+            record_types: vec![ RecordType::SRV, RecordType::AAAA ],
             .. Inputs::fallbacks()
         });
     }
@@ -688,8 +692,8 @@ mod test {
     fn two_classes() {
         let options = Options::getopts(&[ "-q", "lookup.dog", "--class", "IN", "--class", "CH" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::IN, QClass::CH ],
+            domains: vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes: vec![ QClass::IN, QClass::CH ],
             .. Inputs::fallbacks()
         });
     }
@@ -698,10 +702,10 @@ mod test {
     fn all_mixed_1() {
         let options = Options::getopts(&[ "lookup.dog", "--class", "CH", "SOA", "--nameserver", "1.1.1.1" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::CH ],
-            types:      vec![ RecordType::SOA ],
-            resolvers:  vec![ Resolver::specified("1.1.1.1".into()) ],
+            domains:        vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes:        vec![ QClass::CH ],
+            record_types:   vec![ RecordType::SOA ],
+            resolver_types: vec![ ResolverType::Specific("1.1.1.1".into()) ],
             .. Inputs::fallbacks()
         });
     }
@@ -710,9 +714,9 @@ mod test {
     fn all_mixed_2() {
         let options = Options::getopts(&[ "CH", "SOA", "MX", "IN", "-q", "lookup.dog", "--class", "HS" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            classes:    vec![ QClass::HS, QClass::CH, QClass::IN ],
-            types:      vec![ RecordType::SOA, RecordType::MX ],
+            domains:      vec![ Labels::encode("lookup.dog").unwrap() ],
+            classes:      vec![ QClass::HS, QClass::CH, QClass::IN ],
+            record_types: vec![ RecordType::SOA, RecordType::MX ],
             .. Inputs::fallbacks()
         });
     }
@@ -721,9 +725,9 @@ mod test {
     fn all_mixed_3() {
         let options = Options::getopts(&[ "lookup.dog", "--nameserver", "1.1.1.1", "--nameserver", "1.0.0.1" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("lookup.dog").unwrap() ],
-            resolvers:  vec![ Resolver::specified("1.1.1.1".into()),
-                              Resolver::specified("1.0.0.1".into()), ],
+            domains:        vec![ Labels::encode("lookup.dog").unwrap() ],
+            resolver_types: vec![ ResolverType::Specific("1.1.1.1".into()),
+                                  ResolverType::Specific("1.0.0.1".into()), ],
             .. Inputs::fallbacks()
         });
     }
@@ -732,9 +736,9 @@ mod test {
     fn explicit_numerics() {
         let options = Options::getopts(&[ "11", "--class", "22", "--type", "33" ]).unwrap();
         assert_eq!(options.requests.inputs, Inputs {
-            domains:    vec![ Labels::encode("11").unwrap() ],
-            classes:    vec![ QClass::Other(22) ],
-            types:      vec![ RecordType::from(33) ],
+            domains:      vec![ Labels::encode("11").unwrap() ],
+            classes:      vec![ QClass::Other(22) ],
+            record_types: vec![ RecordType::from(33) ],
             .. Inputs::fallbacks()
         });
     }

+ 14 - 7
src/requests.rs

@@ -1,7 +1,9 @@
 //! Request generation based on the user’s input arguments.
 
+use std::io;
+
 use crate::connect::TransportType;
-use crate::resolve::Resolver;
+use crate::resolve::ResolverType;
 use crate::txid::TxidGenerator;
 
 
@@ -31,13 +33,13 @@ pub struct Inputs {
     pub domains: Vec<dns::Labels>,
 
     /// The list of DNS record types to query for.
-    pub types: Vec<dns::record::RecordType>,
+    pub record_types: Vec<dns::record::RecordType>,
 
     /// The list of DNS classes to query for.
     pub classes: Vec<dns::QClass>,
 
     /// The list of resolvers to send queries to.
-    pub resolvers: Vec<Resolver>,
+    pub resolver_types: Vec<ResolverType>,
 
     /// The list of transport types to send queries over.
     pub transport_types: Vec<TransportType>,
@@ -81,12 +83,17 @@ impl RequestGenerator {
 
     /// Iterate through the inputs matrix, returning pairs of DNS request list
     /// and the details of the transport to send them down.
-    pub fn generate(self) -> Vec<(Vec<dns::Request>, Box<dyn dns_transport::Transport>)> {
+    pub fn generate(self) -> io::Result<Vec<(Vec<dns::Request>, Box<dyn dns_transport::Transport>)>> {
         let mut requests = Vec::new();
+
+        let resolvers = self.inputs.resolver_types.into_iter()
+            .map(ResolverType::obtain)
+            .collect::<Result<Vec<_>, _>>()?;
+
         for domain in &self.inputs.domains {
-            for qtype in self.inputs.types.iter().copied() {
+            for qtype in self.inputs.record_types.iter().copied() {
                 for qclass in self.inputs.classes.iter().copied() {
-                    for resolver in &self.inputs.resolvers {
+                    for resolver in &resolvers {
                         for transport_type in &self.inputs.transport_types {
 
                             let mut flags = dns::Flags::query();
@@ -116,7 +123,7 @@ impl RequestGenerator {
             }
         }
 
-        requests
+        Ok(requests)
     }
 }
 

+ 56 - 27
src/resolve.rs

@@ -7,12 +7,42 @@ use log::*;
 use dns::Labels;
 
 
+/// A **resolver type** is the source of a `Resolver`.
+#[derive(PartialEq, Debug)]
+pub enum ResolverType {
+
+    /// Obtain a resolver by consulting the system in order to find a
+    /// nameserver and a search list.
+    SystemDefault,
+
+    /// Obtain a resolver by using the given user-submitted string.
+    Specific(String),
+}
+
+impl ResolverType {
+
+    /// Obtains a resolver by the means specified in this type. Returns an
+    /// error if there was a problem looking up system information.
+    pub fn obtain(self) -> io::Result<Resolver> {
+        match self {
+            Self::SystemDefault => {
+                system_nameservers()
+            }
+            Self::Specific(nameserver) => {
+                let search_list = Vec::new();
+                Ok(Resolver { nameserver, search_list })
+            }
+        }
+    }
+}
+
+
 /// A **resolver** knows the address of the server we should
 /// send DNS requests to, and the search list for name lookup.
-#[derive(PartialEq, Debug)]
+#[derive(Debug)]
 pub struct Resolver {
 
-    /// The address of the name server.
+    /// The address of the nameserver.
     pub nameserver: String,
 
     /// The search list for name lookup.
@@ -21,27 +51,13 @@ pub struct Resolver {
 
 impl Resolver {
 
-    /// Returns a resolver with the specified nameserver and an empty
-    /// search list.
-    pub fn specified(nameserver: String) -> Self {
-        let search_list = Vec::new();
-        Self { nameserver, search_list }
-    }
-
-    /// Returns a resolver that is default for the system.
-    pub fn system_default() -> Self {
-        let (nameserver_opt, search_list) = system_nameservers().expect("Failed to get nameserver");
-        let nameserver = nameserver_opt.expect("No nameserver found");
-        Self { nameserver, search_list }
-    }
-
     /// Returns a nameserver that queries should be sent to.
     pub fn nameserver(&self) -> String {
         self.nameserver.clone()
     }
 
     /// Returns a sequence of names to be queried, taking into account
-    /// of the search list.
+    /// the search list.
     pub fn name_list(&self, name: &Labels) -> Vec<Labels> {
         let mut list = Vec::new();
 
@@ -64,14 +80,18 @@ impl Resolver {
 
 
 /// Looks up the system default nameserver on Unix, by querying
-/// `/etc/resolv.conf` and returning the first line that specifies one.
+/// `/etc/resolv.conf` and using the first line that specifies one.
 /// Returns an error if there’s a problem reading the file, or `None` if no
 /// nameserver is specified in the file.
 #[cfg(unix)]
-fn system_nameservers() -> io::Result<(Option<String>, Vec<String>)> {
+fn system_nameservers() -> io::Result<Resolver> {
     use std::fs::File;
     use std::io::{BufRead, BufReader};
 
+    if cfg!(test) {
+        panic!("system_nameservers() called from test code");
+    }
+
     let f = File::open("/etc/resolv.conf")?;
     let reader = BufReader::new(f);
 
@@ -96,7 +116,8 @@ fn system_nameservers() -> io::Result<(Option<String>, Vec<String>)> {
         }
     }
 
-    Ok((nameservers.first().cloned(), search_list))
+    let nameserver = nameservers.into_iter().next().unwrap();
+    Ok(Resolver { nameserver, search_list })
 }
 
 
@@ -104,9 +125,13 @@ fn system_nameservers() -> io::Result<(Option<String>, Vec<String>)> {
 /// the list of network adapters and returning the first nameserver it finds.
 #[cfg(windows)]
 #[allow(unused)]  // todo: Remove this when the time is right
-fn system_nameservers() -> io::Result<(Option<String>, Vec<String>)> {
+fn system_nameservers() -> io::Result<Resolver> {
     use std::net::{IpAddr, UdpSocket};
 
+    if cfg!(test) {
+        panic!("system_nameservers() called from test code");
+    }
+
     let adapters = match ipconfig::get_adapters() {
         Ok(a) => a,
         Err(e) => {
@@ -147,6 +172,8 @@ fn system_nameservers() -> io::Result<(Option<String>, Vec<String>)> {
         ForceIPFamily::None => get_ipv6().or(get_ipv4()).ok(),
     };
 
+    let search_path = Vec::new();  // todo: implement this
+
     let active_adapters = adapters.iter().filter(|a| {
         a.oper_status() == ipconfig::OperStatus::IfOperStatusUp && !a.gateways().is_empty()
     });
@@ -158,21 +185,23 @@ fn system_nameservers() -> io::Result<(Option<String>, Vec<String>)> {
         .flatten()
     {
         debug!("Found first nameserver {:?}", dns_server);
-        // TODO: Implement dns suffix search list on Windows
-        return Ok((Some(dns_server.to_string()), Vec::new()));
+        let nameserver = dns_server.to_string();
+        Ok(Resolver { nameserver, search_path })
     }
 
     // Fallback
-    if let Some(dns_server) = active_adapters
+    else if let Some(dns_server) = active_adapters
         .flat_map(|a| a.dns_servers())
         .find(|d| (d.is_ipv4() && force_ip_family != ForceIPFamily::V6) || d.is_ipv6())
     {
         debug!("Found first fallback nameserver {:?}", dns_server);
-        return Ok((Some(dns_server.to_string()), Vec::new()));
+        let nameserver = dns_server.to_string();
+        Ok(Resolver { nameserver, search_path })
     }
 
-    warn!("No nameservers available");
-    return Ok((None, Vec::new()));
+    else {
+        panic!("No nameservers available");
+    }
 }