Skip to content

Commit 203440c

Browse files
branchseerclaude
andauthored
Improve shell quote handling with proper nested quote support (#217)
## Summary Replace `unquote_str` with `brush_parser::word::parse` for context-aware shell unquoting. The old function stripped all quote characters uniformly, breaking commands with nested quotes. ## Example A `prepare` script like: ```json { "prepare": "node -e \"const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});\"" } ``` Was parsed incorrectly because `unquote_str` stripped the single quotes inside the double-quoted string: ```diff - require(child_process).execSync(vp config, {stdio: inherit}); + require('child_process').execSync('vp config', {stdio: 'inherit'}); ``` ## Test plan - [x] `cargo test -p vite_shell` — all 7 tests pass - [x] `test_unquote_preserves_nested_quotes` — single quotes in double quotes, double quotes in single quotes, escape sequences - [x] `test_flatten_pieces_recursion` — recursive flattening through `word::parse`, bail on `$VAR` and `$(cmd)` - [x] `test_parse_urllib_prepare` — real-world prepare script with nested quotes https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4bbcba1 commit 203440c

File tree

1 file changed

+110
-16
lines changed

1 file changed

+110
-16
lines changed

crates/vite_shell/src/lib.rs

Lines changed: 110 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use brush_parser::{
88
CommandPrefixOrSuffixItem, CommandSuffix, CompoundListItem, Pipeline, Program,
99
SeparatorOperator, SimpleCommand, SourceLocation, Word,
1010
},
11-
unquote_str,
11+
word::{WordPiece, WordPieceWithSource},
1212
};
1313
use diff::Diff;
1414
use serde::{Deserialize, Serialize};
@@ -42,10 +42,52 @@ impl Display for TaskParsedCommand {
4242
}
4343
}
4444

45-
#[expect(clippy::disallowed_types, reason = "brush_parser::unquote_str returns String")]
46-
fn unquote(word: &Word) -> String {
45+
/// Parser options matching those used in [`try_parse_as_and_list`].
46+
const PARSER_OPTIONS: ParserOptions = ParserOptions {
47+
enable_extended_globbing: false,
48+
posix_mode: true,
49+
sh_mode: true,
50+
tilde_expansion: false,
51+
};
52+
53+
/// Remove shell quoting from a word value, respecting quoting context.
54+
///
55+
/// Uses `brush_parser::word::parse` to properly handle nested quoting
56+
/// (e.g. single quotes inside double quotes are preserved as literal characters).
57+
/// Returns `None` if the word contains expansions that cannot be statically resolved
58+
/// (parameter expansion, command substitution, arithmetic).
59+
fn unquote(word: &Word) -> Option<Str> {
4760
let Word { value, loc: _ } = word;
48-
unquote_str(value.as_str())
61+
let pieces = brush_parser::word::parse(value.as_str(), &PARSER_OPTIONS).ok()?;
62+
let mut result = Str::with_capacity(value.len());
63+
flatten_pieces(&pieces, &mut result)?;
64+
Some(result)
65+
}
66+
67+
/// Recursively extract literal text from parsed word pieces.
68+
///
69+
/// Returns `None` if any piece requires runtime expansion.
70+
fn flatten_pieces(pieces: &[WordPieceWithSource], result: &mut Str) -> Option<()> {
71+
for piece in pieces {
72+
match &piece.piece {
73+
WordPiece::Text(s) | WordPiece::SingleQuotedText(s) | WordPiece::AnsiCQuotedText(s) => {
74+
result.push_str(s);
75+
}
76+
// EscapeSequence contains the raw sequence (e.g. `\"` as two chars);
77+
// the escaped character is everything after the leading backslash.
78+
WordPiece::EscapeSequence(s) => {
79+
result.push_str(s.strip_prefix('\\').unwrap_or(s));
80+
}
81+
WordPiece::DoubleQuotedSequence(inner)
82+
| WordPiece::GettextDoubleQuotedSequence(inner) => {
83+
flatten_pieces(inner, result)?;
84+
}
85+
// Tilde prefix, parameter expansion, command substitution, arithmetic
86+
// cannot be statically resolved — bail out.
87+
_ => return None,
88+
}
89+
}
90+
Some(())
4991
}
5092

5193
fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range<usize>)> {
@@ -78,7 +120,7 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range<
78120
let AssignmentValue::Scalar(value) = value else {
79121
return None;
80122
};
81-
envs.insert(name.as_str().into(), unquote(value).into());
123+
envs.insert(name.as_str().into(), unquote(value)?);
82124
}
83125
}
84126
let mut args = Vec::<Str>::new();
@@ -87,23 +129,15 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range<
87129
let CommandPrefixOrSuffixItem::Word(word) = suffix_item else {
88130
return None;
89131
};
90-
args.push(unquote(word).into());
132+
args.push(unquote(word)?);
91133
}
92134
}
93-
Some((TaskParsedCommand { envs, program: unquote(program).into(), args }, range))
135+
Some((TaskParsedCommand { envs, program: unquote(program)?, args }, range))
94136
}
95137

