From 6bf02a66835fce31e8f4fe28f94ecea8b9b96c59 Mon Sep 17 00:00:00 2001 From: Marika Chlebowska Date: Sat, 11 Nov 2023 16:47:17 +0100 Subject: [PATCH 1/4] fix parsing of strings with brackets as script arguments --- crates/nu-parser/src/deparse.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/nu-parser/src/deparse.rs b/crates/nu-parser/src/deparse.rs index e18da10ad8f8..ccfde2d11bee 100644 --- a/crates/nu-parser/src/deparse.rs +++ b/crates/nu-parser/src/deparse.rs @@ -36,7 +36,7 @@ pub fn escape_for_script_arg(input: &str) -> String { } } - if input.contains(' ') { + if input.contains(' ') || (!input.starts_with('$') && input.contains('(')) { format!("`{input}`") } else if input.contains('"') || input.contains('\\') { escape_quote_string(input) @@ -95,4 +95,18 @@ mod test { // nu b.nu '"' assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string()); } + + #[test] + fn test_values_with_brackets() { + // check for input arg like this: + // nu b.nu test_ver some(thing) $something(else) + assert_eq!( + escape_for_script_arg("some(thing)"), + "`some(thing)`".to_string() + ); + assert_eq!( + escape_for_script_arg("$something(else)"), + "$something(else)".to_string() + ); + } } From 692788356121b7190037e1a53d4bea212279ef93 Mon Sep 17 00:00:00 2001 From: Marika Chlebowska Date: Sun, 12 Nov 2023 15:39:40 +0100 Subject: [PATCH 2/4] also handle string containing special characters --- crates/nu-parser/src/deparse.rs | 71 ++++++++++++--------------------- 1 file changed, 25 insertions(+), 46 deletions(-) diff --git a/crates/nu-parser/src/deparse.rs b/crates/nu-parser/src/deparse.rs index ccfde2d11bee..cbf26c089e16 100644 --- a/crates/nu-parser/src/deparse.rs +++ b/crates/nu-parser/src/deparse.rs @@ -14,38 +14,22 @@ pub fn escape_quote_string(input: &str) -> String { } // Escape rules: -// input argument contains ' '(like abc def), we will convert it to `abc def`. -// input argument contains --version='xx yy', we will convert it to --version=`'xx yy'` -// input argument contains " or \, we will try to escape input. +// input argument is a flag with =, we will wrap the value of flag in quotes and escape \ and " (--version=1\3 -> --vesion="1\\3") +// input argument is a flag without =, we will pass it as it is (--version -> --version) +// input argument is not a flag, we will wrap it in quotes and escape \ and " (some other "argument" -> "some other \"argument\"") pub fn escape_for_script_arg(input: &str) -> String { // handle for flag, maybe we need to escape the value. if input.starts_with("--") { if let Some((arg_name, arg_val)) = input.split_once('=') { // only want to escape arg_val. - let arg_val = if arg_val.contains(' ') { - format!("`{arg_val}`") - } else if arg_val.contains('"') || arg_val.contains('\\') { - escape_quote_string(arg_val) - } else if arg_val.is_empty() { - // return an empty string - "''".to_string() - } else { - arg_val.into() - }; + let arg_val = escape_quote_string(arg_val); return format!("{arg_name}={arg_val}"); + } else { + return input.to_string(); } } - if input.contains(' ') || (!input.starts_with('$') && input.contains('(')) { - format!("`{input}`") - } else if input.contains('"') || input.contains('\\') { - escape_quote_string(input) - } else if input.is_empty() { - // return an empty string - "''".to_string() - } else { - input.to_string() - } + escape_quote_string(input) } #[cfg(test)] @@ -53,60 +37,55 @@ mod test { use super::escape_for_script_arg; #[test] - fn test_not_extra_quote() { + fn test_single_character() { // check for input arg like this: // nu b.nu 8 - assert_eq!(escape_for_script_arg("8"), "8".to_string()); + assert_eq!(escape_for_script_arg("8"), r#""8""#.to_string()); + } + + #[test] + fn test_empty_string() { + // check for empty string as an argument + assert_eq!(escape_for_script_arg(""), r#""""#.to_string()); } #[test] fn test_arg_with_flag() { // check for input arg like this: // nu b.nu linux --version=v5.2 - assert_eq!(escape_for_script_arg("linux"), "linux".to_string()); + assert_eq!(escape_for_script_arg("linux"), "\"linux\"".to_string()); assert_eq!( escape_for_script_arg("--version=v5.2"), - "--version=v5.2".to_string() + r#"--version="v5.2""#.to_string() ); // check for input arg like this: // nu b.nu linux --version v5.2 assert_eq!(escape_for_script_arg("--version"), "--version".to_string()); - assert_eq!(escape_for_script_arg("v5.2"), "v5.2".to_string()); + assert_eq!(escape_for_script_arg("v5.2"), r#""v5.2""#.to_string()); } #[test] fn test_flag_arg_with_values_contains_space() { // check for input arg like this: - // nu b.nu test_ver --version='xx yy' --arch=ghi + // nu b.nu test_ver --version='xx yy' assert_eq!( escape_for_script_arg("--version='xx yy'"), - "--version=`'xx yy'`".to_string() - ); - assert_eq!( - escape_for_script_arg("--arch=ghi"), - "--arch=ghi".to_string() + r#"--version="'xx yy'""#.to_string() ); } #[test] fn test_escape() { // check for input arg like this: - // nu b.nu '"' - assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string()); + // nu b.nu '"\' + assert_eq!(escape_for_script_arg(r#""\"#), r#""\"\\""#.to_string()); } #[test] - fn test_values_with_brackets() { + fn test_special_characters() { // check for input arg like this: - // nu b.nu test_ver some(thing) $something(else) - assert_eq!( - escape_for_script_arg("some(thing)"), - "`some(thing)`".to_string() - ); - assert_eq!( - escape_for_script_arg("$something(else)"), - "$something(else)".to_string() - ); + // nu b.nu "$(`'" + assert_eq!(escape_for_script_arg(r#"$(`'"#), r#""$(`'""#.to_string()); } } From 8d282aa7e931c8ab446ab270f840f63945608e2a Mon Sep 17 00:00:00 2001 From: Marika Chlebowska Date: Sun, 12 Nov 2023 16:10:14 +0100 Subject: [PATCH 3/4] allow alphanumeric without quotes, fix typo --- crates/nu-parser/src/deparse.rs | 69 +++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/nu-parser/src/deparse.rs b/crates/nu-parser/src/deparse.rs index cbf26c089e16..6be552583724 100644 --- a/crates/nu-parser/src/deparse.rs +++ b/crates/nu-parser/src/deparse.rs @@ -14,22 +14,33 @@ pub fn escape_quote_string(input: &str) -> String { } // Escape rules: -// input argument is a flag with =, we will wrap the value of flag in quotes and escape \ and " (--version=1\3 -> --vesion="1\\3") -// input argument is a flag without =, we will pass it as it is (--version -> --version) -// input argument is not a flag, we will wrap it in quotes and escape \ and " (some other "argument" -> "some other \"argument\"") +// input argument is not a flag and contains only alphanumeric characters, it is passed as it is (foo -> foo) +// input argument is not a flag and contains non-alphanumeric characters, quotes are added, " and \ are escaped (two \words -> "two \\words") +// input argument is a flag without =, it's passed as it is (--foo -> --foo) +// input argument is a flag with =, the first two points apply to the value (--foo=bar -> --foo=bar; --foo=bar_ -> --foo="bar_") +// +// quotations are needed in case of some characters (like ', `, (, $) to avoid reinterpretation during parsing which leads to errors pub fn escape_for_script_arg(input: &str) -> String { // handle for flag, maybe we need to escape the value. if input.starts_with("--") { if let Some((arg_name, arg_val)) = input.split_once('=') { // only want to escape arg_val. - let arg_val = escape_quote_string(arg_val); + let arg_val = if arg_val.chars().all(|character| character.is_alphanumeric()) { + arg_val.into() + } else { + escape_quote_string(arg_val) + }; + return format!("{arg_name}={arg_val}"); } else { - return input.to_string(); + return input.into(); } } - - escape_quote_string(input) + if input.chars().all(|character| character.is_alphanumeric()) { + input.into() + } else { + escape_quote_string(input) + } } #[cfg(test)] @@ -37,27 +48,38 @@ mod test { use super::escape_for_script_arg; #[test] - fn test_single_character() { + fn test_not_extra_quote() { // check for input arg like this: - // nu b.nu 8 - assert_eq!(escape_for_script_arg("8"), r#""8""#.to_string()); + // nu b.nu word 8 + assert_eq!(escape_for_script_arg("word"), "word".to_string()); + assert_eq!(escape_for_script_arg("8"), "8".to_string()); } #[test] - fn test_empty_string() { - // check for empty string as an argument - assert_eq!(escape_for_script_arg(""), r#""""#.to_string()); + fn test_quote_non_alphanumeric() { + // check for input arg like this: + // nu b.nu "two words" ma$$ "it's" + assert_eq!( + escape_for_script_arg("two words"), + r#""two words""#.to_string() + ); + assert_eq!(escape_for_script_arg("ma$$"), r#""ma$$""#.to_string()); + assert_eq!(escape_for_script_arg("it's"), r#""it's""#.to_string()); } #[test] fn test_arg_with_flag() { // check for input arg like this: - // nu b.nu linux --version=v5.2 - assert_eq!(escape_for_script_arg("linux"), "\"linux\"".to_string()); + // nu b.nu --linux --version=v5.2 --language=en + assert_eq!(escape_for_script_arg("--linux"), "--linux".to_string()); assert_eq!( escape_for_script_arg("--version=v5.2"), r#"--version="v5.2""#.to_string() ); + assert_eq!( + escape_for_script_arg("--language=en"), + "--language=en".to_string() + ); // check for input arg like this: // nu b.nu linux --version v5.2 @@ -66,26 +88,23 @@ mod test { } #[test] - fn test_flag_arg_with_values_contains_space() { + fn test_flag_arg_with_values_contains_non_alphanumeric() { // check for input arg like this: // nu b.nu test_ver --version='xx yy' assert_eq!( escape_for_script_arg("--version='xx yy'"), r#"--version="'xx yy'""#.to_string() ); + assert_eq!( + escape_for_script_arg("--arch=ghi"), + "--arch=ghi".to_string() + ); } #[test] fn test_escape() { // check for input arg like this: - // nu b.nu '"\' - assert_eq!(escape_for_script_arg(r#""\"#), r#""\"\\""#.to_string()); - } - - #[test] - fn test_special_characters() { - // check for input arg like this: - // nu b.nu "$(`'" - assert_eq!(escape_for_script_arg(r#"$(`'"#), r#""$(`'""#.to_string()); + // nu b.nu '"' + assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string()); } } From 41359d4eaa9d267030d3cef1bb01dbf4083b6809 Mon Sep 17 00:00:00 2001 From: Marika Chlebowska Date: Mon, 13 Nov 2023 17:30:08 +0100 Subject: [PATCH 4/4] quote only with spcial characters --- crates/nu-parser/src/deparse.rs | 61 ++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/crates/nu-parser/src/deparse.rs b/crates/nu-parser/src/deparse.rs index 6be552583724..6188fb3e66f7 100644 --- a/crates/nu-parser/src/deparse.rs +++ b/crates/nu-parser/src/deparse.rs @@ -1,3 +1,10 @@ +fn string_should_be_quoted(input: &str) -> bool { + input.starts_with('$') + || input + .chars() + .any(|c| c == ' ' || c == '(' || c == '\'' || c == '`' || c == '"' || c == '\\') +} + pub fn escape_quote_string(input: &str) -> String { let mut output = String::with_capacity(input.len() + 2); output.push('"'); @@ -14,21 +21,21 @@ pub fn escape_quote_string(input: &str) -> String { } // Escape rules: -// input argument is not a flag and contains only alphanumeric characters, it is passed as it is (foo -> foo) -// input argument is not a flag and contains non-alphanumeric characters, quotes are added, " and \ are escaped (two \words -> "two \\words") +// input argument is not a flag, does not start with $ and doesn't contain special characters, it is passed as it is (foo -> foo) +// input argument is not a flag and either starts with $ or contains special characters, quotes are added, " and \ are escaped (two \words -> "two \\words") // input argument is a flag without =, it's passed as it is (--foo -> --foo) -// input argument is a flag with =, the first two points apply to the value (--foo=bar -> --foo=bar; --foo=bar_ -> --foo="bar_") +// input argument is a flag with =, the first two points apply to the value (--foo=bar -> --foo=bar; --foo=bar' -> --foo="bar'") // -// quotations are needed in case of some characters (like ', `, (, $) to avoid reinterpretation during parsing which leads to errors +// special characters are white space, (, ', `, ",and \ pub fn escape_for_script_arg(input: &str) -> String { // handle for flag, maybe we need to escape the value. if input.starts_with("--") { if let Some((arg_name, arg_val)) = input.split_once('=') { // only want to escape arg_val. - let arg_val = if arg_val.chars().all(|character| character.is_alphanumeric()) { - arg_val.into() - } else { + let arg_val = if string_should_be_quoted(arg_val) { escape_quote_string(arg_val) + } else { + arg_val.into() }; return format!("{arg_name}={arg_val}"); @@ -36,10 +43,10 @@ pub fn escape_for_script_arg(input: &str) -> String { return input.into(); } } - if input.chars().all(|character| character.is_alphanumeric()) { - input.into() - } else { + if string_should_be_quoted(input) { escape_quote_string(input) + } else { + input.into() } } @@ -56,55 +63,55 @@ mod test { } #[test] - fn test_quote_non_alphanumeric() { + fn test_quote_special() { // check for input arg like this: - // nu b.nu "two words" ma$$ "it's" + // nu b.nu "two words" $nake "`123" assert_eq!( escape_for_script_arg("two words"), r#""two words""#.to_string() ); - assert_eq!(escape_for_script_arg("ma$$"), r#""ma$$""#.to_string()); - assert_eq!(escape_for_script_arg("it's"), r#""it's""#.to_string()); + assert_eq!(escape_for_script_arg("$nake"), r#""$nake""#.to_string()); + assert_eq!(escape_for_script_arg("`123"), r#""`123""#.to_string()); } #[test] fn test_arg_with_flag() { // check for input arg like this: - // nu b.nu --linux --version=v5.2 --language=en + // nu b.nu --linux --version=v5.2 assert_eq!(escape_for_script_arg("--linux"), "--linux".to_string()); assert_eq!( escape_for_script_arg("--version=v5.2"), - r#"--version="v5.2""#.to_string() - ); - assert_eq!( - escape_for_script_arg("--language=en"), - "--language=en".to_string() + "--version=v5.2".to_string() ); // check for input arg like this: // nu b.nu linux --version v5.2 assert_eq!(escape_for_script_arg("--version"), "--version".to_string()); - assert_eq!(escape_for_script_arg("v5.2"), r#""v5.2""#.to_string()); + assert_eq!(escape_for_script_arg("v5.2"), "v5.2".to_string()); } #[test] - fn test_flag_arg_with_values_contains_non_alphanumeric() { + fn test_flag_arg_with_values_contains_special() { // check for input arg like this: - // nu b.nu test_ver --version='xx yy' + // nu b.nu test_ver --version='xx yy' --separator="`" assert_eq!( escape_for_script_arg("--version='xx yy'"), r#"--version="'xx yy'""#.to_string() ); assert_eq!( - escape_for_script_arg("--arch=ghi"), - "--arch=ghi".to_string() + escape_for_script_arg("--separator=`"), + r#"--separator="`""#.to_string() ); } #[test] fn test_escape() { // check for input arg like this: - // nu b.nu '"' - assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string()); + // nu b.nu \ --arg='"' + assert_eq!(escape_for_script_arg(r#"\"#), r#""\\""#.to_string()); + assert_eq!( + escape_for_script_arg(r#"--arg=""#), + r#"--arg="\"""#.to_string() + ); } }