Handle single-word domain free arguments

This commit fixes a regression caused by the fix for GH-14, which made _any_ single-word argument be treated as though it should be a DNS class or type name. So `dog aoeu` would attempt to parse `aoeu` as a type, fail, and display an error. Now, it correctly does a domain lookup for `aoeu`.

The root cause was me trying to use the same functions for both parsing free arguments and named arguments, when they do two different things.

dns/src/record/mod.rs

@@ -172,11 +172,12 @@ impl From<u16> for RecordType {
 
 impl RecordType {
 
-    /// Determines the record type with a given name, or `None` if none is known.
+    /// Determines the record type with a given name, or `None` if none is
+    /// known. Matches names case-insensitively.
     pub fn from_type_name(type_name: &str) -> Option<Self> {
         macro_rules! try_record {
             ($record:tt) => {
-                if $record::NAME == type_name {
+                if $record::NAME.eq_ignore_ascii_case(type_name) {
                     return Some(Self::$record);
                 }
             }

dns/src/record/others.rs

@@ -17,7 +17,7 @@ impl UnknownQtype {
     /// Searches the list for an unknown type with the given name, returning a
     /// `HeardOf` variant if one is found, and `None` otherwise.
     pub fn from_type_name(type_name: &str) -> Option<Self> {
-        let (name, num) = TYPES.iter().find(|t| t.0 == type_name)?;
+        let (name, num) = TYPES.iter().find(|t| t.0.eq_ignore_ascii_case(type_name))?;
         Some(Self::HeardOf(name, *num))
     }
 

src/options.rs

@@ -157,63 +157,41 @@ impl Inputs {
 
     fn load_named_args(&mut self, matches: &getopts::Matches) -> Result<(), OptionsError> {
         for domain in matches.opt_strs("query") {
-            match Labels::encode(&domain) {
-                Ok(d)   => self.domains.push(d),
-                Err(e)  => return Err(OptionsError::InvalidDomain(e.into())),
-            }
+            self.add_domain(&domain)?;
         }
 
-        for qtype in matches.opt_strs("type") {
-            self.add_type(&qtype)?;
+        for record_name in matches.opt_strs("type") {
+            if record_name.eq_ignore_ascii_case("OPT") {
+                return Err(OptionsError::QueryTypeOPT);
+            }
+            else if let Some(record_type) = RecordType::from_type_name(&record_name) {
+                self.add_type(record_type);
+            }
+            else if let Ok(type_number) = record_name.parse::<u16>() {
+                self.record_types.push(RecordType::from(type_number));
+            }
+            else {
+                return Err(OptionsError::InvalidQueryType(record_name));
+            }
         }
 
         for ns in matches.opt_strs("nameserver") {
             self.add_nameserver(&ns);
         }
 
-        for qclass in matches.opt_strs("class") {
-            self.add_class(&qclass)?;
-        }
-
-        Ok(())
-    }
-
-    fn add_type(&mut self, input: &str) -> Result<(), OptionsError> {
-        if input.eq_ignore_ascii_case("OPT") {
-            return Err(OptionsError::QueryTypeOPT);
-        }
-
-        let input_uppercase = input.to_ascii_uppercase();
-        if let Some(rt) = RecordType::from_type_name(&input_uppercase) {
-            self.record_types.push(rt);
-            Ok(())
-        }
-        else if let Ok(type_number) = input.parse::<u16>() {
-            self.record_types.push(RecordType::from(type_number));
-            Ok(())
-        }
-        else {
-            Err(OptionsError::InvalidQueryType(input.into()))
-        }
-    }
-
-    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 = parse_class_name(input)
-            .or_else(|| input.parse().ok().map(QClass::Other));
-
-        match qclass {
-            Some(c) => {
-                self.classes.push(c);
-                Ok(())
+        for class_name in matches.opt_strs("class") {
+            if let Some(class) = parse_class_name(&class_name) {
+                self.add_class(class);
             }
-            None => {
-                Err(OptionsError::InvalidQueryClass(input.into()))
+            else if let Ok(class_number) = class_name.parse() {
+                self.add_class(QClass::Other(class_number));
+            }
+            else {
+                return Err(OptionsError::InvalidQueryClass(class_name));
             }
         }
+
+        Ok(())
     }
 
     fn load_free_args(&mut self, matches: getopts::Matches) -> Result<(), OptionsError> {
@@ -223,27 +201,40 @@ impl Inputs {
                 self.add_nameserver(nameserver);
             }
             else if is_constant_name(&argument) {
-                if let Some(class) = parse_class_name(&argument) {
+                if argument.eq_ignore_ascii_case("OPT") {
+                    return Err(OptionsError::QueryTypeOPT);
+                }
+                else if let Some(class) = parse_class_name(&argument) {
                     trace!("Got qclass -> {:?}", &argument);
-                    self.classes.push(class);
+                    self.add_class(class);
                 }
-                else {
+                else if let Some(record_type) = RecordType::from_type_name(&argument) {
                     trace!("Got qtype -> {:?}", &argument);
-                    self.add_type(&argument)?;
+                    self.add_type(record_type);
+                }
+                else {
+                    trace!("Got single-word domain -> {:?}", &argument);
+                    self.add_domain(&argument)?;
                 }
             }
             else {
                 trace!("Got domain -> {:?}", &argument);
-                match Labels::encode(&argument) {
-                    Ok(d)   => self.domains.push(d),
-                    Err(e)  => return Err(OptionsError::InvalidDomain(e.into())),
-                }
+                self.add_domain(&argument)?;
             }
         }
 
         Ok(())
     }
 
+    fn check_for_missing_nameserver(&self) -> Result<(), OptionsError> {
+        if self.resolver_types.is_empty() && self.transport_types == [TransportType::HTTPS] {
+            Err(OptionsError::MissingHttpsUrl)
+        }
+        else {
+            Ok(())
+        }
+    }
+
     fn load_fallbacks(&mut self) {
         if self.record_types.is_empty() {
             self.record_types.push(RecordType::A);
@@ -262,14 +253,27 @@ impl Inputs {
         }
     }
 
-    fn check_for_missing_nameserver(&self) -> Result<(), OptionsError> {
-        if self.resolver_types.is_empty() && self.transport_types == [TransportType::HTTPS] {
-            Err(OptionsError::MissingHttpsUrl)
+    fn add_domain(&mut self, input: &str) -> Result<(), OptionsError> {
+        if let Ok(domain) = Labels::encode(input) {
+            self.domains.push(domain);
+            Ok(())
         }
         else {
-            Ok(())
+            Err(OptionsError::InvalidDomain(input.into()))
         }
     }
+
+    fn add_type(&mut self, rt: RecordType) {
+        self.record_types.push(rt);
+    }
+
+    fn add_nameserver(&mut self, input: &str) {
+        self.resolver_types.push(ResolverType::Specific(input.into()));
+    }
+
+    fn add_class(&mut self, class: QClass) {
+        self.classes.push(class);
+    }
 }
 
 fn is_constant_name(argument: &str) -> bool {
@@ -505,6 +509,7 @@ impl fmt::Display for OptionsError {
 mod test {
     use super::*;
     use pretty_assertions::assert_eq;
+    use dns::record::UnknownQtype;
 
     impl Inputs {
         fn fallbacks() -> Self {
@@ -612,6 +617,26 @@ mod test {
         });
     }
 
+    #[test]
+    fn domain_and_other_type() {
+        let options = Options::getopts(&[ "lookup.dog", "any" ]).unwrap();
+        assert_eq!(options.requests.inputs, Inputs {
+            domains:      vec![ Labels::encode("lookup.dog").unwrap() ],
+            record_types: vec![ RecordType::Other(UnknownQtype::from_type_name("ANY").unwrap()) ],
+            .. Inputs::fallbacks()
+        });
+    }
+
+    #[test]
+    fn domain_and_single_domain() {
+        let options = Options::getopts(&[ "lookup.dog", "mixes" ]).unwrap();
+        assert_eq!(options.requests.inputs, Inputs {
+            domains:      vec![ Labels::encode("lookup.dog").unwrap(),
+                                Labels::encode("mixes").unwrap() ],
+            .. Inputs::fallbacks()
+        });
+    }
+
     #[test]
     fn domain_and_nameserver() {
         let options = Options::getopts(&[ "lookup.dog", "@1.1.1.1" ]).unwrap();
@@ -763,24 +788,15 @@ mod test {
         assert_eq!(options.requests.protocol_tweaks.udp_payload_size, Some(4096));
     }
 
-    #[test]
-    fn udp_size_invalid() {
-        assert_eq!(Options::getopts(&[ "-Z", "bufsize=null" ]),
-                   OptionsResult::InvalidOptions(OptionsError::InvalidTweak("bufsize=null".into())));
-    }
-
-    #[test]
-    fn udp_size_too_big() {
-        assert_eq!(Options::getopts(&[ "-Z", "bufsize=999999999" ]),
-                   OptionsResult::InvalidOptions(OptionsError::InvalidTweak("bufsize=999999999".into())));
-    }
-
     #[test]
     fn short_mode() {
         let tf = TextFormat { format_durations: true };
         let options = Options::getopts(&[ "dom.ain", "--short" ]).unwrap();
         assert_eq!(options.format, OutputFormat::Short(tf));
+    }
 
+    #[test]
+    fn short_mode_seconds() {
         let tf = TextFormat { format_durations: false };
         let options = Options::getopts(&[ "dom.ain", "--short", "--seconds" ]).unwrap();
         assert_eq!(options.format, OutputFormat::Short(tf));
@@ -834,12 +850,6 @@ mod test {
                    OptionsResult::InvalidOptions(OptionsError::InvalidQueryType("999999".into())));
     }
 
-    #[test]
-    fn invalid_capsword() {
-        assert_eq!(Options::getopts(&[ "SMH", "lookup.dog" ]),
-                   OptionsResult::InvalidOptions(OptionsError::InvalidQueryType("SMH".into())));
-    }
-
     #[test]
     fn invalid_txid() {
         assert_eq!(Options::getopts(&[ "lookup.dog", "--txid=0x10000" ]),
@@ -858,6 +868,32 @@ mod test {
                    OptionsResult::InvalidOptions(OptionsError::InvalidTweak("sleep".into())));
     }
 
+    #[test]
+    fn invalid_udp_size() {
+        assert_eq!(Options::getopts(&[ "-Z", "bufsize=null" ]),
+                   OptionsResult::InvalidOptions(OptionsError::InvalidTweak("bufsize=null".into())));
+    }
+
+    #[test]
+    fn invalid_udp_size_size() {
+        assert_eq!(Options::getopts(&[ "-Z", "bufsize=999999999" ]),
+                   OptionsResult::InvalidOptions(OptionsError::InvalidTweak("bufsize=999999999".into())));
+    }
+
+    #[test]
+    fn invalid_udp_size_missing() {
+        assert_eq!(Options::getopts(&[ "-Z", "bufsize=" ]),
+                   OptionsResult::InvalidOptions(OptionsError::InvalidTweak("bufsize=".into())));
+    }
+
+    #[test]
+    fn missing_https_url() {
+        assert_eq!(Options::getopts(&[ "--https", "lookup.dog" ]),
+                   OptionsResult::InvalidOptions(OptionsError::MissingHttpsUrl));
+    }
+
+    // opt tests
+
     #[test]
     fn opt() {
         assert_eq!(Options::getopts(&[ "OPT", "lookup.dog" ]),
@@ -871,9 +907,15 @@ mod test {
     }
 
     #[test]
-    fn missing_https_url() {
-        assert_eq!(Options::getopts(&[ "--https", "lookup.dog" ]),
-                   OptionsResult::InvalidOptions(OptionsError::MissingHttpsUrl));
+    fn opt_arg() {
+        assert_eq!(Options::getopts(&[ "-t", "OPT", "lookup.dog" ]),
+                   OptionsResult::InvalidOptions(OptionsError::QueryTypeOPT));
+    }
+
+    #[test]
+    fn opt_arg_lowercase() {
+        assert_eq!(Options::getopts(&[ "-t", "opt", "lookup.dog" ]),
+                   OptionsResult::InvalidOptions(OptionsError::QueryTypeOPT));
     }
 
     // txid tests

xtests/options/errors.toml

@@ -14,14 +14,6 @@ stderr = { file = "outputs/missing-parameter.txt" }
 status = 3
 tags = [ 'options' ]
 
-[[cmd]]
-name = "Running dog with ‘XYZZY’ warns about the invalid record type"
-shell = "dog XYZZY dns.google"
-stdout = { empty = true }
-stderr = { file = "outputs/invalid-query-type.txt" }
-status = 3
-tags = [ 'options' ]
-
 [[cmd]]
 name = "Running dog with ‘--type XYZZY’ warns about the invalid record type"
 shell = "dog --type XYZZY dns.google"
@@ -62,6 +54,22 @@ stderr = { file = "outputs/opt-query.txt" }
 status = 3
 tags = [ 'options' ]
 
+[[cmd]]
+name = "Running dog with ‘--type OPT’ warns that OPT requests are sent by default"
+shell = "dog --type OPT dns.google"
+stdout = { empty = true }
+stderr = { file = "outputs/opt-query.txt" }
+status = 3
+tags = [ 'options' ]
+
+[[cmd]]
+name = "Running dog with ‘--type opt’ also warns that OPT requests are sent by default"
+shell = "dog --type opt dns.google"
+stdout = { empty = true }
+stderr = { file = "outputs/opt-query.txt" }
+status = 3
+tags = [ 'options' ]
+
 [[cmd]]
 name = "Running dog with a domain longer than 255 bytes warns about it being too long"
 shell = "dog 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"