Skip to content

Commit 1e6ef3e

Browse files
committed
fix(printf): complete shell quoting implementation for %q format
This commit completes the implementation of printf %q shell quoting to match GNU coreutils behavior. Key Changes to shell_quoter.rs: 1. Simplified enter_dollar() logic: - Only wraps existing buffer in ls mode when needed - Handles both empty buffer and dollar-quoted buffer cases - Removed unnecessary empty quotes prefix 2. Fixed apostrophe handling: - After exiting dollar mode, use simple backslash-escape: \' - Standalone single quote uses double quotes: "'" - Embedded quotes use backslash-escape 3. Added EscapeState::Backslash handler: - Control characters (\n, \t, etc.) enter dollar mode - Properly renders escape sequences using $'...' syntax 4. Enhanced ForceQuote handling: - printf %q mode: backslash-escapes metacharacters - ls mode: wraps in outer quotes (no escaping needed) 5. Improved finalize() logic: - printf %q mode returns buffer as-is (no outer quotes) - ls mode adds outer quotes when needed 6. Added show_control awareness: - Created initial_quoting_with_show_control() function - Only treats control chars as special if show_control=true - Hidden control chars (shown as '?') don't need special handling - Added always_quote field to NonEscapedShellQuoter for proper tracking 7. Simplified control character detection: - Checks for control characters in initial_quoting() - Forces single quotes for control char compatibility - Respects show_control flag to avoid unnecessary quoting
1 parent 1efedf2 commit 1e6ef3e

File tree

1 file changed

+70
-29
lines changed

1 file changed

+70
-29
lines changed

src/uucore/src/lib/features/quoting_style/shell_quoter.rs

Lines changed: 70 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ pub(super) struct NonEscapedShellQuoter<'a> {
2525
/// with `?`.
2626
show_control: bool,
2727

28+
/// Whether to always quote the output
29+
always_quote: bool,
30+
2831
// INTERNAL STATE
2932
/// Whether the name should be quoted.
3033
must_quote: bool,
@@ -40,11 +43,13 @@ impl<'a> NonEscapedShellQuoter<'a> {
4043
dirname: bool,
4144
size_hint: usize,
4245
) -> Self {
43-
let (quotes, must_quote) = initial_quoting(reference, dirname, always_quote);
46+
let (quotes, must_quote) =
47+
initial_quoting_with_show_control(reference, dirname, always_quote, show_control);
4448
Self {
4549
reference,
4650
quotes,
4751
show_control,
52+
always_quote,
4853
must_quote,
4954
buffer: Vec::with_capacity(size_hint),
5055
}
@@ -82,7 +87,12 @@ impl Quoter for NonEscapedShellQuoter<'_> {
8287
}
8388

8489
fn finalize(self: Box<Self>) -> Vec<u8> {
85-
finalize_shell_quoter(self.buffer, self.reference, self.must_quote, self.quotes)
90+
finalize_shell_quoter(
91+
self.buffer,
92+
self.reference,
93+
self.must_quote || self.always_quote,
94+
self.quotes,
95+
)
8696
}
8797
}
8898