96138
#[must_use]
97139
pub fn try_parse_as_and_list(cmd: &str) -> Option<Vec<(TaskParsedCommand, Range<usize>)>> {
98-
let mut parser = Parser::new(
99-
cmd.as_bytes(),
100-
&ParserOptions {
101-
enable_extended_globbing: false,
102-
posix_mode: true,
103-
sh_mode: true,
104-
tilde_expansion: false,
105-
},
106-
);
140+
let mut parser = Parser::new(cmd.as_bytes(), &PARSER_OPTIONS);
107141
let Program { complete_commands } = parser.parse_program().ok()?;
108142
let [compound_list] = complete_commands.as_slice() else {
109143
return None;
@@ -202,6 +236,66 @@ mod tests {
202236
assert!(str1.starts_with("ALPHA=first MIDDLE=middle ZEBRA=last"));
203237
}
204238

239+
#[test]
240+
fn test_unquote_preserves_nested_quotes() {
241+
// Single quotes inside double quotes are preserved
242+
let cmd = r#"echo "hello 'world'""#;
243+
let list = try_parse_as_and_list(cmd).unwrap();
244+
assert_eq!(list[0].0.args[0].as_str(), "hello 'world'");
245+
246+
// Double quotes inside single quotes are preserved
247+
let cmd = r#"echo 'hello "world"'"#;
248+
let list = try_parse_as_and_list(cmd).unwrap();
249+
assert_eq!(list[0].0.args[0].as_str(), "hello \"world\"");
250+
251+
// Backslash escaping in double quotes
252+
let cmd = r#"echo "hello\"world""#;
253+
let list = try_parse_as_and_list(cmd).unwrap();
254+
assert_eq!(list[0].0.args[0].as_str(), "hello\"world");
255+
256+
// Backslash escaping outside quotes
257+
let cmd = r"echo hello\ world";
258+
let list = try_parse_as_and_list(cmd).unwrap();
259+
assert_eq!(list[0].0.args[0].as_str(), "hello world");
260+
}
261+
262+
#[test]
263+
fn test_flatten_pieces_recursion() {
264+
fn parse_and_flatten(input: &str) -> Option<Str> {
265+
let pieces = brush_parser::word::parse(input, &PARSER_OPTIONS).ok()?;
266+
let mut result = Str::default();
267+
flatten_pieces(&pieces, &mut result)?;
268+
Some(result)
269+
}
270+
271+
// DoubleQuotedSequence containing Text + EscapeSequence + Text
272+
assert_eq!(parse_and_flatten(r#""hello\"world""#).unwrap(), "hello\"world");
273+
274+
// DoubleQuotedSequence with single quotes preserved as literal text
275+
assert_eq!(parse_and_flatten(r#""it's a 'test'""#).unwrap(), "it's a 'test'");
276+
277+
// Nested escape sequences inside double quotes
278+
assert_eq!(parse_and_flatten(r#""a\\b""#).unwrap(), "a\\b");
279+
280+
// DoubleQuotedSequence bails on parameter expansion inside
281+
assert!(parse_and_flatten(r#""hello $VAR""#).is_none());
282+
283+
// DoubleQuotedSequence bails on command substitution inside
284+
assert!(parse_and_flatten(r#""hello $(cmd)""#).is_none());
285+
}
286+
287+
#[test]
288+
fn test_parse_urllib_prepare() {
289+
let cmd = r#"node -e "const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});""#;
290+
let result = try_parse_as_and_list(cmd);
291+
let (parsed, _) = &result.as_ref().unwrap()[0];
292+
// Single quotes inside double quotes must be preserved as literal characters
293+
assert_eq!(
294+
parsed.args[1].as_str(),
295+
"const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});"
296+
);
297+
}
298+
205299
#[test]
206300
fn test_task_parsed_command_serialization_stability() {
207301
use bincode::{decode_from_slice, encode_to_vec};

0 commit comments

Comments
 (0)