Merge pull request #6882 from jtracey/quoting_style_bytes

quoting_style: Add support for non-UTF-8 bytes

.clippy.toml

@@ -1,4 +1,4 @@
-msrv = "1.77.0"
+msrv = "1.79.0"
 cognitive-complexity-threshold = 24
 missing-docs-in-crate-items = true
 check-private-items = true

.github/workflows/CICD.yml

@@ -11,7 +11,7 @@ env:
   PROJECT_NAME: coreutils
   PROJECT_DESC: "Core universal (cross-platform) utilities"
   PROJECT_AUTH: "uutils"
-  RUST_MIN_SRV: "1.77.0"
+  RUST_MIN_SRV: "1.79.0"
   # * style job configuration
   STYLE_FAIL_ON_FAULT: true ## (bool) fail the build if a style job contains a fault (error or warning); may be overridden on a per-job basis
 

Cargo.toml

@@ -16,7 +16,7 @@ repository = "https://github.com/uutils/coreutils"
 readme = "README.md"
 keywords = ["coreutils", "uutils", "cross-platform", "cli", "utility"]
 categories = ["command-line-utilities"]
-rust-version = "1.77.0"
+rust-version = "1.79.0"
 edition = "2021"
 
 build = "build.rs"

README.md

@@ -14,7 +14,7 @@
 [![dependency status](https://deps.rs/repo/github/uutils/coreutils/status.svg)](https://deps.rs/repo/github/uutils/coreutils)
 
 [![CodeCov](https://codecov.io/gh/uutils/coreutils/branch/master/graph/badge.svg)](https://codecov.io/gh/uutils/coreutils)
-![MSRV](https://img.shields.io/badge/MSRV-1.77.0-brightgreen)
+![MSRV](https://img.shields.io/badge/MSRV-1.79.0-brightgreen)
 
 </div>
 
@@ -70,7 +70,7 @@ the [coreutils docs](https://github.com/uutils/uutils.github.io) repository.
 ### Rust Version
 
 uutils follows Rust's release channels and is tested against stable, beta and
-nightly. The current Minimum Supported Rust Version (MSRV) is `1.77.0`.
+nightly. The current Minimum Supported Rust Version (MSRV) is `1.79.0`.
 
 ## Building
 

src/uu/ls/src/ls.rs

@@ -21,7 +21,7 @@ use std::os::windows::fs::MetadataExt;
 use std::{
     cmp::Reverse,
     error::Error,
-    ffi::OsString,
+    ffi::{OsStr, OsString},
     fmt::{Display, Write as FmtWrite},
     fs::{self, DirEntry, FileType, Metadata, ReadDir},
     io::{stdout, BufWriter, ErrorKind, Stdout, Write},
@@ -55,7 +55,7 @@ use uucore::libc::{dev_t, major, minor};
 #[cfg(unix)]
 use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR};
 use uucore::line_ending::LineEnding;
-use uucore::quoting_style::{escape_dir_name, escape_name, QuotingStyle};
+use uucore::quoting_style::{self, QuotingStyle};
 use uucore::{
     display::Quotable,
     error::{set_exit_code, UError, UResult},
@@ -2048,7 +2048,11 @@ impl PathData {
 /// file11
 /// ```
 fn show_dir_name(path_data: &PathData, out: &mut BufWriter<Stdout>, config: &Config) {
-    let escaped_name = escape_dir_name(path_data.p_buf.as_os_str(), &config.quoting_style);
+    // FIXME: replace this with appropriate behavior for literal unprintable bytes
+    let escaped_name =
+        quoting_style::escape_dir_name(path_data.p_buf.as_os_str(), &config.quoting_style)
+            .to_string_lossy()
+            .to_string();
 
     let name = if config.hyperlink && !config.dired {
         create_hyperlink(&escaped_name, path_data)
@@ -3002,7 +3006,6 @@ use std::sync::Mutex;
 #[cfg(unix)]
 use uucore::entries;
 use uucore::fs::FileInformation;
-use uucore::quoting_style;
 
 #[cfg(unix)]
 fn cached_uid2usr(uid: u32) -> String {
@@ -3542,3 +3545,10 @@ fn calculate_padding_collection(
 
     padding_collections
 }
+
+// FIXME: replace this with appropriate behavior for literal unprintable bytes
+fn escape_name(name: &OsStr, style: &QuotingStyle) -> String {
+    quoting_style::escape_name(name, style)
+        .to_string_lossy()
+        .to_string()
+}

src/uu/wc/src/wc.rs

@@ -13,7 +13,7 @@ mod word_count;
 use std::{
     borrow::{Borrow, Cow},
     cmp::max,
-    ffi::OsString,
+    ffi::{OsStr, OsString},
     fs::{self, File},
     io::{self, Write},
     iter,
@@ -28,7 +28,7 @@ use utf8::{BufReadDecoder, BufReadDecoderError};
 use uucore::{
     error::{FromIo, UError, UResult},
     format_usage, help_about, help_usage,
-    quoting_style::{escape_name, QuotingStyle},
+    quoting_style::{self, QuotingStyle},
     shortcut_value_parser::ShortcutValueParser,
     show,
 };
@@ -259,7 +259,7 @@ impl<'a> Input<'a> {
         match self {
             Self::Path(path) => Some(match path.to_str() {
                 Some(s) if !s.contains('\n') => Cow::Borrowed(s),
-                _ => Cow::Owned(escape_name(path.as_os_str(), QS_ESCAPE)),
+                _ => Cow::Owned(escape_name_wrapper(path.as_os_str())),
             }),
             Self::Stdin(StdinKind::Explicit) => Some(Cow::Borrowed(STDIN_REPR)),
             Self::Stdin(StdinKind::Implicit) => None,
@@ -269,7 +269,7 @@ impl<'a> Input<'a> {
     /// Converts input into the form that appears in errors.
     fn path_display(&self) -> String {
         match self {
-            Self::Path(path) => escape_name(path.as_os_str(), QS_ESCAPE),
+            Self::Path(path) => escape_name_wrapper(path.as_os_str()),
             Self::Stdin(_) => String::from("standard input"),
         }
     }
@@ -361,7 +361,7 @@ impl WcError {
             Some((input, idx)) => {
                 let path = match input {
                     Input::Stdin(_) => STDIN_REPR.into(),
-                    Input::Path(path) => escape_name(path.as_os_str(), QS_ESCAPE).into(),
+                    Input::Path(path) => escape_name_wrapper(path.as_os_str()).into(),
                 };
                 Self::ZeroLengthFileNameCtx { path, idx }
             }
@@ -761,7 +761,9 @@ fn files0_iter_file<'a>(path: &Path) -> UResult<impl Iterator<Item = InputIterIt
         Err(e) => Err(e.map_err_context(|| {
             format!(
                 "cannot open {} for reading",
-                escape_name(path.as_os_str(), QS_QUOTE_ESCAPE)
+                quoting_style::escape_name(path.as_os_str(), QS_QUOTE_ESCAPE)
+                    .into_string()
+                    .expect("All escaped names with the escaping option return valid strings.")
             )
         })),
     }
@@ -793,9 +795,9 @@ fn files0_iter<'a>(
                         Ok(Input::Path(PathBuf::from(s).into()))
                     }
                 }
-                Err(e) => Err(e.map_err_context(|| {
-                    format!("{}: read error", escape_name(&err_path, QS_ESCAPE))
-                }) as Box<dyn UError>),
+                Err(e) => Err(e
+                    .map_err_context(|| format!("{}: read error", escape_name_wrapper(&err_path)))
+                    as Box<dyn UError>),
             }),
     );
     // Loop until there is an error; yield that error and then nothing else.
@@ -808,6 +810,12 @@ fn files0_iter<'a>(
     })
 }
 
+fn escape_name_wrapper(name: &OsStr) -> String {
+    quoting_style::escape_name(name, QS_ESCAPE)
+        .into_string()
+        .expect("All escaped names with the escaping option return valid strings.")
+}
+
 fn wc(inputs: &Inputs, settings: &Settings) -> UResult<()> {
     let mut total_word_count = WordCount::default();
     let mut num_inputs: usize = 0;

src/uucore/src/lib/features/format/argument.rs

@@ -112,7 +112,8 @@ fn extract_value<T: Default>(p: Result<T, ParseError<'_, T>>, input: &str) -> T
                     Default::default()
                 }
                 ParseError::PartialMatch(v, rest) => {
-                    if input.starts_with('\'') {
+                    let bytes = input.as_encoded_bytes();
+                    if !bytes.is_empty() && bytes[0] == b'\'' {
                         show_warning!(
                             "{}: character(s) following character constant have been ignored",
                             &rest,

src/uucore/src/lib/features/format/spec.rs

@@ -353,20 +353,20 @@ impl Spec {
                 writer.write_all(&parsed).map_err(FormatError::IoError)
             }
             Self::QuotedString => {
-                let s = args.get_str();
-                writer
-                    .write_all(
-                        escape_name(
-                            s.as_ref(),
-                            &QuotingStyle::Shell {
-                                escape: true,
-                                always_quote: false,
-                                show_control: false,
-                            },
-                        )
-                        .as_bytes(),
-                    )
-                    .map_err(FormatError::IoError)
+                let s = escape_name(
+                    args.get_str().as_ref(),
+                    &QuotingStyle::Shell {
+                        escape: true,
+                        always_quote: false,
+                        show_control: false,
+                    },
+                );
+                #[cfg(unix)]
+                let bytes = std::os::unix::ffi::OsStringExt::into_vec(s);
+                #[cfg(not(unix))]
+                let bytes = s.to_string_lossy().as_bytes().to_owned();
+
+                writer.write_all(&bytes).map_err(FormatError::IoError)
             }
             Self::SignedInt {
                 width,

src/uucore/src/lib/lib.rs

@@ -255,9 +255,10 @@ pub fn read_yes() -> bool {
     }
 }
 
-/// Helper function for processing delimiter values (which could be non UTF-8)
-/// It converts OsString to &[u8] for unix targets only
-/// On non-unix (i.e. Windows) it will just return an error if delimiter value is not UTF-8
+/// Converts an `OsStr` to a UTF-8 `&[u8]`.
+///
+/// This always succeeds on unix platforms,
+/// and fails on other platforms if the string can't be coerced to UTF-8.
 pub fn os_str_as_bytes(os_string: &OsStr) -> mods::error::UResult<&[u8]> {
     #[cfg(unix)]
     let bytes = os_string.as_bytes();
@@ -273,13 +274,28 @@ pub fn os_str_as_bytes(os_string: &OsStr) -> mods::error::UResult<&[u8]> {
     Ok(bytes)
 }
 
-/// Helper function for converting a slice of bytes into an &OsStr
-/// or OsString in non-unix targets.
+/// Performs a potentially lossy conversion from `OsStr` to UTF-8 bytes.
 ///
-/// It converts `&[u8]` to `Cow<OsStr>` for unix targets only.
-/// On non-unix (i.e. Windows), the conversion goes through the String type
-/// and thus undergo UTF-8 validation, making it fail if the stream contains
-/// non-UTF-8 characters.
+/// This is always lossless on unix platforms,
+/// and wraps [`OsStr::to_string_lossy`] on non-unix platforms.
+pub fn os_str_as_bytes_lossy(os_string: &OsStr) -> Cow<[u8]> {
+    #[cfg(unix)]
+    let bytes = Cow::from(os_string.as_bytes());
+
+    #[cfg(not(unix))]
+    let bytes = match os_string.to_string_lossy() {
+        Cow::Borrowed(slice) => Cow::from(slice.as_bytes()),
+        Cow::Owned(owned) => Cow::from(owned.into_bytes()),
+    };
+
+    bytes
+}
+
+/// Converts a `&[u8]` to an `&OsStr`,
+/// or parses it as UTF-8 into an [`OsString`] on non-unix platforms.
+///
+/// This always succeeds on unix platforms,
+/// and fails on other platforms if the bytes can't be parsed as UTF-8.
 pub fn os_str_from_bytes(bytes: &[u8]) -> mods::error::UResult<Cow<'_, OsStr>> {
     #[cfg(unix)]
     let os_str = Cow::Borrowed(OsStr::from_bytes(bytes));
@@ -291,9 +307,10 @@ pub fn os_str_from_bytes(bytes: &[u8]) -> mods::error::UResult<Cow<'_, OsStr>> {
     Ok(os_str)
 }
 
-/// Helper function for making an `OsString` from a byte field
-/// It converts `Vec<u8>` to `OsString` for unix targets only.
-/// On non-unix (i.e. Windows) it may fail if the bytes are not valid UTF-8
+/// Converts a `Vec<u8>` into an `OsString`, parsing as UTF-8 on non-unix platforms.
+///
+/// This always succeeds on unix platforms,
+/// and fails on other platforms if the bytes can't be parsed as UTF-8.
 pub fn os_string_from_vec(vec: Vec<u8>) -> mods::error::UResult<OsString> {
     #[cfg(unix)]
     let s = OsString::from_vec(vec);