From 85e628036aca325777a84973ee4ee741a6194b27 Mon Sep 17 00:00:00 2001 From: Zhiwei Liang Date: Mon, 21 Sep 2026 03:52:54 -0400 Subject: [PATCH] dd: fix conv=block/unblock conflict handling Signed-off-by: Zhiwei Liang --- src/uu/dd/locales/en-US.ftl | 1 - src/uu/dd/locales/fr-FR.ftl | 1 - src/uu/dd/src/parseargs.rs | 37 ++++---------- src/uu/dd/src/parseargs/unit_tests.rs | 2 +- tests/by-util/test_dd.rs | 69 +++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/uu/dd/locales/en-US.ftl b/src/uu/dd/locales/en-US.ftl index 64531864ff2..0ec90707024 100644 --- a/src/uu/dd/locales/en-US.ftl +++ b/src/uu/dd/locales/en-US.ftl @@ -101,7 +101,6 @@ dd-error-invalid-output-flag = invalid output flag: '{ $flag }' dd-error-conv-flag-no-match = invalid conversion: '{ $flag }' dd-error-multiplier-parse-failure = invalid number: '{ $input }' dd-error-multiplier-overflow = Multiplier string would overflow on current system -> { $input } -dd-error-block-without-cbs = conv=block or conv=unblock specified without cbs=N dd-error-status-not-recognized = invalid status level: '{ $level }' dd-error-unimplemented = feature not implemented on this system -> { $feature } dd-error-bs-out-of-range = { $param }=N cannot fit into memory diff --git a/src/uu/dd/locales/fr-FR.ftl b/src/uu/dd/locales/fr-FR.ftl index e8e0f687fe0..63f93c73561 100644 --- a/src/uu/dd/locales/fr-FR.ftl +++ b/src/uu/dd/locales/fr-FR.ftl @@ -101,7 +101,6 @@ dd-error-invalid-output-flag = indicateur de sortie invalide : '{ $flag }' dd-error-conv-flag-no-match = conversion invalide : '{ $flag }' dd-error-multiplier-parse-failure = nombre invalide : '{ $input }' dd-error-multiplier-overflow = La chaîne de multiplicateur déborderait sur le système actuel -> { $input } -dd-error-block-without-cbs = conv=block ou conv=unblock spécifié sans cbs=N dd-error-status-not-recognized = niveau d'état invalide : '{ $level }' dd-error-unimplemented = fonctionnalité non implémentée sur ce système -> { $feature } dd-error-bs-out-of-range = { $param }=N ne peut pas tenir en mémoire diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index 1749e2d32bd..c83db038085 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -40,8 +40,6 @@ pub enum ParseError { MultiplierStringParseFailure(String), #[error("{}", translate!("dd-error-multiplier-overflow", "input" => .0))] MultiplierStringOverflow(String), - #[error("{}", translate!("dd-error-block-without-cbs"))] - BlockUnblockWithoutCBS, #[error("{}", translate!("dd-error-status-not-recognized", "level" => .0))] StatusLevelNotRecognized(String), #[error("{}", translate!("dd-error-unimplemented", "feature" => .0))] @@ -183,6 +181,12 @@ impl Parser { _ => return Err(ParseError::MultipleFmtTable), }; + let block_requested = conv.block || conv.ebcdic || conv.ibm; + let unblock_requested = conv.unblock || conv.ascii; + if self.cbs.is_some() && block_requested && unblock_requested { + return Err(ParseError::MultipleBlockUnblock); + } + let case = match (conv.ucase, conv.lcase) { (false, false) => None, (true, false) => Some(Case::Upper), @@ -197,31 +201,10 @@ impl Parser { return Err(ParseError::MultipleExclNoCreate); } - // The GNU docs state that - // - ascii implies unblock - // - ebcdic and ibm imply block - // This has a side effect in how it's implemented in GNU, because this errors: - // conv=block,unblock - // but these don't: - // conv=ascii,block,unblock - // conv=block,ascii,unblock - // conv=block,unblock,ascii - // conv=block conv=unblock conv=ascii - let block = if let Some(cbs) = self.cbs { - match conversion { - Some(Conversion::Ascii) => Some(Block::Unblock(cbs)), - Some(_) => Some(Block::Block(cbs)), - None => match (conv.block, conv.unblock) { - (false, false) => None, - (true, false) => Some(Block::Block(cbs)), - (false, true) => Some(Block::Unblock(cbs)), - (true, true) => return Err(ParseError::MultipleBlockUnblock), - }, - } - } else if conv.block || conv.unblock { - return Err(ParseError::BlockUnblockWithoutCBS); - } else { - None + let block = match self.cbs { + Some(cbs) if block_requested => Some(Block::Block(cbs)), + Some(cbs) if unblock_requested => Some(Block::Unblock(cbs)), + _ => None, }; let iconv = IConvFlags { diff --git a/src/uu/dd/src/parseargs/unit_tests.rs b/src/uu/dd/src/parseargs/unit_tests.rs index e41dbe48d33..0ca04cce6e8 100644 --- a/src/uu/dd/src/parseargs/unit_tests.rs +++ b/src/uu/dd/src/parseargs/unit_tests.rs @@ -298,7 +298,7 @@ fn icf_case_error() { #[test] fn icf_block_error() { - let args = ["conv=block,unblock"]; + let args = ["conv=block,unblock", "cbs=1"]; assert!(Parser::new().parse(args).is_err()); } diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 7341b83757c..9abf8a951ed 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -2643,3 +2643,72 @@ dd: unrecognized operand 'bsx=1' .stderr_does_not_contain("╭─"); } } + +// Conversion conflicts are reported in a fixed order: ascii/ebcdic/ibm, +// block/unblock, lcase/ucase, then excl/nocreat. ascii implies unblock; +// ebcdic and ibm imply block, so with cbs an explicit flag in the other +// direction is a block/unblock conflict as well. +#[test] +fn test_block_unblock_conflicts() { + for conv in [ + // block/unblock is reported before the later pairs. + "block,unblock,lcase,ucase", + "block,unblock,excl,nocreat", + // The implied direction conflicts with the explicit flag. + "ascii,block", + "ebcdic,unblock", + "ibm,unblock", + "ascii,block,unblock", + ] { + new_ucmd!() + .args(&[ + format!("conv={conv}"), + "cbs=1".into(), + "if=/dev/null".into(), + ]) + .fails_with_code(1) + .stderr_only("dd: cannot combine block and unblock\n"); + } +} + +#[test] +fn test_implied_block_direction_agrees_with_explicit_flag() { + for conv in ["ascii,unblock", "ebcdic,block", "ibm,block"] { + new_ucmd!() + .args(&[format!("conv={conv}"), "cbs=1".into(), "status=none".into()]) + .pipe_in("") + .succeeds() + .no_output(); + } +} + +// Without cbs, block and unblock have no record length to work with and are +// ignored rather than rejected. +#[test] +fn test_block_unblock_without_cbs_is_ignored() { + for conv in ["block", "unblock", "block,unblock"] { + new_ucmd!() + .args(&[format!("conv={conv}"), "status=none".into()]) + .pipe_in("ab\ncd\n") + .succeeds() + .stdout_only("ab\ncd\n"); + } +} + +// Without cbs, ascii and ebcdic still convert the bytes; only the implied +// unblock or block step is skipped, so the trailing space survives and no +// newline is added. +#[test] +fn test_ascii_ebcdic_without_cbs_still_convert() { + let cases: [(&str, &[u8], &[u8]); 2] = [ + ("ascii", b"\xc1\xc2\x40", b"AB "), + ("ebcdic", b"AB ", b"\xc1\xc2\x40"), + ]; + for (conv, input, expected) in cases { + new_ucmd!() + .args(&[format!("conv={conv}"), "status=none".into()]) + .pipe_in(input) + .succeeds() + .stdout_only_bytes(expected); + } +}