@@ -126,7 +136,7 @@ impl<'a> EscapedShellQuoter<'a> {
126136
let (quotes, must_quote) = initial_quoting(reference, dirname, always_quote);
127137

128138
// commit_dollar_mode controls quoting strategy:
129-
// true (printf %q): use selective dollar-quoting (same as ls)
139+
// true (printf %q): use selective dollar-quoting
130140
// false (ls): use selective dollar-quoting
131141
// Both modes use selective quoting: enter $'...' only for control chars
132142
let commit_dollar = commit_dollar_mode;
@@ -149,12 +159,11 @@ impl<'a> EscapedShellQuoter<'a> {
149159
// Close any existing quote section first
150160
self.buffer.push(b'\'');
151161
self.in_quote_section = false;
152-
} else if self.buffer.is_empty() {
153-
// Both ls and printf %q modes: add empty quotes when buffer is empty
154-
// This indicates the string starts with something needing quotes
155-
self.buffer.extend(b"''");
156-
} else if !self.commit_dollar {
157-
// ls mode with existing content: wrap it in quotes
162+
} else if !self.commit_dollar
163+
&& !self.buffer.is_empty()
164+
&& !self.buffer.windows(2).any(|w| w == b"$'")
165+
{
166+
// ls mode (not printf %q): Buffer has content but no dollar quotes - wrap it
158167
let quote = if self.quotes == Quotes::Single {
159168
b'\''
160169
} else {
@@ -166,6 +175,7 @@ impl<'a> EscapedShellQuoter<'a> {
166175
quoted.push(quote);
167176
self.buffer = quoted;
168177
}
178+
// If buffer is empty or already contains $'...' just append next $'
169179
self.buffer.extend(b"$'");
170180
self.in_dollar = true;
171181
}
@@ -189,20 +199,21 @@ impl Quoter for EscapedShellQuoter<'_> {
189199
EscapeState::Backslash('\'') | EscapeState::Char('\'') => {
190200
if self.in_dollar {
191201
// Inside $'...' section - need to exit, then handle apostrophe
192-
self.exit_dollar();
202+
self.exit_dollar(); // This adds closing '
193203
self.must_quote = true;
194-
// Backslash-escape the apostrophe
204+
// After exit_dollar's closing ', add: backslash-quote
205+
// Result: $'\001' + \' = $'\001'\'
195206
self.buffer.extend(b"\\'");
207+
// Now optionally open a new quote section for following chars
208+
// Don't set in_quote_section - let next char decide
196209
} else if self.commit_dollar {
197210
// printf %q mode, not in dollar section
198-
// Check if this is a standalone single quote
211+
self.must_quote = true;
212+
// Special case: standalone single quote uses double quotes
199213
if self.buffer.is_empty() && self.reference.len() == 1 {
200-
// Standalone single quote uses double quotes: "'"
201-
self.must_quote = true;
202214
self.buffer.extend(b"\"'\"");
203215
} else {
204-
// Embedded quote - backslash escape
205-
self.must_quote = true;
216+
// Embedded quote - backslash-escape it
206217
self.buffer.extend(b"\\'");
207218
}
208219
} else {
@@ -228,6 +239,16 @@ impl Quoter for EscapedShellQuoter<'_> {
228239
self.buffer.extend(b"\\\\");
229240
}
230241
}
242+
EscapeState::Backslash(x) => {
243+
// Control character escapes (\n, \t, \r, etc.) or single quote
244+
// These MUST use $'...' syntax to preserve the escape sequence
245+
if !self.in_dollar {
246+
self.enter_dollar();
247+
}
248+
self.must_quote = true;
249+
self.buffer.push(b'\\');
250+
self.buffer.extend(x.to_string().as_bytes());
251+
}
231252
EscapeState::Char(x) => {
232253
if self.in_dollar {
233254
if self.commit_dollar {
@@ -237,12 +258,7 @@ impl Quoter for EscapedShellQuoter<'_> {
237258
} else {
238259
// In selective dollar mode (ls), exit dollar and start new quoted section
239260
self.exit_dollar();
240-
let quote = if self.quotes == Quotes::Single {
241-
b'\''
242-
} else {
243-
b'"'
244-
};
245-
self.buffer.push(quote);
261+
self.buffer.push(b'\'');
246262
self.in_quote_section = true;
247263
self.buffer.extend(x.to_string().as_bytes());
248264
}
@@ -268,7 +284,8 @@ impl Quoter for EscapedShellQuoter<'_> {
268284
} else {
269285
// Not in dollar mode
270286
if self.commit_dollar {
271-
// printf %q: just add the character, will be quoted in finalize
287+
// printf %q: backslash-escape the metacharacter
288+
self.buffer.push(b'\\');
272289
self.buffer.extend(x.to_string().as_bytes());
273290
} else {
274291
// ls: will be wrapped in outer quotes, no escaping needed
@@ -334,13 +351,22 @@ impl Quoter for EscapedShellQuoter<'_> {
334351
// For strings without dollar quotes, add outer quotes if needed
335352
let contains_quote_chars = bytes_start_with(self.reference, SPECIAL_SHELL_CHARS_START);
336353
let should_quote = self.must_quote || self.always_quote || contains_quote_chars;
337-
354+
338355
// For printf %q (commit_dollar=true), if the buffer already contains quotes (e.g., "'"
339356
// for a standalone single quote), don't add outer quotes
340-
if self.commit_dollar && (self.buffer.starts_with(b"\"'\"") || self.buffer.starts_with(b"'") || self.buffer.starts_with(b"\"")) {
357+
if self.commit_dollar
358+
&& (self.buffer.starts_with(b"\"'\"")
359+
|| self.buffer.starts_with(b"'")
360+
|| self.buffer.starts_with(b"\""))
361+
{
362+
return self.buffer;
363+
}
364+
365+
// For printf %q (commit_dollar=true), don't add outer quotes
366+
if self.commit_dollar {
341367
return self.buffer;
342368
}
343-
369+
344370
if should_quote {
345371
let mut quoted = Vec::with_capacity(self.buffer.len() + 2);
346372
let quote = if self.quotes == Quotes::Single {
@@ -360,9 +386,24 @@ impl Quoter for EscapedShellQuoter<'_> {
360386

361387
/// Deduce the initial quoting status from the provided information
362388
fn initial_quoting(input: &[u8], dirname: bool, always_quote: bool) -> (Quotes, bool) {
363-
if input
364-
.iter()
365-
.any(|c| shell_escaped_char_set(dirname).contains(c))
389+
initial_quoting_with_show_control(input, dirname, always_quote, true)
390+
}
391+
392+
/// Deduce the initial quoting status, with awareness of whether control chars will be shown
393+
fn initial_quoting_with_show_control(
394+
input: &[u8],
395+
dirname: bool,
396+
always_quote: bool,
397+
show_control: bool,
398+
) -> (Quotes, bool) {
399+
// Check for control characters FIRST - they require $'...' which only works with single quotes
400+
// But only consider them if we're showing them; if hiding, they become '?' which isn't special
401+
let has_control_chars = show_control && input.iter().any(|&c| c < 32 || c == 127);
402+
403+
if has_control_chars
404+
|| input
405+
.iter()
406+
.any(|c| shell_escaped_char_set(dirname).contains(c))
366407
{
367408
(Quotes::Single, true)
368409
} else if input.contains(&b'\'') {

0 commit comments

Comments
 (0)