From f4c2699c8154bf53a1933f018c427d99d438ec44 Mon Sep 17 00:00:00 2001 From: "Steven Zimmerman, CPA" <15812269+EffortlessSteven@users.noreply.github.com> Date: Tue, 21 Jul 2026 02:40:55 -0400 Subject: [PATCH] tr: handle trailing-backslash warnings for both sets tr checked only SET1 for an unescaped trailing backslash, so SET2 never produced GNU's portability warning. Warning writes also went through helpers that discard stderr errors, so redirecting a warning to /dev/full still exited successfully. Move terminal-backslash detection from the SET1-only pre-scan in tr.rs to operand parse completion, so SET1 and SET2 are checked in parse order and the warning follows any warning emitted while scanning the same operand. Parse both operands before applying semantic validation, so SET2 warnings and syntax errors surface ahead of a semantic rejection of a syntactically valid SET1. Add a tr-local checked diagnostic writer that records exit status 1 when a warning cannot be written, without returning early, so stdin processing and stdout output continue. Route tr's existing ambiguous-octal and invalid-UTF-8 warnings through the same writer. The shared uucore diagnostic macros are unchanged. Move the localized "warning: " label into the tr-warning-* Fluent messages so the label and its locale-specific punctuation are translated rather than hardcoded in Rust. This changes two existing message keys from warning bodies into complete warnings. --- src/uu/tr/locales/en-US.ftl | 4 +- src/uu/tr/locales/fr-FR.ftl | 4 +- src/uu/tr/src/operation.rs | 50 ++++++++++++---- src/uu/tr/src/tr.rs | 14 +---- tests/by-util/test_tr.rs | 110 ++++++++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 27 deletions(-) diff --git a/src/uu/tr/locales/en-US.ftl b/src/uu/tr/locales/en-US.ftl index f91f2de3033..e4ef235efba 100644 --- a/src/uu/tr/locales/en-US.ftl +++ b/src/uu/tr/locales/en-US.ftl @@ -22,8 +22,8 @@ tr-error-write-error = write error # Warning messages tr-warning-unescaped-backslash = warning: an unescaped backslash at end of string is not portable -tr-warning-ambiguous-octal-escape = the ambiguous octal escape \{ $origin_octal } is being interpreted as the 2-byte sequence \0{ $actual_octal_tail }, { $outstand_char } -tr-warning-invalid-utf8 = invalid utf8 sequence +tr-warning-ambiguous-octal-escape = warning: the ambiguous octal escape \{ $origin_octal } is being interpreted as the 2-byte sequence \0{ $actual_octal_tail }, { $outstand_char } +tr-warning-invalid-utf8 = warning: invalid utf8 sequence # Sequence parsing error messages tr-error-missing-char-class-name = missing character class name '[::]' diff --git a/src/uu/tr/locales/fr-FR.ftl b/src/uu/tr/locales/fr-FR.ftl index 0b59ea133b0..9a293ad8b8d 100644 --- a/src/uu/tr/locales/fr-FR.ftl +++ b/src/uu/tr/locales/fr-FR.ftl @@ -22,9 +22,9 @@ tr-error-write-error = erreur d'écriture # Messages d'avertissement tr-warning-unescaped-backslash = avertissement : une barre oblique inverse non échappée à la fin de la chaîne n'est pas portable -tr-warning-ambiguous-octal-escape = l'échappement octal ambigu \{ $origin_octal } est en cours +tr-warning-ambiguous-octal-escape = avertissement : l'échappement octal ambigu \{ $origin_octal } est en cours d'interprétation comme la séquence de 2 octets \0{ $actual_octal_tail }, { $outstand_char } -tr-warning-invalid-utf8 = séquence UTF-8 non valide +tr-warning-invalid-utf8 = avertissement : séquence UTF-8 non valide # Messages d'erreur d'analyse de séquence tr-error-missing-char-class-name = nom de classe de caractères manquant '[::]' diff --git a/src/uu/tr/src/operation.rs b/src/uu/tr/src/operation.rs index 44c28c160c8..11204bbe1a0 100644 --- a/src/uu/tr/src/operation.rs +++ b/src/uu/tr/src/operation.rs @@ -21,10 +21,28 @@ use std::{ fmt::{Debug, Display}, io::{BufRead, Write}, }; -use uucore::error::{FromIo, UError, UResult}; +use uucore::error::{FromIo, UError, UResult, set_exit_code}; use uucore::translate; -use uucore::show_warning; +/// Write a diagnostic to stderr, recording exit status 1 if the write fails. +/// +/// Unlike `show_warning!`, this does not discard stderr write errors. The +/// failure is recorded but not propagated, so translation of stdin continues +/// and the translated stdout is preserved, as GNU does. A successful +/// diagnostic leaves the stored status unchanged. +fn emit_diagnostic(message: impl Display) { + let mut stderr = std::io::stderr().lock(); + + if writeln!(stderr, "{}: {message}", uucore::util_name()).is_err() { + set_exit_code(1); + } +} + +/// Whether the raw operand ends in an odd-length run of backslashes, i.e. a +/// final backslash that is not itself escaped. +fn has_unescaped_trailing_backslash(input: &[u8]) -> bool { + input.iter().rev().take_while(|&&b| b == b'\\').count() % 2 == 1 +} /// Common trait for operations that can process chunks of data pub trait ChunkProcessor { @@ -217,12 +235,18 @@ impl Sequence { ) -> Result<(Vec, Vec), BadSequence> { let is_char_star = |s: &&Self| -> bool { matches!(s, Self::CharStar(_)) }; + // Both operands are parsed before either is validated, so that warnings + // and syntax errors from set2 are reported ahead of a semantic + // rejection of a syntactically valid set1, as GNU does. Set1 is still + // parsed first, so a syntax error in it prevents set2 from being + // parsed at all. let set1 = Self::from_str(set1_str)?; + let mut set2 = Self::from_str(set2_str)?; + if set1.iter().filter(is_char_star).count() != 0 { return Err(BadSequence::CharRepeatInSet1); } - let mut set2 = Self::from_str(set2_str)?; if set2.iter().filter(is_char_star).count() > 1 { return Err(BadSequence::MultipleCharRepeatInSet2); } @@ -350,7 +374,7 @@ impl Sequence { impl Sequence { pub fn from_str(input: &[u8]) -> Result, BadSequence> { - many0(alt(( + let parsed = many0(alt(( Self::parse_char_range, Self::parse_char_star, Self::parse_char_repeat, @@ -363,9 +387,16 @@ impl Sequence { ))) .parse(input) .map(|(_, r)| r) - .unwrap() - .into_iter() - .collect::, _>>() + .unwrap(); + + // Warn once per operand, after the whole operand has been scanned, so + // that this warning follows any warning emitted while parsing it and + // precedes a syntax error found in the same operand. + if has_unescaped_trailing_backslash(input) { + emit_diagnostic(translate!("tr-warning-unescaped-backslash")); + } + + parsed.into_iter().collect::, _>>() } fn parse_octal(input: &[u8]) -> IResult<&[u8], u8> { @@ -409,12 +440,11 @@ impl Sequence { if let Ok(origin_octal) = std::str::from_utf8(input) { let actual_octal_tail: &str = std::str::from_utf8(&input[0..2]).unwrap(); let outstand_char: char = char::from_u32(input[2] as u32).unwrap(); - show_warning!( - "{}", + emit_diagnostic( translate!("tr-warning-ambiguous-octal-escape", "origin_octal" => origin_octal, "actual_octal_tail" => actual_octal_tail, "outstand_char" => outstand_char) ); } else { - show_warning!("{}", translate!("tr-warning-invalid-utf8")); + emit_diagnostic(translate!("tr-warning-invalid-utf8")); } } result diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 58976d3ac06..857d0748ee0 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -19,7 +19,7 @@ use uucore::display::Quotable; use uucore::error::{UResult, USimpleError, UUsageError}; use uucore::fs::is_stdin_directory; use uucore::translate; -use uucore::{format_usage, os_str_as_bytes, show}; +use uucore::{format_usage, os_str_as_bytes}; mod options { pub const COMPLEMENT: &str = "complement"; @@ -84,18 +84,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - if let Some(first) = sets.first() { - let slice = os_str_as_bytes(first)?; - let trailing_backslashes = slice.iter().rev().take_while(|&&c| c == b'\\').count(); - if trailing_backslashes % 2 == 1 { - // The trailing backslash has a non-backslash character before it. - show!(USimpleError::new( - 0, - translate!("tr-warning-unescaped-backslash") - )); - } - } - let stdin = stdin(); let mut locked_stdin = stdin.lock(); let mut locked_stdout = stdout().lock(); diff --git a/tests/by-util/test_tr.rs b/tests/by-util/test_tr.rs index 066474fc985..b4bcd6a128f 100644 --- a/tests/by-util/test_tr.rs +++ b/tests/by-util/test_tr.rs @@ -1572,6 +1572,116 @@ fn test_trailing_backslash() { .stdout_is("abc"); } +#[test] +fn test_trailing_backslash_warning_in_set2() { + new_ucmd!() + .args(&[".", r"\"]) + .pipe_in(".") + .succeeds() + .stderr_is("tr: warning: an unescaped backslash at end of string is not portable\n") + .stdout_is("\\"); +} + +#[test] +fn test_trailing_backslash_warning_in_both_sets() { + new_ucmd!() + .args(&[r"\", r"\"]) + .pipe_in("\\") + .succeeds() + .stderr_is(concat!( + "tr: warning: an unescaped backslash at end of string is not portable\n", + "tr: warning: an unescaped backslash at end of string is not portable\n", + )) + .stdout_is("\\"); +} + +#[test] +fn test_escaped_trailing_backslash_in_set2_does_not_warn() { + new_ucmd!() + .args(&[".", r"\\"]) + .pipe_in(".") + .succeeds() + .no_stderr() + .stdout_is("\\"); +} + +#[test] +fn test_set1_syntax_error_prevents_set2_warning() { + new_ucmd!() + .args(&["z-a", r"\"]) + .pipe_in("") + .fails() + .stderr_only("tr: range-endpoints of 'z-a' are in reverse collating sequence order\n"); +} + +#[test] +fn test_set2_warning_precedes_set1_semantic_error() { + new_ucmd!() + .args(&["[x*]", r"\"]) + .pipe_in("") + .fails() + .stderr_is(concat!( + "tr: warning: an unescaped backslash at end of string is not portable\n", + "tr: the [c*] repeat construct may not appear in string1\n", + )); +} + +#[test] +fn test_set2_syntax_error_precedes_set1_semantic_error() { + new_ucmd!() + .args(&["[x*]", "z-a"]) + .pipe_in("") + .fails() + .stderr_only("tr: range-endpoints of 'z-a' are in reverse collating sequence order\n"); +} + +#[test] +fn test_set2_trailing_warning_precedes_its_syntax_error() { + new_ucmd!() + .args(&["x", r"z-a\"]) + .pipe_in("") + .fails() + .stderr_is(concat!( + "tr: warning: an unescaped backslash at end of string is not portable\n", + "tr: range-endpoints of 'z-a' are in reverse collating sequence order\n", + )); +} + +#[test] +fn test_parser_warning_precedes_trailing_backslash_warning() { + // Only the relative warning order is relevant to this regression. + let result = new_ucmd!() + .args(&["-d", r"\501\"]) + .pipe_in("(1Ł)") + .succeeds(); + result.stdout_is("Ł)"); + + let stderr = result.stderr_str(); + let ambiguous_octal = stderr.find("warning: the ambiguous octal escape").unwrap(); + let trailing_backslash = stderr + .find("warning: an unescaped backslash at end of string is not portable") + .unwrap(); + assert!(ambiguous_octal < trailing_backslash); +} + +#[cfg(target_os = "linux")] +#[test] +fn test_warning_write_failures() { + // A warning that cannot be written sets the exit status without discarding + // the translated output, for both the trailing-backslash and the parser + // warning paths. + for (args, input, expected_stdout) in + [([".", r"\"], ".", "\\"), (["-d", r"\501"], "(1Ł)", "Ł)")] + { + new_ucmd!() + .args(&args) + .pipe_in(input) + .set_stderr(std::fs::File::create("/dev/full").unwrap()) + .fails_with_code(1) + .stdout_is(expected_stdout); + } +} + #[test] fn test_multibyte_octal_sequence() { new_ucmd!()