From 2dea9e6f1f2b5a063625d81877a18d8b4eaabeb7 Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Sat, 11 Jun 2022 16:52:31 +0800 Subject: [PATCH] fix arg parse (#5754) * fix arg parse * add ut, fix clippy * simplify code * fmt code --- crates/nu-parser/src/deparse.rs | 158 ++++++++++++++------------------ crates/nu-parser/src/lib.rs | 2 +- src/main.rs | 4 +- 3 files changed, 72 insertions(+), 92 deletions(-) diff --git a/crates/nu-parser/src/deparse.rs b/crates/nu-parser/src/deparse.rs index d3b4844a75..9e0952b2b1 100644 --- a/crates/nu-parser/src/deparse.rs +++ b/crates/nu-parser/src/deparse.rs @@ -1,6 +1,3 @@ -use std::fs::File; -use std::io::{BufRead, BufReader}; - pub fn escape_quote_string(input: &str) -> String { let mut output = String::with_capacity(input.len() + 2); output.push('"'); @@ -16,97 +13,80 @@ pub fn escape_quote_string(input: &str) -> String { output } -fn looks_like_flag(input: &str) -> bool { - if !input.starts_with('-') { - false - // it does not start with '-' - } else if !input.starts_with("--") { - if input.len() > 2 - && input.chars().nth(2).expect("this should never trigger") != '=' - && input.chars().nth(2).expect("this should never trigger") != ' ' - { - false - // while it start with '-', it is not of the form '-x=y' or '-x y' - } else { - input.len() >= 2 +// 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. +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 { + arg_val.into() + }; + return format!("{}={}", arg_name, arg_val); } + } + + if input.contains(' ') { + format!("`{}`", input) + } else if input.contains('"') || input.contains('\\') { + escape_quote_string(input) } else { - input.len() > 2 - // it is either a flag --x or a '--' + input.to_string() } } -fn escape_quote_string_when_flags_are_unclear(input: &str) -> String { - // internal use only. When reading the file for flags goes wrong, revert back to a manual check - // for flags. - let mut output = String::new(); - if !looks_like_flag(input) { - output.push('"'); - for c in input.chars() { - if c == '"' || c == '\\' { - output.push('\\'); - } - output.push(c); - } - output.push('"'); - output - } else if input.contains(' ') || input.contains('=') { - // this is a flag that requires delicate handling - let mut flag_tripped = false; - for c in input.chars() { - if c == '"' || c == '\\' { - output.push('\\'); - } - output.push(c); - if (c == ' ' || c == '=') && !flag_tripped { - flag_tripped = true; - output.push('"'); - } - } - output.push('"'); - output - } else { - // this is a normal flag, aka "--x" - String::from(input) - } -} +#[cfg(test)] +mod test { + use super::escape_for_script_arg; -pub fn escape_quote_string_with_file(input: &str, file: &str) -> String { - // use when you want to cross-compare to a file to ensure flags are checked properly - let file = File::open(file); - match file { - Ok(f) => { - let lines = BufReader::new(f).lines(); - for line in lines { - let mut flag_start = false; - let mut word = String::new(); - let line_or = line.unwrap_or_else(|_| String::from(" ")); - if line_or.contains('-') { - for n in line_or.chars() { - if n == '-' { - flag_start = true; - } - if n == ' ' || n == ':' || n == ')' { - flag_start = false; - } - if flag_start { - word.push(n); - } - } - } - if word.contains(input) || { - let s: Vec<&str> = input.split('=').collect(); - word.contains(s[0]) - } { - return input.to_string(); - } - } - let mut final_word = String::new(); - final_word.push('"'); - final_word.push_str(input); - final_word.push('"'); - final_word - } - _ => escape_quote_string_when_flags_are_unclear(input), + #[test] + fn test_not_extra_quote() { + // check for input arg like this: + // nu b.nu 8 + assert_eq!(escape_for_script_arg("8"), "8".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("--version=v5.2"), + "--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()); + } + + #[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 + 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() + ); + } + + #[test] + fn test_escape() { + // check for input arg like this: + // nu b.nu '"' + assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string()); } } diff --git a/crates/nu-parser/src/lib.rs b/crates/nu-parser/src/lib.rs index 0847ee33c2..00aa1813b4 100644 --- a/crates/nu-parser/src/lib.rs +++ b/crates/nu-parser/src/lib.rs @@ -8,7 +8,7 @@ mod parse_keywords; mod parser; mod type_check; -pub use deparse::{escape_quote_string, escape_quote_string_with_file}; +pub use deparse::{escape_for_script_arg, escape_quote_string}; pub use errors::ParseError; pub use flatten::{flatten_block, flatten_expression, flatten_pipeline, FlatShape}; pub use known_external::KnownExternal; diff --git a/src/main.rs b/src/main.rs index 5dc6660e8b..f9222b843a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,7 @@ use nu_cli::{ }; use nu_command::{create_default_context, BufferedReader}; use nu_engine::{get_full_help, CallExt}; -use nu_parser::{escape_quote_string, escape_quote_string_with_file, parse}; +use nu_parser::{escape_for_script_arg, escape_quote_string, parse}; use nu_protocol::{ ast::{Call, Expr, Expression}, engine::{Command, EngineState, Stack, StateWorkingSet}, @@ -93,7 +93,7 @@ fn main() -> Result<()> { let mut args = std::env::args().skip(1); while let Some(arg) = args.next() { if !script_name.is_empty() { - args_to_script.push(escape_quote_string_with_file(&arg, &script_name)); + args_to_script.push(escape_for_script_arg(&arg)); } else if arg.starts_with('-') { // Cool, it's a flag let flag_value = match arg.as_ref() {