Repository: clap-rs/clap
Pull request #2205 (2020-11-08 18:01:39)

Fix wrong StrictUTF8 handling when AllowExternalSubcommands
The first commit that adds tests will fail in CI, and the second commit will fix bugs and make the failed tests green.

ldm0: The problem is that the branch I modifed is actually an untested branch. I think we could notice it earlier with some code coverage hint....

Patch:
diff --git a/src/parse/parser.rs b/src/parse/parser.rs
index 22788101..64007df1 100644
--- a/src/parse/parser.rs
+++ b/src/parse/parser.rs
@@ -539,8 +539,8 @@ impl<'help, 'app> Parser<'help, 'app> {
                     debug!("Parser::get_matches_with: Bumping the positional counter...");
                     pos_counter += 1;
                 }
-            } else if (self.is_set(AS::AllowMissingPositional) && self.is_set(AS::TrailingValues))
-                || (self.is_set(AS::ContainsLast) && self.is_set(AS::TrailingValues))
+            } else if self.is_set(AS::TrailingValues)
+                && (self.is_set(AS::AllowMissingPositional) || self.is_set(AS::ContainsLast))
             {
                 // Came to -- and one positional has .last(true) set, so we go immediately
                 // to the last (highest index) positional
@@ -603,7 +603,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                 let sc_name = match arg_os.to_str() {
                     Some(s) => s.to_string(),
                     None => {
-                        if !self.is_set(AS::StrictUtf8) {
+                        if self.is_set(AS::StrictUtf8) {
                             return Err(ClapError::invalid_utf8(
                                 Usage::new(self).create_usage_with_title(&[]),
                                 self.app.color(),
@@ -617,7 +617,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                 let mut sc_m = ArgMatcher::default();
 
                 while let Some((v, _)) = it.next() {
-                    if v.to_str().is_none() && !self.is_set(AS::StrictUtf8) {
+                    if self.is_set(AS::StrictUtf8) && v.to_str().is_none() {
                         return Err(ClapError::invalid_utf8(
                             Usage::new(self).create_usage_with_title(&[]),
                             self.app.color(),


Test patch:
diff --git a/tests/utf8.rs b/tests/utf8.rs
index a2aa404f..474ce172 100644
--- a/tests/utf8.rs
+++ b/tests/utf8.rs
@@ -264,3 +264,78 @@ fn invalid_utf8_option_long_equals() {
         &*OsString::from_vec(vec![0xe9])
     );
 }
+
+#[test]
+fn refuse_invalid_utf8_subcommand_with_allow_external_subcommands() {
+    let m = App::new("bad_utf8")
+        .setting(AppSettings::StrictUtf8)
+        .setting(AppSettings::AllowExternalSubcommands)
+        .try_get_matches_from(vec![
+            OsString::from(""),
+            OsString::from_vec(vec![0xe9]),
+            OsString::from("normal"),
+        ]);
+    assert!(m.is_err());
+    assert_eq!(m.unwrap_err().kind, ErrorKind::InvalidUtf8);
+}
+
+#[test]
+fn allow_invalid_utf8_subcommand_with_allow_external_subcommands() {
+    let m = App::new("bad_utf8")
+        .setting(AppSettings::AllowExternalSubcommands)
+        .try_get_matches_from(vec![
+            OsString::from(""),
+            OsString::from_vec(vec![0xe9]),
+            OsString::from("normal"),
+        ]);
+    assert!(m.is_ok());
+    let m = m.unwrap();
+    let (_subcommand, args) = m.subcommand().unwrap();
+    let args = args.values_of_os("").unwrap().collect::<Vec<_>>();
+    assert_eq!(args, vec![OsString::from("normal"),]);
+    // TODO: currently subcommand can only be &str, which means following code
+    // will panic.
+    // assert_eq!(_subcommand, OsString::from_vec(vec![0xe9]));
+}
+
+#[test]
+fn refuse_invalid_utf8_subcommand_args_with_allow_external_subcommands() {
+    let m = App::new("bad_utf8")
+        .setting(AppSettings::StrictUtf8)
+        .setting(AppSettings::AllowExternalSubcommands)
+        .try_get_matches_from(vec![
+            OsString::from(""),
+            OsString::from("subcommand"),
+            OsString::from("normal"),
+            OsString::from_vec(vec![0xe9]),
+            OsString::from("--another_normal"),
+        ]);
+    assert!(m.is_err());
+    assert_eq!(m.unwrap_err().kind, ErrorKind::InvalidUtf8);
+}
+
+#[test]
+fn allow_invalid_utf8_subcommand_args_with_allow_external_subcommands() {
+    let m = App::new("bad_utf8")
+        .setting(AppSettings::AllowExternalSubcommands)
+        .try_get_matches_from(vec![
+            OsString::from(""),
+            OsString::from("subcommand"),
+            OsString::from("normal"),
+            OsString::from_vec(vec![0xe9]),
+            OsString::from("--another_normal"),
+        ]);
+    assert!(m.is_ok());
+    let m = m.unwrap();
+    let (subcommand, args) = m.subcommand().unwrap();
+    let args = args.values_of_os("").unwrap().collect::<Vec<_>>();
+    assert_eq!(subcommand, OsString::from("subcommand"));
+    assert_eq!(
+        args,
+        vec![
+            OsString::from("normal"),
+            OsString::from_vec(vec![0xe9]),
+            OsString::from("--another_normal"),
+        ]
+    );
+}