diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 9e43d51f9..4ac8cde5a 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -364,29 +364,15 @@ jobs: test -h /tmp/usr/local/bin/sha512sum test -h /tmp/usr/local/bin/shake128sum test -h /tmp/usr/local/bin/shake256sum - - name: "`make install MULTICALL=y`" + - name: "`make install MULTICALL=y LN=ln -svf`" shell: bash run: | set -x - DESTDIR=/tmp/ make PROFILE=release MULTICALL=y install - # Check that the utils are present - test -f /tmp/usr/local/bin/coreutils - # Check that hashsum symlinks are present - test -h /tmp/usr/local/bin/b2sum - test -h /tmp/usr/local/bin/b3sum - test -h /tmp/usr/local/bin/md5sum - test -h /tmp/usr/local/bin/sha1sum - test -h /tmp/usr/local/bin/sha224sum - test -h /tmp/usr/local/bin/sha256sum - test -h /tmp/usr/local/bin/sha3-224sum - test -h /tmp/usr/local/bin/sha3-256sum - test -h /tmp/usr/local/bin/sha3-384sum - test -h /tmp/usr/local/bin/sha3-512sum - test -h /tmp/usr/local/bin/sha384sum - test -h /tmp/usr/local/bin/sha3sum - test -h /tmp/usr/local/bin/sha512sum - test -h /tmp/usr/local/bin/shake128sum - test -h /tmp/usr/local/bin/shake256sum + DESTDIR=/tmp/ make PROFILE=release MULTICALL=y LN="ln -svf" install + # Check that relative symlinks of hashsum are present + [ $(readlink /tmp/usr/local/bin/b2sum) = coreutils ] + [ $(readlink /tmp/usr/local/bin/md5sum) = coreutils ] + [ $(readlink /tmp/usr/local/bin/sha512sum) = coreutils ] - name: "`make UTILS=XXX`" shell: bash run: | @@ -483,9 +469,13 @@ jobs: run: | ## `make install` make install DESTDIR=target/size-release/ - make install MULTICALL=y DESTDIR=target/size-multi-release/ + make install MULTICALL=y LN="ln -vf" DESTDIR=target/size-multi-release/ # strip the results strip target/size*/usr/local/bin/* + - name: Test for hardlinks + shell: bash + run: | + [ $(stat -c %i target/size-multi-release/usr/local/bin/cp) = $(stat -c %i target/size-multi-release/usr/local/bin/coreutils) ] - name: Compute uutil release sizes shell: bash run: | @@ -1199,11 +1189,14 @@ jobs: test_separately: name: Separate Builds - runs-on: ${{ matrix.os }} + runs-on: ${{ matrix.job.os }} strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest, windows-latest] + job: + - { os: ubuntu-latest , features: feat_os_unix } + - { os: macos-latest , features: feat_os_macos } + - { os: windows-latest , features: feat_os_windows } steps: - uses: actions/checkout@v5 with: @@ -1213,7 +1206,8 @@ jobs: - name: build and test all programs individually shell: bash run: | - for f in $(util/show-utils.sh) + CARGO_FEATURES_OPTION='--features=${{ matrix.job.features }}' ; + for f in $(util/show-utils.sh ${CARGO_FEATURES_OPTION}) do echo "Building and testing $f" cargo test -p "uu_$f" || exit 1 @@ -1222,12 +1216,14 @@ jobs: test_all_features: name: Test all features separately needs: [ min_version, deps ] - runs-on: ${{ matrix.os }} + runs-on: ${{ matrix.job.os }} strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] - # windows-latest - https://github.com/uutils/coreutils/issues/7044 + job: + - { os: ubuntu-latest , features: feat_os_unix } + - { os: macos-latest , features: feat_os_macos } + # - { os: windows-latest , features: feat_os_windows } https://github.com/uutils/coreutils/issues/7044 steps: - uses: actions/checkout@v5 with: @@ -1237,7 +1233,8 @@ jobs: - name: build and test all features individually shell: bash run: | - for f in $(util/show-utils.sh) + CARGO_FEATURES_OPTION='--features=${{ matrix.job.features }}' ; + for f in $(util/show-utils.sh ${CARGO_FEATURES_OPTION}) do echo "Running tests with --features=$f and --no-default-features" cargo test --features=$f --no-default-features @@ -1280,6 +1277,56 @@ jobs: - name: Lint with SELinux run: lima bash -c "cd work && cargo clippy --all-targets --features 'feat_selinux' -- -D warnings" + test_selinux_stubs: + name: Build/SELinux-Stubs (Non-Linux) + needs: [ min_version, deps ] + runs-on: ${{ matrix.job.os }} + strategy: + fail-fast: false + matrix: + job: + - { os: macos-latest , features: feat_os_macos } + - { os: windows-latest , features: feat_os_windows } + + steps: + - uses: actions/checkout@v5 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - name: Build SELinux utilities as stubs + run: cargo build -p uu_chcon -p uu_runcon + - name: Verify stub binaries exist + shell: bash + run: | + if [ "${{ runner.os }}" = "Windows" ]; then + test -f target/debug/chcon.exe || exit 1 + test -f target/debug/runcon.exe || exit 1 + else + test -f target/debug/chcon || exit 1 + test -f target/debug/runcon || exit 1 + fi + - name: Verify workspace builds with stubs + run: cargo build --features ${{ matrix.job.features }} + + test_safe_traversal: + name: Safe Traversal Security Check + runs-on: ubuntu-latest + needs: [ min_version, deps ] + + steps: + - uses: actions/checkout@v5 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - name: Install strace + run: sudo apt-get update && sudo apt-get install -y strace + - name: Build utilities with safe traversal + run: cargo build --release -p uu_rm -p uu_chmod -p uu_chown -p uu_chgrp -p uu_mv -p uu_du + - name: Run safe traversal verification + run: ./util/check-safe-traversal.sh + benchmarks: name: Run benchmarks (CodSpeed) runs-on: ubuntu-latest diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 6c816ec7d..c6623a57a 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -110,6 +110,7 @@ jobs: ## Install/setup prerequisites case '${{ matrix.job.os }}' in ubuntu-*) + sudo apt-get -y update # selinux and systemd headers needed to enable all features sudo apt-get -y install libselinux1-dev libsystemd-dev ;; diff --git a/.github/workflows/l10n.yml b/.github/workflows/l10n.yml index 6b0baa3f2..f5f1871f7 100644 --- a/.github/workflows/l10n.yml +++ b/.github/workflows/l10n.yml @@ -1147,146 +1147,108 @@ jobs: run: | bash util/test_locale_regression.sh - l10n_locale_embedding_regression_test: - name: L10n/Locale Embedding Regression Test + l10n_locale_embedding_cat: + name: L10n/Locale Embedding - Cat Utility runs-on: ubuntu-latest - env: - SCCACHE_GHA_ENABLED: "true" - RUSTC_WRAPPER: "sccache" steps: - uses: actions/checkout@v5 with: persist-credentials: false - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - - name: Run sccache-cache - uses: mozilla-actions/sccache-action@v0.0.9 - - name: Install/setup prerequisites - shell: bash + with: + # Use different cache key for each build to avoid conflicts + key: cat-locale-embedding + - name: Install prerequisites + run: sudo apt-get -y update && sudo apt-get -y install libselinux1-dev build-essential + - name: Build cat with targeted locale embedding + run: UUCORE_TARGET_UTIL=cat cargo build -p uu_cat --release + - name: Verify cat locale count run: | - ## Install/setup prerequisites - sudo apt-get -y update ; sudo apt-get -y install libselinux1-dev build-essential - - name: Build binaries for locale embedding test - shell: bash - run: | - ## Build individual utilities and multicall binary for locale embedding test - echo "Building binaries with different locale embedding configurations..." - mkdir -p target - - # Build cat utility with targeted locale embedding - echo "Building cat utility with targeted locale embedding..." - echo "cat" > target/uucore_target_util.txt - cargo build -p uu_cat --release - - # Build ls utility with targeted locale embedding - echo "Building ls utility with targeted locale embedding..." - echo "ls" > target/uucore_target_util.txt - cargo build -p uu_ls --release - - # Build multicall binary (should have all locales) - echo "Building multicall binary (should have all locales)..." - echo "multicall" > target/uucore_target_util.txt - cargo build --release - - echo "✓ All binaries built successfully" - env: - RUST_BACKTRACE: "1" - - - name: Analyze embedded locale files - shell: bash - run: | - ## Extract and analyze .ftl files embedded in each binary - echo "=== Embedded Locale File Analysis ===" - - # Analyze cat binary - echo "--- cat binary embedded .ftl files ---" - cat_ftl_files=$(strings target/release/cat | grep -o "[a-z_][a-z_]*/en-US\.ftl" | sort | uniq) - cat_locales=$(echo "$cat_ftl_files" | wc -l) - if [ -n "$cat_ftl_files" ]; then - echo "$cat_ftl_files" - else - echo "(no locale keys found)" + locale_file=$(find target/release/build -name "embedded_locales.rs" | head -1) + if [ -z "$locale_file" ]; then + echo "ERROR: Could not find embedded_locales.rs" + exit 1 fi - echo "Total: $cat_locales files" - echo - - # Analyze ls binary - echo "--- ls binary embedded .ftl files ---" - ls_ftl_files=$(strings target/release/ls | grep -o "[a-z_][a-z_]*/en-US\.ftl" | sort | uniq) - ls_locales=$(echo "$ls_ftl_files" | wc -l) - if [ -n "$ls_ftl_files" ]; then - echo "$ls_ftl_files" + locale_count=$(grep -c '/en-US\.ftl' "$locale_file") + echo "Cat binary has $locale_count embedded locales" + if [ "$locale_count" -le 5 ]; then + echo "✓ SUCCESS: Cat uses targeted locale embedding ($locale_count files)" else - echo "(no locale keys found)" - fi - echo "Total: $ls_locales files" - echo - - # Analyze multicall binary - echo "--- multicall binary embedded .ftl files (first 10) ---" - multi_ftl_files=$(strings target/release/coreutils | grep -o "[a-z_][a-z_]*/en-US\.ftl" | sort | uniq) - multi_locales=$(echo "$multi_ftl_files" | wc -l) - if [ -n "$multi_ftl_files" ]; then - echo "$multi_ftl_files" | head -10 - echo "... (showing first 10 of $multi_locales total files)" - else - echo "(no locale keys found)" - fi - echo - - # Store counts for validation step - echo "cat_locales=$cat_locales" >> $GITHUB_ENV - echo "ls_locales=$ls_locales" >> $GITHUB_ENV - echo "multi_locales=$multi_locales" >> $GITHUB_ENV - - - name: Validate cat binary locale embedding - shell: bash - run: | - ## Validate that cat binary only embeds its own locale files - echo "Validating cat binary locale embedding..." - if [ "$cat_locales" -le 5 ]; then - echo "✓ SUCCESS: cat binary uses targeted locale embedding ($cat_locales files)" - else - echo "✗ FAILURE: cat binary has too many embedded locale files ($cat_locales). Expected ≤ 5." - echo "This indicates LOCALE EMBEDDING REGRESSION - all locales are being embedded instead of just the target utility's locale." - echo "The optimization is not working correctly!" + echo "✗ FAILURE: Cat has too many locale files ($locale_count). Expected ≤ 5" exit 1 fi - - name: Validate ls binary locale embedding - shell: bash + l10n_locale_embedding_ls: + name: L10n/Locale Embedding - Ls Utility + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + with: + # Use different cache key for each build to avoid conflicts + key: ls-locale-embedding + - name: Install prerequisites + run: sudo apt-get -y update && sudo apt-get -y install libselinux1-dev build-essential + - name: Build ls with targeted locale embedding + run: UUCORE_TARGET_UTIL=ls cargo build -p uu_ls --release + - name: Verify ls locale count run: | - ## Validate that ls binary only embeds its own locale files - echo "Validating ls binary locale embedding..." - if [ "$ls_locales" -le 5 ]; then - echo "✓ SUCCESS: ls binary uses targeted locale embedding ($ls_locales files)" + locale_file=$(find target/release/build -name "embedded_locales.rs" | head -1) + if [ -z "$locale_file" ]; then + echo "ERROR: Could not find embedded_locales.rs" + exit 1 + fi + locale_count=$(grep -c '/en-US\.ftl' "$locale_file") + echo "Ls binary has $locale_count embedded locales" + if [ "$locale_count" -le 5 ]; then + echo "✓ SUCCESS: Ls uses targeted locale embedding ($locale_count files)" else - echo "✗ FAILURE: ls binary has too many embedded locale files ($ls_locales). Expected ≤ 5." - echo "This indicates LOCALE EMBEDDING REGRESSION - all locales are being embedded instead of just the target utility's locale." - echo "The optimization is not working correctly!" + echo "✗ FAILURE: Ls has too many locale files ($locale_count). Expected ≤ 5" exit 1 fi - - name: Validate multicall binary locale embedding - shell: bash + l10n_locale_embedding_multicall: + name: L10n/Locale Embedding - Multicall Binary + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + with: + # Use different cache key for each build to avoid conflicts + key: multicall-locale-embedding + - name: Install prerequisites + run: sudo apt-get -y update && sudo apt-get -y install libselinux1-dev build-essential + - name: Build multicall binary with all locales + run: cargo build --release + - name: Verify multicall locale count run: | - ## Validate that multicall binary embeds all utility locale files - echo "Validating multicall binary locale embedding..." - if [ "$multi_locales" -ge 80 ]; then - echo "✓ SUCCESS: multicall binary has all locales ($multi_locales files)" + locale_file=$(find target/release/build -name "embedded_locales.rs" | head -1) + if [ -z "$locale_file" ]; then + echo "ERROR: Could not find embedded_locales.rs" + exit 1 + fi + locale_count=$(grep -c '/en-US\.ftl' "$locale_file") + echo "Multicall binary has $locale_count embedded locales" + echo "First 10 locales:" + grep -o '[a-z_][a-z_0-9]*/en-US\.ftl' "$locale_file" | head -10 + if [ "$locale_count" -ge 80 ]; then + echo "✓ SUCCESS: Multicall has all locales ($locale_count files)" else - echo "✗ FAILURE: multicall binary has too few embedded locale files ($multi_locales). Expected ≥ 80." - echo "This indicates the multicall binary is not getting all required locales." + echo "✗ FAILURE: Multicall has too few locale files ($locale_count). Expected ≥ 80" exit 1 fi - - name: Finalize locale embedding tests - shell: bash - run: | - ## Clean up and report overall test results - rm -f test.txt target/uucore_target_util.txt - echo "✓ All locale embedding regression tests passed" - echo "Summary:" - echo " - cat binary: $cat_locales locale files (targeted embedding)" - echo " - ls binary: $ls_locales locale files (targeted embedding)" - echo " - multicall binary: $multi_locales locale files (full embedding)" + l10n_locale_embedding_regression_test: + name: L10n/Locale Embedding Regression Test + runs-on: ubuntu-latest + needs: [l10n_locale_embedding_cat, l10n_locale_embedding_ls, l10n_locale_embedding_multicall] + steps: + - name: All locale embedding tests passed + run: echo "✓ All locale embedding tests passed successfully" diff --git a/.vscode/cSpell.json b/.vscode/cSpell.json index 5d3e3524b..1d360d990 100644 --- a/.vscode/cSpell.json +++ b/.vscode/cSpell.json @@ -34,6 +34,7 @@ "docs/src/release-notes/**", "src/uu/*/benches/*.rs", "src/uucore/src/lib/features/benchmark.rs", + "util/check-safe-traversal.sh", ], "enableGlobDot": true, diff --git a/Cargo.lock b/Cargo.lock index 06cd0182c..3811556f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -876,9 +876,9 @@ dependencies = [ [[package]] name = "ctor" -version = "0.5.0" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67773048316103656a637612c4a62477603b777d91d9c62ff2290f9cde178fdb" +checksum = "59c9b8bdf64ee849747c1b12eb861d21aa47fa161564f48332f1afe2373bf899" dependencies = [ "ctor-proc-macro", "dtor", @@ -886,9 +886,9 @@ dependencies = [ [[package]] name = "ctor-proc-macro" -version = "0.0.6" +version = "0.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2931af7e13dc045d8e9d26afccc6fa115d64e115c9c84b1166288b46f6782c2" +checksum = "52560adf09603e58c9a7ee1fe1dcb95a16927b17c127f0ac02d6e768a0e25bc1" [[package]] name = "ctrlc" @@ -3299,8 +3299,10 @@ version = "0.2.2" dependencies = [ "bstr", "clap", + "codspeed-divan-compat", "fluent", "memchr", + "tempfile", "uucore", ] @@ -3501,7 +3503,9 @@ name = "uu_hashsum" version = "0.2.2" dependencies = [ "clap", + "codspeed-divan-compat", "fluent", + "tempfile", "uucore", ] @@ -3688,10 +3692,12 @@ name = "uu_mv" version = "0.2.2" dependencies = [ "clap", + "codspeed-divan-compat", "fluent", "fs_extra", "indicatif", "libc", + "tempfile", "thiserror 2.0.16", "uucore", "windows-sys 0.61.0", @@ -3865,8 +3871,10 @@ name = "uu_rm" version = "0.2.2" dependencies = [ "clap", + "codspeed-divan-compat", "fluent", "libc", + "tempfile", "thiserror 2.0.16", "uucore", "windows-sys 0.61.0", @@ -3900,9 +3908,11 @@ version = "0.2.2" dependencies = [ "bigdecimal", "clap", + "codspeed-divan-compat", "fluent", "num-bigint", "num-traits", + "tempfile", "thiserror 2.0.16", "uucore", ] @@ -3967,8 +3977,10 @@ name = "uu_split" version = "0.2.2" dependencies = [ "clap", + "codspeed-divan-compat", "fluent", "memchr", + "tempfile", "thiserror 2.0.16", "uucore", ] @@ -4174,7 +4186,9 @@ name = "uu_unexpand" version = "0.2.2" dependencies = [ "clap", + "codspeed-divan-compat", "fluent", + "tempfile", "thiserror 2.0.16", "unicode-width 0.2.1", "uucore", @@ -4943,9 +4957,9 @@ dependencies = [ [[package]] name = "zip" -version = "5.1.1" +version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f852905151ac8d4d06fdca66520a661c09730a74c6d4e2b0f27b436b382e532" +checksum = "eb2a05c7c36fde6c09b08576c9f7fb4cda705990f73b58fe011abf7dfb24168b" dependencies = [ "arbitrary", "crc32fast", diff --git a/Cargo.toml b/Cargo.toml index 0306dde6a..7a0c69e33 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ # coreutils (uutils) # * see the repository LICENSE, README, and CONTRIBUTING files for more information -# spell-checker:ignore (libs) bigdecimal datetime serde bincode gethostid kqueue libselinux mangen memmap uuhelp startswith constness expl unnested logind +# spell-checker:ignore (libs) bigdecimal datetime serde bincode gethostid kqueue libselinux mangen memmap uuhelp startswith constness expl unnested logind cfgs [package] name = "coreutils" @@ -314,7 +314,7 @@ clap_complete = "4.4" clap_mangen = "0.2" compare = "0.1.0" crossterm = "0.29.0" -ctor = "0.5.0" +ctor = "0.6.0" ctrlc = { version = "3.4.7", features = ["termination"] } divan = { package = "codspeed-divan-compat", version = "*" } dns-lookup = { version = "3.0.0" } @@ -381,7 +381,7 @@ walkdir = "2.5" winapi-util = "0.1.8" windows-sys = { version = "0.61.0", default-features = false } xattr = "1.3.1" -zip = { version = "5.0.0", default-features = false, features = ["deflate"] } +zip = { version = "6.0.0", default-features = false, features = ["deflate"] } hex = "0.4.3" md-5 = "0.10.6" @@ -623,7 +623,6 @@ missing_panics_doc = "allow" # TODO remove when https://github.com/rust-lang/rust-clippy/issues/13774 is fixed large_stack_arrays = "allow" -use_self = "warn" needless_pass_by_value = "warn" semicolon_if_nothing_returned = "warn" single_char_pattern = "warn" @@ -639,6 +638,9 @@ pedantic = { level = "deny", priority = -1 } # Eventually the clippy settings from the `[lints]` section should be moved here. # In order to use these, all crates have `[lints] workspace = true` section. [workspace.lints.rust] +# Allow "fuzzing" as a "cfg" condition name +# https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(fuzzing)'] } #unused_qualifications = "warn" // TODO: fix warnings in uucore, then re-enable this lint [workspace.lints.clippy] @@ -653,6 +655,7 @@ pedantic = { level = "deny", priority = -1 } all = { level = "warn", priority = -1 } cargo = { level = "warn", priority = -1 } pedantic = { level = "warn", priority = -1 } +use_self = "warn" # nursery lint cargo_common_metadata = "allow" # 3240 multiple_crate_versions = "allow" # 2882 missing_errors_doc = "allow" # 1572 diff --git a/GNUmakefile b/GNUmakefile index 83a169906..41ba59349 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -69,6 +69,13 @@ TOYBOX_SRC := $(TOYBOX_ROOT)/toybox-$(TOYBOX_VER) #------------------------------------------------------------------------ OS ?= $(shell uname -s) +# Windows does not allow symlink by default. +# Allow to override LN for AppArmor. +ifeq ($(OS),Windows_NT) + LN ?= ln -f +endif +LN ?= ln -sf + ifdef SELINUX_ENABLED override SELINUX_ENABLED := 0 # Now check if we should enable it (only on non-Windows) @@ -482,18 +489,18 @@ endif ifeq (${MULTICALL}, y) $(INSTALL) -m 755 $(BUILDDIR)/coreutils $(INSTALLDIR_BIN)/$(PROG_PREFIX)coreutils $(foreach prog, $(filter-out coreutils, $(INSTALLEES)), \ - cd $(INSTALLDIR_BIN) && ln -fs $(PROG_PREFIX)coreutils $(PROG_PREFIX)$(prog) $(newline) \ + cd $(INSTALLDIR_BIN) && $(LN) $(PROG_PREFIX)coreutils $(PROG_PREFIX)$(prog) $(newline) \ ) $(foreach prog, $(HASHSUM_PROGS), \ - cd $(INSTALLDIR_BIN) && ln -fs $(PROG_PREFIX)coreutils $(PROG_PREFIX)$(prog) $(newline) \ + cd $(INSTALLDIR_BIN) && $(LN) $(PROG_PREFIX)coreutils $(PROG_PREFIX)$(prog) $(newline) \ ) - $(if $(findstring test,$(INSTALLEES)), cd $(INSTALLDIR_BIN) && ln -fs $(PROG_PREFIX)coreutils $(PROG_PREFIX)[) + $(if $(findstring test,$(INSTALLEES)), cd $(INSTALLDIR_BIN) && $(LN) $(PROG_PREFIX)coreutils $(PROG_PREFIX)[) else $(foreach prog, $(INSTALLEES), \ $(INSTALL) -m 755 $(BUILDDIR)/$(prog) $(INSTALLDIR_BIN)/$(PROG_PREFIX)$(prog) $(newline) \ ) $(foreach prog, $(HASHSUM_PROGS), \ - cd $(INSTALLDIR_BIN) && ln -fs $(PROG_PREFIX)hashsum $(PROG_PREFIX)$(prog) $(newline) \ + cd $(INSTALLDIR_BIN) && $(LN) $(PROG_PREFIX)hashsum $(PROG_PREFIX)$(prog) $(newline) \ ) $(if $(findstring test,$(INSTALLEES)), $(INSTALL) -m 755 $(BUILDDIR)/test $(INSTALLDIR_BIN)/$(PROG_PREFIX)[) endif diff --git a/README.md b/README.md index b273ff3d3..087bf8830 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,11 @@ So, to install completions for `ls` on `bash` to cargo run completion ls bash > /usr/local/share/bash-completion/completions/ls ``` +Completion for prefixed `cp` with `uu-` on `zsh` is generated by +```shell +env PROG_PREFIX=uu- cargo run completion cp zsh +``` + ### Manually install manpages To generate manpages, the syntax is: diff --git a/deny.toml b/deny.toml index e11ff5f2d..9c690a4ec 100644 --- a/deny.toml +++ b/deny.toml @@ -107,6 +107,8 @@ skip = [ { name = "rand_core", version = "0.6.4" }, # utmp-classic { name = "zerocopy", version = "0.7.35" }, + # zerocopy + { name = "zerocopy-derive", version = "0.7.35" }, # divans/codspeed tooling { name = "nix", version = "0.29.0" }, ] diff --git a/src/uu/base32/locales/en-US.ftl b/src/uu/base32/locales/en-US.ftl index c083d8928..925e5c70a 100644 --- a/src/uu/base32/locales/en-US.ftl +++ b/src/uu/base32/locales/en-US.ftl @@ -42,6 +42,7 @@ basenc-help-base2msbf = bit string with most significant bit (msb) first basenc-help-z85 = ascii85-like encoding; when encoding, input length must be a multiple of 4; when decoding, input length must be a multiple of 5 +basenc-help-base58 = visually unambiguous base58 encoding # Error messages basenc-error-missing-encoding-type = missing encoding type diff --git a/src/uu/base32/locales/fr-FR.ftl b/src/uu/base32/locales/fr-FR.ftl index 98c554bfb..c5ca10b71 100644 --- a/src/uu/base32/locales/fr-FR.ftl +++ b/src/uu/base32/locales/fr-FR.ftl @@ -37,6 +37,7 @@ basenc-help-base2msbf = chaîne de bits avec le bit de poids fort (msb) en premi basenc-help-z85 = encodage de type ascii85 ; lors de l'encodage, la longueur d'entrée doit être un multiple de 4 ; lors du décodage, la longueur d'entrée doit être un multiple de 5 +basenc-help-base58 = encodage base58 visuellement non ambigu # Messages d'erreur basenc-error-missing-encoding-type = type d'encodage manquant diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index 7e874f4c8..fe13e46cc 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -12,8 +12,8 @@ use std::io::{self, ErrorKind, Read, Seek}; use std::path::{Path, PathBuf}; use uucore::display::Quotable; use uucore::encoding::{ - BASE2LSBF, BASE2MSBF, Base64SimdWrapper, EncodingWrapper, Format, SupportsFastDecodeAndEncode, - Z85Wrapper, + BASE2LSBF, BASE2MSBF, Base58Wrapper, Base64SimdWrapper, EncodingWrapper, Format, + SupportsFastDecodeAndEncode, Z85Wrapper, for_base_common::{BASE32, BASE32HEX, BASE64URL, HEXUPPER_PERMISSIVE}, }; use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; @@ -285,6 +285,7 @@ pub fn get_supports_fast_decode_and_encode( b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789=_-", )), Format::Z85 => Box::from(Z85Wrapper {}), + Format::Base58 => Box::from(Base58Wrapper {}), } } diff --git a/src/uu/basenc/src/basenc.rs b/src/uu/basenc/src/basenc.rs index 649883227..42e4ef295 100644 --- a/src/uu/basenc/src/basenc.rs +++ b/src/uu/basenc/src/basenc.rs @@ -39,6 +39,7 @@ fn get_encodings() -> Vec<(&'static str, Format, String)> { translate!("basenc-help-base2msbf"), ), ("z85", Format::Z85, translate!("basenc-help-z85")), + ("base58", Format::Base58, translate!("basenc-help-base58")), ] } diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 6d19c0572..ff34ca25b 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -63,7 +63,7 @@ impl LineNumber { buf[print_start..].copy_from_slice(init_str.as_bytes()); - LineNumber { + Self { buf, print_start, num_start, diff --git a/src/uu/chcon/src/main.rs b/src/uu/chcon/src/main.rs index d1354d840..c143ebf88 100644 --- a/src/uu/chcon/src/main.rs +++ b/src/uu/chcon/src/main.rs @@ -1,2 +1,12 @@ -#![cfg(target_os = "linux")] +// On non-Linux targets, provide a stub main to keep the binary target present +// and the workspace buildable. Using item-level cfg avoids excluding the crate +// entirely (via #![cfg(...)]), which can break tooling and cross builds that +// expect this binary to exist even when it's a no-op off Linux. +#[cfg(target_os = "linux")] uucore::bin!(uu_chcon); + +#[cfg(not(target_os = "linux"))] +fn main() { + eprintln!("chcon: SELinux is not supported on this platform"); + std::process::exit(1); +} diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index 2eb872bfb..a791b6987 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -41,8 +41,8 @@ enum FileNumber { impl FileNumber { fn as_str(&self) -> &'static str { match self { - FileNumber::One => "1", - FileNumber::Two => "2", + Self::One => "1", + Self::Two => "2", } } } diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index a4901903d..a5c7e76c1 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -9,8 +9,9 @@ #[cfg(windows)] use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::convert::identity; use std::env; -use std::fs; +use std::fs::{self, exists}; use std::io; use std::path::{Path, PathBuf, StripPrefixError}; @@ -20,10 +21,9 @@ use uucore::error::UIoError; use uucore::fs::{ FileInformation, MissingHandling, ResolveMode, canonicalize, path_ends_with_terminator, }; -use uucore::translate; - use uucore::show; use uucore::show_error; +use uucore::translate; use uucore::uio_error; use walkdir::{DirEntry, WalkDir}; @@ -194,15 +194,23 @@ impl Entry { get_local_to_root_parent(&source_absolute, context.root_parent.as_deref())?; if no_target_dir { let source_is_dir = source.is_dir(); - if path_ends_with_terminator(context.target) && source_is_dir { + if path_ends_with_terminator(context.target) + && source_is_dir + && !exists(context.target).is_ok_and(identity) + { if let Err(e) = fs::create_dir_all(context.target) { eprintln!( "{}", translate!("cp-error-failed-to-create-directory", "error" => e) ); } - } else { - descendant = descendant.strip_prefix(context.root)?.to_path_buf(); + } else if let Some(stripped) = context + .root + .components() + .next_back() + .and_then(|stripped| descendant.strip_prefix(stripped).ok()) + { + descendant = stripped.to_path_buf(); } } else if context.root == Path::new(".") && context.target.is_dir() { // Special case: when copying current directory (.) to an existing directory, diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 1754fbb0a..83f93fcef 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -175,11 +175,11 @@ impl Default for ReflinkMode { fn default() -> Self { #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] { - ReflinkMode::Auto + Self::Auto } #[cfg(not(any(target_os = "linux", target_os = "android", target_os = "macos")))] { - ReflinkMode::Never + Self::Never } } } diff --git a/src/uu/cut/Cargo.toml b/src/uu/cut/Cargo.toml index 360ec1fee..0133180f0 100644 --- a/src/uu/cut/Cargo.toml +++ b/src/uu/cut/Cargo.toml @@ -24,6 +24,15 @@ memchr = { workspace = true } bstr = { workspace = true } fluent = { workspace = true } +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } + [[bin]] name = "cut" path = "src/main.rs" + +[[bench]] +name = "cut_bench" +harness = false diff --git a/src/uu/cut/benches/cut_bench.rs b/src/uu/cut/benches/cut_bench.rs new file mode 100644 index 000000000..997235f88 --- /dev/null +++ b/src/uu/cut/benches/cut_bench.rs @@ -0,0 +1,76 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use uu_cut::uumain; +use uucore::benchmark::{run_util_function, setup_test_file, text_data}; + +/// Benchmark cutting specific byte ranges +#[divan::bench] +fn cut_bytes(bencher: Bencher) { + let data = text_data::generate_by_lines(100_000, 80); + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["-b", "1-20", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark cutting specific character ranges +#[divan::bench] +fn cut_characters(bencher: Bencher) { + let data = text_data::generate_mixed_data(100_000); + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["-c", "5-30", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark cutting fields with tab delimiter +#[divan::bench] +fn cut_fields_tab(bencher: Bencher) { + let mut data = Vec::new(); + for i in 0..100_000 { + let line = format!("field1\tfield2_{i}\tfield3\tfield4\tfield5\n"); + data.extend_from_slice(line.as_bytes()); + } + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["-f", "2,4", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark cutting fields with custom delimiter +#[divan::bench] +fn cut_fields_custom_delim(bencher: Bencher) { + let mut data = Vec::new(); + for i in 0..100_000 { + let line = format!("apple,banana_{i},cherry,date,elderberry\n"); + data.extend_from_slice(line.as_bytes()); + } + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["-d", ",", "-f", "1,3,5", file_path.to_str().unwrap()], + )); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 6df95f7be..f9bbd9d03 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -202,9 +202,9 @@ struct Cell { impl Cell { /// Create a cell, knowing that s contains only 1-length chars - fn from_ascii_string>(s: T) -> Cell { + fn from_ascii_string>(s: T) -> Self { let s = s.as_ref(); - Cell { + Self { bytes: s.as_bytes().into(), width: s.len(), } @@ -212,17 +212,17 @@ impl Cell { /// Create a cell from an unknown origin string that may contain /// wide characters. - fn from_string>(s: T) -> Cell { + fn from_string>(s: T) -> Self { let s = s.as_ref(); - Cell { + Self { bytes: s.as_bytes().into(), width: UnicodeWidthStr::width(s), } } /// Create a cell from an `OsString` - fn from_os_string(os: &OsString) -> Cell { - Cell { + fn from_os_string(os: &OsString) -> Self { + Self { bytes: uucore::os_str_as_bytes(os).unwrap().to_vec(), width: UnicodeWidthStr::width(os.to_string_lossy().as_ref()), } diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index c41f6318f..fbd233105 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -72,7 +72,7 @@ pub enum EnvError { impl From for EnvError { fn from(value: string_parser::Error) -> Self { - EnvError::EnvInternalError(value.peek_position, value) + Self::EnvInternalError(value.peek_position, value) } } diff --git a/src/uu/expand/src/expand.rs b/src/uu/expand/src/expand.rs index 0891a2de4..f6289a573 100644 --- a/src/uu/expand/src/expand.rs +++ b/src/uu/expand/src/expand.rs @@ -358,6 +358,15 @@ fn expand_line( ) -> std::io::Result<()> { use self::CharType::{Backspace, Other, Tab}; + // Fast path: if there are no tabs, backspaces, and (in UTF-8 mode or no carriage returns), + // we can write the buffer directly without character-by-character processing + if !buf.contains(&b'\t') && !buf.contains(&b'\x08') && (options.uflag || !buf.contains(&b'\r')) + { + output.write_all(buf)?; + buf.truncate(0); + return Ok(()); + } + let mut col = 0; let mut byte = 0; let mut init = true; @@ -435,7 +444,6 @@ fn expand_line( byte += nbytes; // advance the pointer } - output.flush()?; buf.truncate(0); // clear the buffer Ok(()) @@ -471,6 +479,10 @@ fn expand(options: &Options) -> UResult<()> { } } } + // Flush once at the end + output + .flush() + .map_err_context(|| translate!("expand-error-failed-to-write-output"))?; Ok(()) } diff --git a/src/uu/hashsum/Cargo.toml b/src/uu/hashsum/Cargo.toml index dbc7ceb9e..00eb152ed 100644 --- a/src/uu/hashsum/Cargo.toml +++ b/src/uu/hashsum/Cargo.toml @@ -25,3 +25,12 @@ fluent = { workspace = true } [[bin]] name = "hashsum" path = "src/main.rs" + +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } + +[[bench]] +name = "hashsum_bench" +harness = false diff --git a/src/uu/hashsum/benches/hashsum_bench.rs b/src/uu/hashsum/benches/hashsum_bench.rs new file mode 100644 index 000000000..27572c560 --- /dev/null +++ b/src/uu/hashsum/benches/hashsum_bench.rs @@ -0,0 +1,138 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use std::io::Write; +use tempfile::NamedTempFile; +use uu_hashsum::uumain; +use uucore::benchmark::{run_util_function, setup_test_file, text_data}; + +/// Benchmark MD5 hashing +#[divan::bench] +fn hashsum_md5(bencher: Bencher) { + let data = text_data::generate_by_size(10, 80); + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["--md5", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark SHA1 hashing +#[divan::bench] +fn hashsum_sha1(bencher: Bencher) { + let data = text_data::generate_by_size(10, 80); + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["--sha1", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark SHA256 hashing +#[divan::bench] +fn hashsum_sha256(bencher: Bencher) { + let data = text_data::generate_by_size(10, 80); + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["--sha256", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark SHA512 hashing +#[divan::bench] +fn hashsum_sha512(bencher: Bencher) { + let data = text_data::generate_by_size(10, 80); + let file_path = setup_test_file(&data); + + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["--sha512", file_path.to_str().unwrap()], + )); + }); +} + +/// Benchmark MD5 checksum verification +#[divan::bench] +fn hashsum_md5_check(bencher: Bencher) { + bencher + .with_inputs(|| { + // Create test file + let data = text_data::generate_by_size(10, 80); + let test_file = setup_test_file(&data); + + // Create checksum file - keep it alive by returning it + let checksum_file = NamedTempFile::new().unwrap(); + let checksum_path = checksum_file.path().to_str().unwrap().to_string(); + + // Write checksum content + { + let mut file = std::fs::File::create(&checksum_path).unwrap(); + writeln!( + file, + "d41d8cd98f00b204e9800998ecf8427e {}", + test_file.to_str().unwrap() + ) + .unwrap(); + } + + (checksum_file, checksum_path) + }) + .bench_values(|(_checksum_file, checksum_path)| { + black_box(run_util_function( + uumain, + &["--md5", "--check", &checksum_path], + )); + }); +} + +/// Benchmark SHA256 checksum verification +#[divan::bench] +fn hashsum_sha256_check(bencher: Bencher) { + bencher + .with_inputs(|| { + // Create test file + let data = text_data::generate_by_size(10, 80); + let test_file = setup_test_file(&data); + + // Create checksum file - keep it alive by returning it + let checksum_file = NamedTempFile::new().unwrap(); + let checksum_path = checksum_file.path().to_str().unwrap().to_string(); + + // Write checksum content + { + let mut file = std::fs::File::create(&checksum_path).unwrap(); + writeln!( + file, + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 {}", + test_file.to_str().unwrap() + ) + .unwrap(); + } + + (checksum_file, checksum_path) + }) + .bench_values(|(_checksum_file, checksum_path)| { + black_box(run_util_function( + uumain, + &["--sha256", "--check", &checksum_path], + )); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/head/src/take.rs b/src/uu/head/src/take.rs index 57a7e887f..6f05b77e5 100644 --- a/src/uu/head/src/take.rs +++ b/src/uu/head/src/take.rs @@ -16,7 +16,7 @@ struct TakeAllBuffer { impl TakeAllBuffer { fn new() -> Self { - TakeAllBuffer { + Self { buffer: vec![], start_index: 0, } @@ -151,7 +151,7 @@ struct BytesAndLines { impl TakeAllLinesBuffer { fn new() -> Self { - TakeAllLinesBuffer { + Self { inner: TakeAllBuffer::new(), terminated_lines: 0, partial_line: false, diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index ba38236fa..a3fde8f4a 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -410,7 +410,17 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { } OverwriteMode::Force => { if !dst.is_symlink() && paths_refer_to_same_file(src, dst, true) { - return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into()); + // Even in force overwrite mode, verify we are not targeting the same entry and return a SameFile error if so + let same_entry = match ( + canonicalize(src, MissingHandling::Missing, ResolveMode::Physical), + canonicalize(dst, MissingHandling::Missing, ResolveMode::Physical), + ) { + (Ok(src), Ok(dst)) => src == dst, + _ => true, + }; + if same_entry { + return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into()); + } } if fs::remove_file(dst).is_ok() {} // In case of error, don't do anything diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 07642a0e5..97e87585b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1085,11 +1085,11 @@ impl Config { time_format_older, context, selinux_supported: { - #[cfg(feature = "selinux")] + #[cfg(all(feature = "selinux", target_os = "linux"))] { uucore::selinux::is_selinux_enabled() } - #[cfg(not(feature = "selinux"))] + #[cfg(not(all(feature = "selinux", target_os = "linux")))] { false } @@ -3309,7 +3309,7 @@ fn get_security_context<'a>( } if config.selinux_supported { - #[cfg(feature = "selinux")] + #[cfg(all(feature = "selinux", target_os = "linux"))] { match selinux::SecurityContext::of_path(path, must_dereference, false) { Err(_r) => { diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index 9c172db9c..796a1469f 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -40,7 +40,7 @@ enum MoreError { impl std::fmt::Display for MoreError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - MoreError::IsDirectory(path) => { + Self::IsDirectory(path) => { write!( f, "{}", @@ -50,7 +50,7 @@ impl std::fmt::Display for MoreError { ) ) } - MoreError::CannotOpenNoSuchFile(path) => { + Self::CannotOpenNoSuchFile(path) => { write!( f, "{}", @@ -60,7 +60,7 @@ impl std::fmt::Display for MoreError { ) ) } - MoreError::CannotOpenIOError(path, error) => { + Self::CannotOpenIOError(path, error) => { write!( f, "{}", @@ -71,7 +71,7 @@ impl std::fmt::Display for MoreError { ) ) } - MoreError::BadUsage => { + Self::BadUsage => { write!(f, "{}", translate!("more-error-bad-usage")) } } @@ -325,15 +325,15 @@ enum InputType { impl InputType { fn read_line(&mut self, buf: &mut String) -> std::io::Result { match self { - InputType::File(reader) => reader.read_line(buf), - InputType::Stdin(stdin) => stdin.read_line(buf), + Self::File(reader) => reader.read_line(buf), + Self::Stdin(stdin) => stdin.read_line(buf), } } fn len(&self) -> std::io::Result> { let len = match self { - InputType::File(reader) => Some(reader.get_ref().metadata()?.len()), - InputType::Stdin(_) => None, + Self::File(reader) => Some(reader.get_ref().metadata()?.len()), + Self::Stdin(_) => None, }; Ok(len) } @@ -907,7 +907,7 @@ mod tests { type Target = Vec; fn deref(&self) -> &Vec { match self { - OutputType::Test(buf) => buf, + Self::Test(buf) => buf, _ => unreachable!(), } } @@ -916,7 +916,7 @@ mod tests { impl DerefMut for OutputType { fn deref_mut(&mut self) -> &mut Vec { match self { - OutputType::Test(buf) => buf, + Self::Test(buf) => buf, _ => unreachable!(), } } diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index 329bb78ba..0ed038fe6 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -47,3 +47,12 @@ selinux = ["uucore/selinux"] [[bin]] name = "mv" path = "src/main.rs" + +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } + +[[bench]] +name = "mv_bench" +harness = false diff --git a/src/uu/mv/benches/mv_bench.rs b/src/uu/mv/benches/mv_bench.rs new file mode 100644 index 000000000..80c5500fb --- /dev/null +++ b/src/uu/mv/benches/mv_bench.rs @@ -0,0 +1,120 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use tempfile::TempDir; +use uu_mv::uumain; +use uucore::benchmark::{fs_tree, run_util_function}; + +/// Benchmark moving a single file (repeated to reach 100ms) +#[divan::bench] +fn mv_single_file(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + fs_tree::create_wide_tree(temp_dir.path(), 1000, 0); + let files: Vec<(String, String)> = (0..1000) + .map(|i| { + let src = temp_dir.path().join(format!("f{i}")); + let dst = temp_dir.path().join(format!("moved_{i}")); + ( + src.to_str().unwrap().to_string(), + dst.to_str().unwrap().to_string(), + ) + }) + .collect(); + (temp_dir, files) + }) + .bench_values(|(temp_dir, files)| { + for (src, dst) in &files { + black_box(run_util_function(uumain, &[src, dst])); + } + drop(temp_dir); + }); +} + +/// Benchmark moving multiple files to directory +#[divan::bench] +fn mv_multiple_to_dir(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + fs_tree::create_wide_tree(temp_dir.path(), 1000, 0); + let dest_dir = temp_dir.path().join("dest"); + std::fs::create_dir(&dest_dir).unwrap(); + + let mut args: Vec = (0..1000) + .map(|i| { + temp_dir + .path() + .join(format!("f{i}")) + .to_str() + .unwrap() + .to_string() + }) + .collect(); + args.push(dest_dir.to_str().unwrap().to_string()); + (temp_dir, args) + }) + .bench_values(|(temp_dir, args)| { + let arg_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + black_box(run_util_function(uumain, &arg_refs)); + drop(temp_dir); + }); +} + +/// Benchmark moving directory recursively +#[divan::bench] +fn mv_directory(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + let src_dir = temp_dir.path().join("src_tree"); + std::fs::create_dir(&src_dir).unwrap(); + // Increase tree size for longer benchmark + fs_tree::create_balanced_tree(&src_dir, 5, 5, 10); + let dst_dir = temp_dir.path().join("dest_tree"); + ( + temp_dir, + src_dir.to_str().unwrap().to_string(), + dst_dir.to_str().unwrap().to_string(), + ) + }) + .bench_values(|(temp_dir, src, dst)| { + black_box(run_util_function(uumain, &[&src, &dst])); + drop(temp_dir); + }); +} + +/// Benchmark force overwrite +#[divan::bench] +fn mv_force_overwrite(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + fs_tree::create_wide_tree(temp_dir.path(), 2000, 0); + let files: Vec<(String, String)> = (0..1000) + .map(|i| { + let src = temp_dir.path().join(format!("f{i}")); + let dst = temp_dir.path().join(format!("f{}", i + 1000)); + ( + src.to_str().unwrap().to_string(), + dst.to_str().unwrap().to_string(), + ) + }) + .collect(); + (temp_dir, files) + }) + .bench_values(|(temp_dir, files)| { + for (src, dst) in &files { + black_box(run_util_function(uumain, &["-f", src, dst])); + } + drop(temp_dir); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/mv/src/hardlink.rs b/src/uu/mv/src/hardlink.rs index d3c4350c0..63bb152fd 100644 --- a/src/uu/mv/src/hardlink.rs +++ b/src/uu/mv/src/hardlink.rs @@ -53,11 +53,11 @@ pub enum HardlinkError { impl std::fmt::Display for HardlinkError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - HardlinkError::Io(e) => write!(f, "I/O error during hardlink operation: {e}"), - HardlinkError::Scan(msg) => { + Self::Io(e) => write!(f, "I/O error during hardlink operation: {e}"), + Self::Scan(msg) => { write!(f, "Failed to scan files for hardlinks: {msg}") } - HardlinkError::Preservation { source, target } => { + Self::Preservation { source, target } => { write!( f, "Failed to preserve hardlink: {} -> {}", @@ -65,7 +65,7 @@ impl std::fmt::Display for HardlinkError { target.display() ) } - HardlinkError::Metadata { path, error } => { + Self::Metadata { path, error } => { write!(f, "Metadata access error for {}: {}", path.display(), error) } } @@ -75,8 +75,8 @@ impl std::fmt::Display for HardlinkError { impl std::error::Error for HardlinkError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - HardlinkError::Io(e) => Some(e), - HardlinkError::Metadata { error, .. } => Some(error), + Self::Io(e) => Some(e), + Self::Metadata { error, .. } => Some(error), _ => None, } } @@ -84,7 +84,7 @@ impl std::error::Error for HardlinkError { impl From for HardlinkError { fn from(error: io::Error) -> Self { - HardlinkError::Io(error) + Self::Io(error) } } @@ -92,14 +92,14 @@ impl From for io::Error { fn from(error: HardlinkError) -> Self { match error { HardlinkError::Io(e) => e, - HardlinkError::Scan(msg) => io::Error::other(msg), - HardlinkError::Preservation { source, target } => io::Error::other(format!( + HardlinkError::Scan(msg) => Self::other(msg), + HardlinkError::Preservation { source, target } => Self::other(format!( "Failed to preserve hardlink: {} -> {}", source.display(), target.display() )), - HardlinkError::Metadata { path, error } => io::Error::other(format!( + HardlinkError::Metadata { path, error } => Self::other(format!( "Metadata access error for {}: {}", path.display(), error diff --git a/src/uu/nl/locales/en-US.ftl b/src/uu/nl/locales/en-US.ftl index 13ae5977e..b23c5965e 100644 --- a/src/uu/nl/locales/en-US.ftl +++ b/src/uu/nl/locales/en-US.ftl @@ -31,6 +31,7 @@ nl-help-number-width = use NUMBER columns for line numbers # Error messages nl-error-invalid-arguments = Invalid arguments supplied. nl-error-could-not-read-line = could not read line +nl-error-could-not-write = could not write output nl-error-line-number-overflow = line number overflow nl-error-invalid-line-width = Invalid line number field width: ‘{ $value }’: Numerical result out of range nl-error-invalid-regex = invalid regular expression diff --git a/src/uu/nl/locales/fr-FR.ftl b/src/uu/nl/locales/fr-FR.ftl index 09ed4f4ad..487f7b581 100644 --- a/src/uu/nl/locales/fr-FR.ftl +++ b/src/uu/nl/locales/fr-FR.ftl @@ -31,6 +31,7 @@ nl-help-number-width = utiliser NUMBER colonnes pour les numéros de ligne # Messages d'erreur nl-error-invalid-arguments = Arguments fournis invalides. nl-error-could-not-read-line = impossible de lire la ligne +nl-error-could-not-write = impossible d'écrire la sortie nl-error-line-number-overflow = débordement du numéro de ligne nl-error-invalid-line-width = Largeur de champ de numéro de ligne invalide : ‘{ $value }’ : Résultat numérique hors limites nl-error-invalid-regex = expression régulière invalide diff --git a/src/uu/nl/src/nl.rs b/src/uu/nl/src/nl.rs index 261d7897f..e94fa847d 100644 --- a/src/uu/nl/src/nl.rs +++ b/src/uu/nl/src/nl.rs @@ -6,7 +6,7 @@ use clap::{Arg, ArgAction, Command}; use std::ffi::{OsStr, OsString}; use std::fs::File; -use std::io::{BufRead, BufReader, Read, stdin}; +use std::io::{BufRead, BufReader, BufWriter, Read, Write, stdin, stdout}; use std::path::Path; use uucore::error::{FromIo, UResult, USimpleError, set_exit_code}; use uucore::{format_usage, show_error, translate}; @@ -346,6 +346,7 @@ pub fn uu_app() -> Command { /// `nl` implements the main functionality for an individual buffer. fn nl(reader: &mut BufReader, stats: &mut Stats, settings: &Settings) -> UResult<()> { + let mut writer = BufWriter::new(stdout()); let mut current_numbering_style = &settings.body_numbering; let mut line = Vec::new(); @@ -382,7 +383,7 @@ fn nl(reader: &mut BufReader, stats: &mut Stats, settings: &Settings if settings.renumber { stats.line_number = Some(settings.starting_line_number); } - println!(); + writeln!(writer).map_err_context(|| translate!("nl-error-could-not-write"))?; } else { let is_line_numbered = match current_numbering_style { // consider $join_blank_lines consecutive empty lines to be one logical line @@ -407,14 +408,16 @@ fn nl(reader: &mut BufReader, stats: &mut Stats, settings: &Settings translate!("nl-error-line-number-overflow"), )); }; - println!( + writeln!( + writer, "{}{}{}", settings .number_format .format(line_number, settings.number_width), settings.number_separator.to_string_lossy(), String::from_utf8_lossy(&line), - ); + ) + .map_err_context(|| translate!("nl-error-could-not-write"))?; // update line number for the potential next line match line_number.checked_add(settings.line_increment) { Some(new_line_number) => stats.line_number = Some(new_line_number), @@ -422,10 +425,14 @@ fn nl(reader: &mut BufReader, stats: &mut Stats, settings: &Settings } } else { let spaces = " ".repeat(settings.number_width + 1); - println!("{spaces}{}", String::from_utf8_lossy(&line)); + writeln!(writer, "{spaces}{}", String::from_utf8_lossy(&line)) + .map_err_context(|| translate!("nl-error-could-not-write"))?; } } } + writer + .flush() + .map_err_context(|| translate!("nl-error-could-not-write"))?; Ok(()) } diff --git a/src/uu/paste/src/paste.rs b/src/uu/paste/src/paste.rs index 23b6d0757..7a8aaab63 100644 --- a/src/uu/paste/src/paste.rs +++ b/src/uu/paste/src/paste.rs @@ -271,7 +271,7 @@ enum DelimiterState<'a> { } impl<'a> DelimiterState<'a> { - fn new(unescaped_and_encoded_delimiters: &'a [Box<[u8]>]) -> DelimiterState<'a> { + fn new(unescaped_and_encoded_delimiters: &'a [Box<[u8]>]) -> Self { match unescaped_and_encoded_delimiters { [] => DelimiterState::NoDelimiters, [only_delimiter] => { @@ -364,8 +364,8 @@ enum InputSource { impl InputSource { fn read_until(&mut self, byte: u8, buf: &mut Vec) -> UResult { let us = match self { - InputSource::File(bu) => bu.read_until(byte, buf)?, - InputSource::StandardInput(rc) => rc + Self::File(bu) => bu.read_until(byte, buf)?, + Self::StandardInput(rc) => rc .try_borrow() .map_err(|bo| { USimpleError::new(1, translate!("paste-error-stdin-borrow", "error" => bo)) diff --git a/src/uu/ptx/src/ptx.rs b/src/uu/ptx/src/ptx.rs index 111d74d39..a0fe9a5e8 100644 --- a/src/uu/ptx/src/ptx.rs +++ b/src/uu/ptx/src/ptx.rs @@ -224,7 +224,7 @@ fn get_config(matches: &clap::ArgMatches) -> UResult { } config.auto_ref = matches.get_flag(options::AUTO_REFERENCE); config.input_ref = matches.get_flag(options::REFERENCES); - config.right_ref &= matches.get_flag(options::RIGHT_SIDE_REFS); + config.right_ref = matches.get_flag(options::RIGHT_SIDE_REFS); config.ignore_case = matches.get_flag(options::IGNORE_CASE); if matches.contains_id(options::MACRO_NAME) { config.macro_name = matches @@ -661,7 +661,7 @@ fn prepare_line_chunks( } fn write_traditional_output( - config: &Config, + config: &mut Config, file_map: &FileMap, words: &BTreeSet, output_filename: &OsStr, @@ -677,6 +677,15 @@ fn write_traditional_output( let context_reg = Regex::new(&config.context_regex).unwrap(); + if !config.right_ref { + let max_ref_len = if config.auto_ref { + get_auto_max_reference_len(words) + } else { + 0 + }; + config.line_width -= max_ref_len; + } + for word_ref in words { let file_map_value: &FileContent = file_map .get(&word_ref.filename) @@ -722,6 +731,31 @@ fn write_traditional_output( Ok(()) } +fn get_auto_max_reference_len(words: &BTreeSet) -> usize { + //Get the maximum length of the reference field + let line_num = words + .iter() + .map(|w| { + if w.local_line_nr == 0 { + 1 + } else { + (w.local_line_nr as f64).log10() as usize + 1 + } + }) + .max() + .unwrap_or(0); + + let filename_len = words + .iter() + .filter(|w| w.filename != "-") + .map(|w| w.filename.maybe_quote().to_string().len()) + .max() + .unwrap_or(0); + + // +1 for the colon + line_num + filename_len + 1 +} + mod options { pub mod format { pub static ROFF: &str = "roff"; @@ -749,7 +783,7 @@ mod options { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; - let config = get_config(&matches)?; + let mut config = get_config(&matches)?; let input_files; let output_file: OsString; @@ -783,7 +817,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let word_filter = WordFilter::new(&matches, &config)?; let file_map = read_input(&input_files).map_err_context(String::new)?; let word_set = create_word_set(&config, &word_filter, &file_map); - write_traditional_output(&config, &file_map, &word_set, &output_file) + write_traditional_output(&mut config, &file_map, &word_set, &output_file) } pub fn uu_app() -> Command { diff --git a/src/uu/rm/Cargo.toml b/src/uu/rm/Cargo.toml index b8d0955f5..d4b9db954 100644 --- a/src/uu/rm/Cargo.toml +++ b/src/uu/rm/Cargo.toml @@ -32,3 +32,12 @@ windows-sys = { workspace = true, features = ["Win32_Storage_FileSystem"] } [[bin]] name = "rm" path = "src/main.rs" + +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } + +[[bench]] +name = "rm_bench" +harness = false diff --git a/src/uu/rm/benches/rm_bench.rs b/src/uu/rm/benches/rm_bench.rs new file mode 100644 index 000000000..1e37bb130 --- /dev/null +++ b/src/uu/rm/benches/rm_bench.rs @@ -0,0 +1,112 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use tempfile::TempDir; +use uu_rm::uumain; +use uucore::benchmark::{fs_tree, run_util_function}; + +/// Benchmark removing a single file (repeated to reach 100ms) +#[divan::bench] +fn rm_single_file(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + fs_tree::create_wide_tree(temp_dir.path(), 1000, 0); + let paths: Vec = (0..1000) + .map(|i| { + temp_dir + .path() + .join(format!("f{i}")) + .to_str() + .unwrap() + .to_string() + }) + .collect(); + (temp_dir, paths) + }) + .bench_values(|(temp_dir, paths)| { + for path in &paths { + black_box(run_util_function(uumain, &[path])); + } + drop(temp_dir); + }); +} + +/// Benchmark removing multiple files +#[divan::bench] +fn rm_multiple_files(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + fs_tree::create_wide_tree(temp_dir.path(), 1000, 0); + let paths: Vec = (0..1000) + .map(|i| { + temp_dir + .path() + .join(format!("f{i}")) + .to_str() + .unwrap() + .to_string() + }) + .collect(); + (temp_dir, paths) + }) + .bench_values(|(temp_dir, paths)| { + let args: Vec<&str> = paths.iter().map(|s| s.as_str()).collect(); + black_box(run_util_function(uumain, &args)); + drop(temp_dir); + }); +} + +/// Benchmark recursive directory removal +#[divan::bench] +fn rm_recursive_tree(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + let test_dir = temp_dir.path().join("test_tree"); + std::fs::create_dir(&test_dir).unwrap(); + // Increase depth and width for longer benchmark + fs_tree::create_balanced_tree(&test_dir, 5, 5, 10); + (temp_dir, test_dir.to_str().unwrap().to_string()) + }) + .bench_values(|(temp_dir, path)| { + black_box(run_util_function(uumain, &["-r", &path])); + drop(temp_dir); + }); +} + +/// Benchmark force removal +#[divan::bench] +fn rm_force_files(bencher: Bencher) { + bencher + .with_inputs(|| { + let temp_dir = TempDir::new().unwrap(); + fs_tree::create_wide_tree(temp_dir.path(), 1000, 0); + let paths: Vec = (0..1000) + .map(|i| { + temp_dir + .path() + .join(format!("f{i}")) + .to_str() + .unwrap() + .to_string() + }) + .collect(); + (temp_dir, paths) + }) + .bench_values(|(temp_dir, paths)| { + let mut args = vec!["-f"]; + let path_refs: Vec<&str> = paths.iter().map(|s| s.as_str()).collect(); + args.extend(path_refs); + black_box(run_util_function(uumain, &args)); + drop(temp_dir); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs new file mode 100644 index 000000000..265229cab --- /dev/null +++ b/src/uu/rm/src/platform/linux.rs @@ -0,0 +1,324 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +// Linux-specific implementations for the rm utility + +// spell-checker:ignore fstatat unlinkat + +use std::ffi::OsStr; +use std::fs; +use std::path::Path; +use uucore::display::Quotable; +use uucore::error::FromIo; +use uucore::safe_traversal::DirFd; +use uucore::show_error; +use uucore::translate; + +use super::super::{ + InteractiveMode, Options, is_dir_empty, is_readable_metadata, prompt_descend, prompt_dir, + prompt_file, remove_file, show_permission_denied_error, show_removal_error, + verbose_removed_directory, verbose_removed_file, +}; + +/// Whether the given file or directory is readable. +pub fn is_readable(path: &Path) -> bool { + fs::metadata(path).is_ok_and(|metadata| is_readable_metadata(&metadata)) +} + +/// Remove a single file using safe traversal +pub fn safe_remove_file(path: &Path, options: &Options) -> Option { + let parent = path.parent()?; + let file_name = path.file_name()?; + + let dir_fd = DirFd::open(parent).ok()?; + + match dir_fd.unlink_at(file_name, false) { + Ok(_) => { + verbose_removed_file(path, options); + Some(false) + } + Err(e) => { + if e.kind() == std::io::ErrorKind::PermissionDenied { + show_error!("cannot remove {}: Permission denied", path.quote()); + } else { + let _ = show_removal_error(e, path); + } + Some(true) + } + } +} + +/// Remove an empty directory using safe traversal +pub fn safe_remove_empty_dir(path: &Path, options: &Options) -> Option { + let parent = path.parent()?; + let dir_name = path.file_name()?; + + let dir_fd = DirFd::open(parent).ok()?; + + match dir_fd.unlink_at(dir_name, true) { + Ok(_) => { + verbose_removed_directory(path, options); + Some(false) + } + Err(e) => { + let e = + e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())); + show_error!("{e}"); + Some(true) + } + } +} + +/// Helper to handle errors with force mode consideration +fn handle_error_with_force(e: std::io::Error, path: &Path, options: &Options) -> bool { + // Permission denied errors should be shown even in force mode + // This matches GNU rm behavior + if e.kind() == std::io::ErrorKind::PermissionDenied { + show_permission_denied_error(path); + return true; + } + + if !options.force { + let e = e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())); + show_error!("{e}"); + } + !options.force +} + +/// Helper to handle permission denied errors +fn handle_permission_denied( + dir_fd: &DirFd, + entry_name: &OsStr, + entry_path: &Path, + options: &Options, +) -> bool { + // When we can't open a subdirectory due to permission denied, + // try to remove it directly (it might be empty). + // This matches GNU rm behavior with -f flag. + if let Err(remove_err) = dir_fd.unlink_at(entry_name, true) { + // Failed to remove - show appropriate error + if remove_err.kind() == std::io::ErrorKind::PermissionDenied { + // Permission denied errors are always shown, even with force + show_permission_denied_error(entry_path); + return true; + } else if !options.force { + let remove_err = remove_err.map_err_context( + || translate!("rm-error-cannot-remove", "file" => entry_path.quote()), + ); + show_error!("{remove_err}"); + return true; + } + // With force mode, suppress non-permission errors + return !options.force; + } + // Successfully removed empty directory + verbose_removed_directory(entry_path, options); + false +} + +/// Helper to handle unlink operation with error reporting +fn handle_unlink( + dir_fd: &DirFd, + entry_name: &OsStr, + entry_path: &Path, + is_dir: bool, + options: &Options, +) -> bool { + if let Err(e) = dir_fd.unlink_at(entry_name, is_dir) { + let e = e + .map_err_context(|| translate!("rm-error-cannot-remove", "file" => entry_path.quote())); + show_error!("{e}"); + true + } else { + if is_dir { + verbose_removed_directory(entry_path, options); + } else { + verbose_removed_file(entry_path, options); + } + false + } +} + +/// Helper function to remove directory handling special cases +pub fn remove_dir_with_special_cases(path: &Path, options: &Options, error_occurred: bool) -> bool { + match fs::remove_dir(path) { + Err(_) if !error_occurred && !is_readable(path) => { + // For compatibility with GNU test case + // `tests/rm/unread2.sh`, show "Permission denied" in this + // case instead of "Directory not empty". + show_permission_denied_error(path); + true + } + Err(_) if !error_occurred && path.read_dir().is_err() => { + // For compatibility with GNU test case on Linux + // Check if directory is readable by attempting to read it + show_permission_denied_error(path); + true + } + Err(e) if !error_occurred => show_removal_error(e, path), + Err(_) => { + // If we already had errors while + // trying to remove the children, then there is no need to + // show another error message as we return from each level + // of the recursion. + error_occurred + } + Ok(_) => { + verbose_removed_directory(path, options); + false + } + } +} + +pub fn safe_remove_dir_recursive(path: &Path, options: &Options) -> bool { + // Base case 1: this is a file or a symbolic link. + // Use lstat to avoid race condition between check and use + match fs::symlink_metadata(path) { + Ok(metadata) if !metadata.is_dir() => { + return remove_file(path, options); + } + Ok(_) => {} + Err(e) => { + return show_removal_error(e, path); + } + } + + // Try to open the directory using DirFd for secure traversal + let dir_fd = match DirFd::open(path) { + Ok(fd) => fd, + Err(e) => { + // If we can't open the directory for safe traversal, + // handle the error appropriately and try to remove if possible + if e.kind() == std::io::ErrorKind::PermissionDenied { + // Try to remove the directory directly if it's empty + if fs::remove_dir(path).is_ok() { + verbose_removed_directory(path, options); + return false; + } + // If we can't read the directory AND can't remove it, + // show permission denied error for GNU compatibility + return show_permission_denied_error(path); + } + return show_removal_error(e, path); + } + }; + + let error = safe_remove_dir_recursive_impl(path, &dir_fd, options); + + // After processing all children, remove the directory itself + if error { + error + } else { + // Ask user permission if needed + if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) { + return false; + } + + // Before trying to remove the directory, check if it's actually empty + // This handles the case where some children weren't removed due to user "no" responses + if !is_dir_empty(path) { + // Directory is not empty, so we can't/shouldn't remove it + // In interactive mode, this might be expected if user said "no" to some children + // In non-interactive mode, this indicates an error (some children couldn't be removed) + if options.interactive == InteractiveMode::Always { + return false; + } + // Try to remove the directory anyway and let the system tell us why it failed + // Use false for error_occurred since this is the main error we want to report + return remove_dir_with_special_cases(path, options, false); + } + + // Directory is empty and user approved removal + remove_dir_with_special_cases(path, options, error) + } +} + +pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options) -> bool { + // Read directory entries using safe traversal + let entries = match dir_fd.read_dir() { + Ok(entries) => entries, + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + if !options.force { + show_permission_denied_error(path); + } + return !options.force; + } + Err(e) => { + return handle_error_with_force(e, path, options); + } + }; + + let mut error = false; + + // Process each entry + for entry_name in entries { + let entry_path = path.join(&entry_name); + + // Get metadata for the entry using fstatat + let entry_stat = match dir_fd.stat_at(&entry_name, false) { + Ok(stat) => stat, + Err(e) => { + error = handle_error_with_force(e, &entry_path, options); + continue; + } + }; + + // Check if it's a directory + let is_dir = (entry_stat.st_mode & libc::S_IFMT) == libc::S_IFDIR; + + if is_dir { + // Ask user if they want to descend into this directory + if options.interactive == InteractiveMode::Always + && !is_dir_empty(&entry_path) + && !prompt_descend(&entry_path) + { + continue; + } + + // Recursively remove subdirectory using safe traversal + let child_dir_fd = match dir_fd.open_subdir(&entry_name) { + Ok(fd) => fd, + Err(e) => { + // If we can't open the subdirectory for safe traversal, + // try to handle it as best we can with safe operations + if e.kind() == std::io::ErrorKind::PermissionDenied { + error = handle_permission_denied( + dir_fd, + entry_name.as_ref(), + &entry_path, + options, + ); + } else { + error = handle_error_with_force(e, &entry_path, options); + } + continue; + } + }; + + let child_error = safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options); + error = error || child_error; + + // Ask user permission if needed for this subdirectory + if !child_error + && options.interactive == InteractiveMode::Always + && !prompt_dir(&entry_path, options) + { + continue; + } + + // Remove the now-empty subdirectory using safe unlinkat + if !child_error { + error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options); + } + } else { + // Remove file - check if user wants to remove it first + if prompt_file(&entry_path, options) { + error = handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options); + } + } + } + + error +} diff --git a/src/uu/rm/src/platform/mod.rs b/src/uu/rm/src/platform/mod.rs new file mode 100644 index 000000000..1f2911acb --- /dev/null +++ b/src/uu/rm/src/platform/mod.rs @@ -0,0 +1,12 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +// Platform-specific implementations for the rm utility + +#[cfg(target_os = "linux")] +pub mod linux; + +#[cfg(target_os = "linux")] +pub use linux::*; diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 763590f79..3309ab006 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -21,12 +21,13 @@ use thiserror::Error; use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult}; use uucore::parser::shortcut_value_parser::ShortcutValueParser; -#[cfg(target_os = "linux")] -use uucore::safe_traversal::DirFd; use uucore::translate; - use uucore::{format_usage, os_str_as_bytes, prompt_yes, show_error}; +mod platform; +#[cfg(target_os = "linux")] +use platform::{safe_remove_dir_recursive, safe_remove_empty_dir, safe_remove_file}; + #[derive(Debug, Error)] enum RmError { #[error("{}", translate!("rm-error-missing-operand", "util_name" => uucore::execution_phrase()))] @@ -47,6 +48,55 @@ enum RmError { impl UError for RmError {} +/// Helper function to print verbose message for removed file +fn verbose_removed_file(path: &Path, options: &Options) { + if options.verbose { + println!( + "{}", + translate!("rm-verbose-removed", "file" => normalize(path).quote()) + ); + } +} + +/// Helper function to print verbose message for removed directory +fn verbose_removed_directory(path: &Path, options: &Options) { + if options.verbose { + println!( + "{}", + translate!("rm-verbose-removed-directory", "file" => normalize(path).quote()) + ); + } +} + +/// Helper function to show error with context and return error status +fn show_removal_error(error: std::io::Error, path: &Path) -> bool { + if error.kind() == std::io::ErrorKind::PermissionDenied { + show_error!("cannot remove {}: Permission denied", path.quote()); + } else { + let e = + error.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())); + show_error!("{e}"); + } + true +} + +/// Helper function for permission denied errors +fn show_permission_denied_error(path: &Path) -> bool { + show_error!("cannot remove {}: Permission denied", path.quote()); + true +} + +/// Helper function to remove a directory and handle results +fn remove_dir_with_feedback(path: &Path, options: &Options) -> bool { + match fs::remove_dir(path) { + Ok(_) => { + verbose_removed_directory(path, options); + false + } + Err(e) => show_removal_error(e, path), + } +} + #[derive(Eq, PartialEq, Clone, Copy)] /// Enum, determining when the `rm` will prompt the user about the file deletion pub enum InteractiveMode { @@ -395,6 +445,7 @@ fn is_readable_metadata(metadata: &Metadata) -> bool { /// Whether the given file or directory is readable. #[cfg(unix)] +#[cfg(not(target_os = "linux"))] fn is_readable(path: &Path) -> bool { match fs::metadata(path) { Err(_) => false, @@ -430,140 +481,6 @@ fn is_writable(_path: &Path) -> bool { true } -#[cfg(target_os = "linux")] -fn safe_remove_dir_recursive(path: &Path, options: &Options) -> bool { - // Try to open the directory using DirFd for secure traversal - let dir_fd = match DirFd::open(path) { - Ok(fd) => fd, - Err(e) => { - show_error!( - "{}", - e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())) - ); - return true; - } - }; - - let error = safe_remove_dir_recursive_impl(path, &dir_fd, options); - - // After processing all children, remove the directory itself - if error { - error - } else { - // Ask user permission if needed - if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) { - return false; - } - - // Use regular fs::remove_dir for the root since we can't unlinkat ourselves - match fs::remove_dir(path) { - Ok(_) => false, - Err(e) => { - let e = e.map_err_context( - || translate!("rm-error-cannot-remove", "file" => path.quote()), - ); - show_error!("{e}"); - true - } - } - } -} - -#[cfg(target_os = "linux")] -fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options) -> bool { - // Check if we should descend into this directory - if options.interactive == InteractiveMode::Always - && !is_dir_empty(path) - && !prompt_descend(path) - { - return false; - } - - // Read directory entries using safe traversal - let entries = match dir_fd.read_dir() { - Ok(entries) => entries, - Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { - // This is not considered an error - just like the original - return false; - } - Err(e) => { - show_error!( - "{}", - e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())) - ); - return true; - } - }; - - let mut error = false; - - // Process each entry - for entry_name in entries { - let entry_path = path.join(&entry_name); - - // Get metadata for the entry using fstatat - let entry_stat = match dir_fd.stat_at(&entry_name, false) { - Ok(stat) => stat, - Err(e) => { - let e = e.map_err_context( - || translate!("rm-error-cannot-remove", "file" => entry_path.quote()), - ); - show_error!("{e}"); - error = true; - continue; - } - }; - - // Check if it's a directory - let is_dir = (entry_stat.st_mode & libc::S_IFMT) == libc::S_IFDIR; - - if is_dir { - // Recursively remove directory - let subdir_fd = match dir_fd.open_subdir(&entry_name) { - Ok(fd) => fd, - Err(e) => { - let e = e.map_err_context( - || translate!("rm-error-cannot-remove", "file" => entry_path.quote()), - ); - show_error!("{e}"); - error = true; - continue; - } - }; - - let child_error = safe_remove_dir_recursive_impl(&entry_path, &subdir_fd, options); - error = error || child_error; - - // Try to remove the directory (even if there were some child errors) - // Ask user permission if needed - if options.interactive == InteractiveMode::Always && !prompt_dir(&entry_path, options) { - continue; - } - - if let Err(e) = dir_fd.unlink_at(&entry_name, true) { - let e = e.map_err_context( - || translate!("rm-error-cannot-remove", "file" => entry_path.quote()), - ); - show_error!("{e}"); - error = true; - } - } else { - // Remove file - check if user wants to remove it first - if prompt_file(&entry_path, options) { - if let Err(e) = dir_fd.unlink_at(&entry_name, false) { - let e = e.map_err_context( - || translate!("rm-error-cannot-remove", "file" => entry_path.quote()), - ); - show_error!("{e}"); - error = true; - } - } - } - } - - error -} - /// Recursively remove the directory tree rooted at the given path. /// /// If `path` is a file or a symbolic link, just remove it. If it is a @@ -590,17 +507,13 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool { return false; } - // Use secure traversal on Linux for long paths + // Use secure traversal on Linux for all recursive directory removals #[cfg(target_os = "linux")] { - if let Some(s) = path.to_str() { - if s.len() > 1000 { - return safe_remove_dir_recursive(path, options); - } - } + safe_remove_dir_recursive(path, options) } - // Fallback for non-Linux or shorter paths + // Fallback for non-Linux or use fs::remove_dir_all for very long paths #[cfg(not(target_os = "linux"))] { if let Some(s) = path.to_str() { @@ -617,62 +530,59 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool { } } } - } - // Recursive case: this is a directory. - let mut error = false; - match fs::read_dir(path) { - Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { - // This is not considered an error. - } - Err(_) => error = true, - Ok(iter) => { - for entry in iter { - match entry { - Err(_) => error = true, - Ok(entry) => { - let child_error = remove_dir_recursive(&entry.path(), options); - error = error || child_error; + // Recursive case: this is a directory. + let mut error = false; + match fs::read_dir(path) { + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + // This is not considered an error. + } + Err(_) => error = true, + Ok(iter) => { + for entry in iter { + match entry { + Err(_) => error = true, + Ok(entry) => { + let child_error = remove_dir_recursive(&entry.path(), options); + error = error || child_error; + } } } } } - } - // Ask the user whether to remove the current directory. - if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) { - return false; - } + // Ask the user whether to remove the current directory. + if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) { + return false; + } - // Try removing the directory itself. - match fs::remove_dir(path) { - Err(_) if !error && !is_readable(path) => { - // For compatibility with GNU test case - // `tests/rm/unread2.sh`, show "Permission denied" in this - // case instead of "Directory not empty". - show_error!("cannot remove {}: Permission denied", path.quote()); - error = true; + // Try removing the directory itself. + match fs::remove_dir(path) { + Err(_) if !error && !is_readable(path) => { + // For compatibility with GNU test case + // `tests/rm/unread2.sh`, show "Permission denied" in this + // case instead of "Directory not empty". + show_permission_denied_error(path); + error = true; + } + Err(e) if !error => { + let e = e.map_err_context( + || translate!("rm-error-cannot-remove", "file" => path.quote()), + ); + show_error!("{e}"); + error = true; + } + Err(_) => { + // If there has already been at least one error when + // trying to remove the children, then there is no need to + // show another error message as we return from each level + // of the recursion. + } + Ok(_) => verbose_removed_directory(path, options), } - Err(e) if !error => { - let e = - e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())); - show_error!("{e}"); - error = true; - } - Err(_) => { - // If there has already been at least one error when - // trying to remove the children, then there is no need to - // show another error message as we return from each level - // of the recursion. - } - Ok(_) if options.verbose => println!( - "{}", - translate!("rm-verbose-removed-directory", "file" => normalize(path).quote()) - ), - Ok(_) => {} - } - error + error + } } fn handle_dir(path: &Path, options: &Options) -> bool { @@ -725,36 +635,32 @@ fn remove_dir(path: &Path, options: &Options) -> bool { return true; } - // Try to remove the directory. - match fs::remove_dir(path) { - Ok(_) => { - if options.verbose { - println!( - "{}", - translate!("rm-verbose-removed-directory", "file" => normalize(path).quote()) - ); - } - false - } - Err(e) => { - let e = - e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())); - show_error!("{e}"); - true + // Use safe traversal on Linux for empty directory removal + #[cfg(target_os = "linux")] + { + if let Some(result) = safe_remove_empty_dir(path, options) { + return result; } } + + // Fallback method for non-Linux or when safe traversal is unavailable + remove_dir_with_feedback(path, options) } fn remove_file(path: &Path, options: &Options) -> bool { if prompt_file(path, options) { + // Use safe traversal on Linux for individual file removal + #[cfg(target_os = "linux")] + { + if let Some(result) = safe_remove_file(path, options) { + return result; + } + } + + // Fallback method for non-Linux or when safe traversal is unavailable match fs::remove_file(path) { Ok(_) => { - if options.verbose { - println!( - "{}", - translate!("rm-verbose-removed", "file" => normalize(path).quote()) - ); - } + verbose_removed_file(path, options); } Err(e) => { if e.kind() == std::io::ErrorKind::PermissionDenied { @@ -764,7 +670,7 @@ fn remove_file(path: &Path, options: &Options) -> bool { RmError::CannotRemovePermissionDenied(path.as_os_str().to_os_string()) ); } else { - show_error!("cannot remove {}: {e}", path.quote()); + return show_removal_error(e, path); } return true; } @@ -857,6 +763,7 @@ fn handle_writable_directory(path: &Path, options: &Options, metadata: &Metadata options.interactive, ) { (false, _, _, InteractiveMode::PromptProtected) => true, + (false, false, false, InteractiveMode::Never) => true, // Don't prompt when interactive is never (_, false, false, _) => prompt_yes!( "attempt removal of inaccessible directory {}?", path.quote() diff --git a/src/uu/runcon/src/main.rs b/src/uu/runcon/src/main.rs index ab4c4b159..dde0f2394 100644 --- a/src/uu/runcon/src/main.rs +++ b/src/uu/runcon/src/main.rs @@ -1,2 +1,12 @@ -#![cfg(target_os = "linux")] +// On non-Linux targets, provide a stub main to keep the binary target present +// and the workspace buildable. Using item-level cfg avoids excluding the crate +// entirely (via #![cfg(...)]), which can break tooling and cross builds that +// expect this binary to exist even when it's a no-op off Linux. +#[cfg(target_os = "linux")] uucore::bin!(uu_runcon); + +#[cfg(not(target_os = "linux"))] +fn main() { + eprintln!("runcon: SELinux is not supported on this platform"); + std::process::exit(1); +} diff --git a/src/uu/seq/Cargo.toml b/src/uu/seq/Cargo.toml index f96e98079..6f74ce37a 100644 --- a/src/uu/seq/Cargo.toml +++ b/src/uu/seq/Cargo.toml @@ -1,4 +1,4 @@ -# spell-checker:ignore bigdecimal cfgs extendedbigdecimal +# spell-checker:ignore bigdecimal extendedbigdecimal [package] name = "uu_seq" description = "seq ~ (uutils) display a sequence of numbers" @@ -12,6 +12,9 @@ categories.workspace = true edition.workspace = true readme.workspace = true +[lints] +workspace = true + [lib] path = "src/seq.rs" @@ -34,12 +37,11 @@ fluent = { workspace = true } name = "seq" path = "src/main.rs" -# FIXME: this is the only crate that has a separate lints configuration, -# which for now means a full copy of all clippy and rust lints here. -[lints.clippy] -all = { level = "deny", priority = -1 } +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } -# Allow "fuzzing" as a "cfg" condition name -# https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html -[lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(fuzzing)'] } +[[bench]] +name = "seq_bench" +harness = false diff --git a/src/uu/seq/benches/seq_bench.rs b/src/uu/seq/benches/seq_bench.rs new file mode 100644 index 000000000..d8c52131d --- /dev/null +++ b/src/uu/seq/benches/seq_bench.rs @@ -0,0 +1,47 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use uu_seq::uumain; +use uucore::benchmark::run_util_function; + +/// Benchmark simple integer sequence +#[divan::bench] +fn seq_integers(bencher: Bencher) { + bencher.bench(|| { + black_box(run_util_function(uumain, &["1", "1000000"])); + }); +} + +/// Benchmark sequence with custom separator +#[divan::bench] +fn seq_custom_separator(bencher: Bencher) { + bencher.bench(|| { + black_box(run_util_function(uumain, &["-s", ",", "1", "1000000"])); + }); +} + +/// Benchmark sequence with step +#[divan::bench] +fn seq_with_step(bencher: Bencher) { + bencher.bench(|| { + black_box(run_util_function(uumain, &["1", "2", "1000000"])); + }); +} + +/// Benchmark formatted output +#[divan::bench] +fn seq_formatted(bencher: Bencher) { + bencher.bench(|| { + black_box(run_util_function( + uumain, + &["-f", "%.3f", "1", "0.1", "10000"], + )); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/seq/src/numberparse.rs b/src/uu/seq/src/numberparse.rs index 2777acd63..40c427aaf 100644 --- a/src/uu/seq/src/numberparse.rs +++ b/src/uu/seq/src/numberparse.rs @@ -40,7 +40,7 @@ fn compute_num_digits(input: &str, ebd: ExtendedBigDecimal) -> PreciseNumber { return PreciseNumber { number: ebd, num_integral_digits: 0, - num_fractional_digits: if input.contains(".") || input.contains("p") { + num_fractional_digits: if input.contains('.') || input.contains('p') { None } else { Some(0) @@ -49,17 +49,17 @@ fn compute_num_digits(input: &str, ebd: ExtendedBigDecimal) -> PreciseNumber { } // Split the exponent part, if any - let parts: Vec<&str> = input.split("e").collect(); + let parts: Vec<&str> = input.split('e').collect(); debug_assert!(parts.len() <= 2); // Count all the digits up to `.`, `-` sign is included. - let (mut int_digits, mut frac_digits) = match parts[0].find(".") { + let (mut int_digits, mut frac_digits) = match parts[0].find('.') { Some(i) => { // Cover special case .X and -.X where we behave as if there was a leading 0: // 0.X, -0.X. let int_digits = match i { 0 => 1, - 1 if parts[0].starts_with("-") => 2, + 1 if parts[0].starts_with('-') => 2, _ => i, }; @@ -75,7 +75,7 @@ fn compute_num_digits(input: &str, ebd: ExtendedBigDecimal) -> PreciseNumber { // For positive exponents, effectively expand the number. Ignore negative exponents. // Also ignore overflowed exponents (unwrap_or(0)). if exp > 0 { - int_digits += exp.try_into().unwrap_or(0) + int_digits += exp.try_into().unwrap_or(0); }; frac_digits = if exp < frac_digits as i64 { // Subtract from i128 to avoid any overflow @@ -106,7 +106,7 @@ impl FromStr for PreciseNumber { ebd } ExtendedBigDecimal::Infinity | ExtendedBigDecimal::MinusInfinity => { - return Ok(PreciseNumber { + return Ok(Self { number: ebd, num_integral_digits: 0, num_fractional_digits: Some(0), diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 4c050c2c7..6e0910951 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -106,10 +106,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let options = SeqOptions { separator: matches .get_one::(OPT_SEPARATOR) - .map_or(OsString::from("\n"), |s| s.to_os_string()), + .cloned() + .unwrap_or_else(|| OsString::from("\n")), terminator: matches .get_one::(OPT_TERMINATOR) - .map_or(OsString::from("\n"), |s| s.to_os_string()), + .cloned() + .unwrap_or_else(|| OsString::from("\n")), equal_width: matches.get_flag(OPT_EQUAL_WIDTH), format: matches.get_one::(OPT_FORMAT).map(|s| s.as_str()), }; diff --git a/src/uu/split/Cargo.toml b/src/uu/split/Cargo.toml index 3d5c7934d..d6cf871ac 100644 --- a/src/uu/split/Cargo.toml +++ b/src/uu/split/Cargo.toml @@ -27,3 +27,12 @@ fluent = { workspace = true } [[bin]] name = "split" path = "src/main.rs" + +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } + +[[bench]] +name = "split_bench" +harness = false diff --git a/src/uu/split/benches/split_bench.rs b/src/uu/split/benches/split_bench.rs new file mode 100644 index 000000000..d09d658b0 --- /dev/null +++ b/src/uu/split/benches/split_bench.rs @@ -0,0 +1,97 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use tempfile::TempDir; +use uu_split::uumain; +use uucore::benchmark::{run_util_function, setup_test_file, text_data}; + +/// Benchmark splitting by line count +#[divan::bench] +fn split_lines(bencher: Bencher) { + let data = text_data::generate_by_lines(100_000, 80); + let file_path = setup_test_file(&data); + + bencher + .with_inputs(|| { + let output_dir = TempDir::new().unwrap(); + let prefix = output_dir.path().join("x"); + (output_dir, prefix.to_str().unwrap().to_string()) + }) + .bench_values(|(output_dir, prefix)| { + black_box(run_util_function( + uumain, + &["-l", "1000", file_path.to_str().unwrap(), &prefix], + )); + drop(output_dir); + }); +} + +/// Benchmark splitting by byte size +#[divan::bench] +fn split_bytes(bencher: Bencher) { + let data = text_data::generate_by_size(10, 80); + let file_path = setup_test_file(&data); + + bencher + .with_inputs(|| { + let output_dir = TempDir::new().unwrap(); + let prefix = output_dir.path().join("x"); + (output_dir, prefix.to_str().unwrap().to_string()) + }) + .bench_values(|(output_dir, prefix)| { + black_box(run_util_function( + uumain, + &["-b", "100K", file_path.to_str().unwrap(), &prefix], + )); + drop(output_dir); + }); +} + +/// Benchmark splitting by number of chunks +#[divan::bench] +fn split_number_chunks(bencher: Bencher) { + let data = text_data::generate_by_lines(100_000, 80); + let file_path = setup_test_file(&data); + + bencher + .with_inputs(|| { + let output_dir = TempDir::new().unwrap(); + let prefix = output_dir.path().join("x"); + (output_dir, prefix.to_str().unwrap().to_string()) + }) + .bench_values(|(output_dir, prefix)| { + black_box(run_util_function( + uumain, + &["-n", "10", file_path.to_str().unwrap(), &prefix], + )); + drop(output_dir); + }); +} + +/// Benchmark splitting with numeric suffix +#[divan::bench] +fn split_numeric_suffix(bencher: Bencher) { + let data = text_data::generate_by_lines(100_000, 80); + let file_path = setup_test_file(&data); + + bencher + .with_inputs(|| { + let output_dir = TempDir::new().unwrap(); + let prefix = output_dir.path().join("x"); + (output_dir, prefix.to_str().unwrap().to_string()) + }) + .bench_values(|(output_dir, prefix)| { + black_box(run_util_function( + uumain, + &["-d", "-l", "500", file_path.to_str().unwrap(), &prefix], + )); + drop(output_dir); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index 5f9e88417..45d3e389b 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -183,9 +183,9 @@ impl std::str::FromStr for QuotingStyle { fn from_str(s: &str) -> Result { match s { - "locale" => Ok(QuotingStyle::Locale), - "shell" => Ok(QuotingStyle::Shell), - "shell-escape-always" => Ok(QuotingStyle::ShellEscapeAlways), + "locale" => Ok(Self::Locale), + "shell" => Ok(Self::Shell), + "shell-escape-always" => Ok(Self::ShellEscapeAlways), // The others aren't exposed to the user _ => Err(StatError::InvalidQuotingStyle { style: s.to_string(), diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 85380bf40..c1c599c91 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -132,7 +132,7 @@ struct Graph<'input> { } impl<'input> Graph<'input> { - fn new(name: String) -> Graph<'input> { + fn new(name: String) -> Self { Self { name, nodes: HashMap::default(), diff --git a/src/uu/unexpand/Cargo.toml b/src/uu/unexpand/Cargo.toml index 95610ad5a..19128ad03 100644 --- a/src/uu/unexpand/Cargo.toml +++ b/src/uu/unexpand/Cargo.toml @@ -24,6 +24,15 @@ unicode-width = { workspace = true } uucore = { workspace = true } fluent = { workspace = true } +[dev-dependencies] +divan = { workspace = true } +tempfile = { workspace = true } +uucore = { workspace = true, features = ["benchmark"] } + [[bin]] name = "unexpand" path = "src/main.rs" + +[[bench]] +name = "unexpand_bench" +harness = false diff --git a/src/uu/unexpand/benches/unexpand_bench.rs b/src/uu/unexpand/benches/unexpand_bench.rs new file mode 100644 index 000000000..1f9c19469 --- /dev/null +++ b/src/uu/unexpand/benches/unexpand_bench.rs @@ -0,0 +1,54 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use divan::{Bencher, black_box}; +use uu_unexpand::uumain; +use uucore::benchmark::{create_test_file, run_util_function}; + +/// Generate text data with leading spaces (typical unexpand use case) +fn generate_indented_text(num_lines: usize) -> Vec { + let mut data = Vec::new(); + for i in 0..num_lines { + // Add varying amounts of leading spaces (4, 8, 12, etc.) + let indent = (i % 4 + 1) * 4; + data.extend(vec![b' '; indent]); + data.extend_from_slice(b"This is a line of text with leading spaces\n"); + } + data +} + +/// Benchmark unexpanding many lines with leading spaces (most common use case) +#[divan::bench(args = [100_000])] +fn unexpand_many_lines(bencher: Bencher, num_lines: usize) { + let temp_dir = tempfile::tempdir().unwrap(); + let data = generate_indented_text(num_lines); + let file_path = create_test_file(&data, temp_dir.path()); + let file_path_str = file_path.to_str().unwrap(); + + bencher.bench(|| { + black_box(run_util_function(uumain, &[file_path_str])); + }); +} + +/// Benchmark large file with spaces (tests performance on large files) +#[divan::bench(args = [10])] +fn unexpand_large_file(bencher: Bencher, size_mb: usize) { + let temp_dir = tempfile::tempdir().unwrap(); + + // Generate approximately size_mb worth of indented lines + let line_size = 50; // approximate bytes per line + let num_lines = (size_mb * 1024 * 1024) / line_size; + let data = generate_indented_text(num_lines); + let file_path = create_test_file(&data, temp_dir.path()); + let file_path_str = file_path.to_str().unwrap(); + + bencher.bench(|| { + black_box(run_util_function(uumain, &[file_path_str])); + }); +} + +fn main() { + divan::main(); +} diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs index 9f306c999..5d1b3319f 100644 --- a/src/uu/unexpand/src/unexpand.rs +++ b/src/uu/unexpand/src/unexpand.rs @@ -6,6 +6,7 @@ // spell-checker:ignore (ToDO) nums aflag uflag scol prevtab amode ctype cwidth nbytes lastcol pctype Preprocess use clap::{Arg, ArgAction, Command}; +use std::ffi::OsString; use std::fs::File; use std::io::{BufRead, BufReader, BufWriter, Read, Stdout, Write, stdin, stdout}; use std::num::IntErrorKind; @@ -76,7 +77,7 @@ mod options { } struct Options { - files: Vec, + files: Vec, tabstops: Vec, aflag: bool, uflag: bool, @@ -93,9 +94,9 @@ impl Options { && !matches.get_flag(options::FIRST_ONLY); let uflag = !matches.get_flag(options::NO_UTF8); - let files = match matches.get_many::(options::FILE) { + let files = match matches.get_many::(options::FILE) { Some(v) => v.cloned().collect(), - None => vec!["-".to_owned()], + None => vec![OsString::from("-")], }; Ok(Self { @@ -115,24 +116,28 @@ fn is_digit_or_comma(c: char) -> bool { /// Preprocess command line arguments and expand shortcuts. For example, "-7" is expanded to /// "--tabs=7 --first-only" and "-1,3" to "--tabs=1 --tabs=3 --first-only". However, if "-a" or /// "--all" is provided, "--first-only" is omitted. -fn expand_shortcuts(args: &[String]) -> Vec { +fn expand_shortcuts(args: Vec) -> Vec { let mut processed_args = Vec::with_capacity(args.len()); let mut is_all_arg_provided = false; let mut has_shortcuts = false; for arg in args { - if arg.starts_with('-') && arg[1..].chars().all(is_digit_or_comma) { - arg[1..] - .split(',') - .filter(|s| !s.is_empty()) - .for_each(|s| processed_args.push(format!("--tabs={s}"))); - has_shortcuts = true; - } else { - processed_args.push(arg.to_string()); + if let Some(arg) = arg.to_str() { + if arg.starts_with('-') && arg[1..].chars().all(is_digit_or_comma) { + arg[1..] + .split(',') + .filter(|s| !s.is_empty()) + .for_each(|s| processed_args.push(OsString::from(format!("--tabs={s}")))); + has_shortcuts = true; + } else { + processed_args.push(arg.into()); - if arg == "--all" || arg == "-a" { - is_all_arg_provided = true; + if arg == "--all" || arg == "-a" { + is_all_arg_provided = true; + } } + } else { + processed_args.push(arg); } } @@ -145,9 +150,8 @@ fn expand_shortcuts(args: &[String]) -> Vec { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args = args.collect_ignore(); - - let matches = uucore::clap_localization::handle_clap_result(uu_app(), expand_shortcuts(&args))?; + let matches = + uucore::clap_localization::handle_clap_result(uu_app(), expand_shortcuts(args.collect()))?; unexpand(&Options::new(&matches)?) } @@ -163,7 +167,8 @@ pub fn uu_app() -> Command { Arg::new(options::FILE) .hide(true) .action(ArgAction::Append) - .value_hint(clap::ValueHint::FilePath), + .value_hint(clap::ValueHint::FilePath) + .value_parser(clap::value_parser!(OsString)), ) .arg( Arg::new(options::ALL) @@ -196,7 +201,7 @@ pub fn uu_app() -> Command { ) } -fn open(path: &str) -> UResult>> { +fn open(path: &OsString) -> UResult>> { let file_buf; let filename = Path::new(path); if filename.is_dir() { @@ -207,7 +212,7 @@ fn open(path: &str) -> UResult>> { } else if path == "-" { Ok(BufReader::new(Box::new(stdin()) as Box)) } else { - file_buf = File::open(path).map_err_context(|| path.to_string())?; + file_buf = File::open(path).map_err_context(|| path.to_string_lossy().to_string())?; Ok(BufReader::new(Box::new(file_buf) as Box)) } } @@ -313,12 +318,52 @@ fn unexpand_line( lastcol: usize, ts: &[usize], ) -> UResult<()> { + // Fast path: if we're not converting all spaces (-a flag not set) + // and the line doesn't start with spaces, just write it directly + if !options.aflag && !buf.is_empty() && buf[0] != b' ' && buf[0] != b'\t' { + output.write_all(buf)?; + buf.truncate(0); + return Ok(()); + } + let mut byte = 0; // offset into the buffer let mut col = 0; // the current column let mut scol = 0; // the start col for the current span, i.e., the already-printed width let mut init = true; // are we at the start of the line? let mut pctype = CharType::Other; + // Fast path for leading spaces in non-UTF8 mode: count consecutive spaces/tabs at start + if !options.uflag && !options.aflag { + // In default mode (not -a), we only convert leading spaces + // So we can batch process them and then copy the rest + while byte < buf.len() { + match buf[byte] { + b' ' => { + col += 1; + byte += 1; + } + b'\t' => { + col += next_tabstop(ts, col).unwrap_or(1); + byte += 1; + pctype = CharType::Tab; + } + _ => break, + } + } + + // If we found spaces/tabs, write them as tabs + if byte > 0 { + write_tabs(output, ts, 0, col, pctype == CharType::Tab, true, true)?; + } + + // Write the rest of the line directly (no more tab conversion needed) + if byte < buf.len() { + output.write_all(&buf[byte..])?; + } + buf.truncate(0); + return Ok(()); + } + while byte < buf.len() { // when we have a finite number of columns, never convert past the last column if lastcol > 0 && col >= lastcol { @@ -379,7 +424,6 @@ fn unexpand_line( // write out anything remaining write_tabs(output, ts, scol, col, pctype == CharType::Tab, init, true)?; - output.flush()?; buf.truncate(0); // clear out the buffer Ok(()) @@ -407,6 +451,7 @@ fn unexpand(options: &Options) -> UResult<()> { unexpand_line(&mut buf, &mut output, options, lastcol, ts)?; } } + output.flush()?; Ok(()) } diff --git a/src/uucore/build.rs b/src/uucore/build.rs index ded60b65c..d5637ef3f 100644 --- a/src/uucore/build.rs +++ b/src/uucore/build.rs @@ -69,6 +69,9 @@ fn project_root() -> Result> { fn detect_target_utility() -> Option { use std::fs; + // Tell Cargo to rerun if this environment variable changes + println!("cargo:rerun-if-env-changed=UUCORE_TARGET_UTIL"); + // First check if an explicit environment variable was set if let Ok(target_util) = env::var("UUCORE_TARGET_UTIL") { if !target_util.is_empty() { diff --git a/src/uucore/src/lib/features.rs b/src/uucore/src/lib/features.rs index c7edd9a05..ac03fb79d 100644 --- a/src/uucore/src/lib/features.rs +++ b/src/uucore/src/lib/features.rs @@ -67,7 +67,7 @@ pub mod pipes; pub mod proc_info; #[cfg(all(unix, feature = "process"))] pub mod process; -#[cfg(all(target_os = "linux", feature = "safe-traversal"))] +#[cfg(target_os = "linux")] pub mod safe_traversal; #[cfg(all(target_os = "linux", feature = "tty"))] pub mod tty; diff --git a/src/uucore/src/lib/features/buf_copy/common.rs b/src/uucore/src/lib/features/buf_copy/common.rs index 82ae815f3..d771ff6be 100644 --- a/src/uucore/src/lib/features/buf_copy/common.rs +++ b/src/uucore/src/lib/features/buf_copy/common.rs @@ -15,8 +15,8 @@ pub enum Error { impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Error::WriteError(msg) => write!(f, "splice() write error: {msg}"), - Error::Io(err) => write!(f, "I/O error: {err}"), + Self::WriteError(msg) => write!(f, "splice() write error: {msg}"), + Self::Io(err) => write!(f, "I/O error: {err}"), } } } diff --git a/src/uucore/src/lib/features/checksum.rs b/src/uucore/src/lib/features/checksum.rs index 159620418..b878ce084 100644 --- a/src/uucore/src/lib/features/checksum.rs +++ b/src/uucore/src/lib/features/checksum.rs @@ -319,9 +319,9 @@ impl FileChecksumResult { /// either succeeded or failed. fn from_bool(checksum_correct: bool) -> Self { if checksum_correct { - FileChecksumResult::Ok + Self::Ok } else { - FileChecksumResult::Failed + Self::Failed } } @@ -329,9 +329,9 @@ impl FileChecksumResult { /// comparison on STDOUT. fn can_display(&self, verbose: ChecksumVerbose) -> bool { match self { - FileChecksumResult::Ok => verbose.over_quiet(), - FileChecksumResult::Failed => verbose.over_status(), - FileChecksumResult::CantOpen => true, + Self::Ok => verbose.over_quiet(), + Self::Failed => verbose.over_status(), + Self::CantOpen => true, } } } @@ -339,9 +339,9 @@ impl FileChecksumResult { impl Display for FileChecksumResult { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - FileChecksumResult::Ok => write!(f, "OK"), - FileChecksumResult::Failed => write!(f, "FAILED"), - FileChecksumResult::CantOpen => write!(f, "FAILED open or read"), + Self::Ok => write!(f, "OK"), + Self::Failed => write!(f, "FAILED"), + Self::CantOpen => write!(f, "FAILED open or read"), } } } @@ -557,7 +557,7 @@ impl LineFormat { algo_bit_len: algo_bits, checksum: checksum_utf8, filename: filename.to_vec(), - format: LineFormat::AlgoBased, + format: Self::AlgoBased, }) } @@ -587,7 +587,7 @@ impl LineFormat { algo_bit_len: None, checksum: checksum_utf8, filename: filename.to_vec(), - format: LineFormat::Untagged, + format: Self::Untagged, }) } @@ -619,7 +619,7 @@ impl LineFormat { algo_bit_len: None, checksum: checksum_utf8, filename: filename.to_vec(), - format: LineFormat::SingleSpace, + format: Self::SingleSpace, }) } } diff --git a/src/uucore/src/lib/features/encoding.rs b/src/uucore/src/lib/features/encoding.rs index 566dfe19f..6a2dccd4f 100644 --- a/src/uucore/src/lib/features/encoding.rs +++ b/src/uucore/src/lib/features/encoding.rs @@ -5,6 +5,7 @@ // spell-checker:ignore (encodings) lsbf msbf // spell-checker:ignore unpadded +// spell-checker:ignore ABCDEFGHJKLMNPQRSTUVWXY Zabcdefghijkmnopqrstuvwxyz use crate::error::{UResult, USimpleError}; use base64_simd; @@ -105,6 +106,7 @@ pub enum Format { Base2Lsbf, Base2Msbf, Z85, + Base58, } pub const BASE2LSBF: Encoding = new_encoding! { @@ -119,6 +121,8 @@ pub const BASE2MSBF: Encoding = new_encoding! { pub struct Z85Wrapper {} +pub struct Base58Wrapper {} + pub struct EncodingWrapper { pub alphabet: &'static [u8], pub encoding: Encoding, @@ -181,6 +185,159 @@ pub trait SupportsFastDecodeAndEncode { fn valid_decoding_multiple(&self) -> usize; } +impl SupportsFastDecodeAndEncode for Base58Wrapper { + fn alphabet(&self) -> &'static [u8] { + // Base58 alphabet + b"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" + } + + fn decode_into_vec(&self, input: &[u8], output: &mut Vec) -> UResult<()> { + if input.is_empty() { + return Ok(()); + } + + // Count leading zeros (will become leading 1s in base58) + let leading_ones = input.iter().take_while(|&&b| b == b'1').count(); + + // Skip leading 1s for conversion + let input_trimmed = &input[leading_ones..]; + if input_trimmed.is_empty() { + output.resize(output.len() + leading_ones, 0); + return Ok(()); + } + + // Convert base58 to big integer + let mut num: Vec = vec![0]; + let alphabet = self.alphabet(); + + for &byte in input_trimmed { + // Find position in alphabet + let digit = alphabet + .iter() + .position(|&b| b == byte) + .ok_or_else(|| USimpleError::new(1, "error: invalid input".to_owned()))?; + + // Multiply by 58 and add digit + let mut carry = digit as u32; + for n in &mut num { + let tmp = (*n as u64) * 58 + carry as u64; + *n = tmp as u32; + carry = (tmp >> 32) as u32; + } + if carry > 0 { + num.push(carry); + } + } + + // Convert to bytes (little endian, then reverse) + let mut result = Vec::new(); + for &n in &num { + result.extend_from_slice(&n.to_le_bytes()); + } + + // Remove trailing zeros and reverse to get big endian + while result.last() == Some(&0) && result.len() > 1 { + result.pop(); + } + result.reverse(); + + // Add leading zeros for leading 1s in input + let mut final_result = vec![0; leading_ones]; + final_result.extend_from_slice(&result); + + output.extend_from_slice(&final_result); + Ok(()) + } + + fn encode_to_vec_deque(&self, input: &[u8], output: &mut VecDeque) -> UResult<()> { + if input.is_empty() { + return Ok(()); + } + + // Count leading zeros + let leading_zeros = input.iter().take_while(|&&b| b == 0).count(); + + // Skip leading zeros + let input_trimmed = &input[leading_zeros..]; + if input_trimmed.is_empty() { + for _ in 0..leading_zeros { + output.push_back(b'1'); + } + return Ok(()); + } + + // Convert bytes to big integer (Vec in little-endian format) + let mut num = Vec::with_capacity(input_trimmed.len().div_ceil(4) + 1); + for &byte in input_trimmed { + let mut carry = byte as u64; + for n in &mut num { + let tmp = (*n as u64) * 256 + carry; + *n = tmp as u32; + carry = tmp >> 32; + } + if carry > 0 { + num.push(carry as u32); + } + } + + // Convert to base58 + let mut result = Vec::with_capacity((input_trimmed.len() * 138 / 100) + 1); + let alphabet = self.alphabet(); + + // Optimized check: stop when all elements are zero + while !num.is_empty() { + // Check if we're done (all zeros) + let mut all_zero = true; + let mut carry = 0u64; + + for n in num.iter_mut().rev() { + let tmp = carry * (1u64 << 32) + *n as u64; + *n = (tmp / 58) as u32; + carry = tmp % 58; + if *n != 0 { + all_zero = false; + } + } + + result.push(alphabet[carry as usize]); + + if all_zero { + break; + } + + // Trim trailing zeros less frequently + if num.len() > 1 && result.len() % 8 == 0 { + while num.last() == Some(&0) && num.len() > 1 { + num.pop(); + } + } + } + + // Add leading 1s for leading zeros in input + for _ in 0..leading_zeros { + output.push_back(b'1'); + } + + // Add result (reversed because we built it backwards) + for &byte in result.iter().rev() { + output.push_back(byte); + } + + Ok(()) + } + + fn unpadded_multiple(&self) -> usize { + // Base58 must encode the entire input as one big integer, not in chunks + // Use a very large value to effectively disable chunking, but avoid overflow + // when multiplied by ENCODE_IN_CHUNKS_OF_SIZE_MULTIPLE (1024) in base_common + usize::MAX / 2048 + } + + fn valid_decoding_multiple(&self) -> usize { + 1 // Any length is valid for Base58 + } +} + impl SupportsFastDecodeAndEncode for Z85Wrapper { fn alphabet(&self) -> &'static [u8] { // Z85 alphabet diff --git a/src/uucore/src/lib/features/extendedbigdecimal.rs b/src/uucore/src/lib/features/extendedbigdecimal.rs index 5748b6f1a..d119da4f7 100644 --- a/src/uucore/src/lib/features/extendedbigdecimal.rs +++ b/src/uucore/src/lib/features/extendedbigdecimal.rs @@ -83,20 +83,20 @@ impl From for ExtendedBigDecimal { fn from(val: f64) -> Self { if val.is_nan() { if val.is_sign_negative() { - ExtendedBigDecimal::MinusNan + Self::MinusNan } else { - ExtendedBigDecimal::Nan + Self::Nan } } else if val.is_infinite() { if val.is_sign_negative() { - ExtendedBigDecimal::MinusInfinity + Self::MinusInfinity } else { - ExtendedBigDecimal::Infinity + Self::Infinity } } else if val.is_zero() && val.is_sign_negative() { - ExtendedBigDecimal::MinusZero + Self::MinusZero } else { - ExtendedBigDecimal::BigDecimal(BigDecimal::from_f64(val).unwrap()) + Self::BigDecimal(BigDecimal::from_f64(val).unwrap()) } } } @@ -124,7 +124,7 @@ impl ExtendedBigDecimal { pub fn to_biguint(&self) -> Option { match self { - ExtendedBigDecimal::BigDecimal(big_decimal) => { + Self::BigDecimal(big_decimal) => { let (bi, scale) = big_decimal.as_bigint_and_scale(); if bi.is_negative() || scale > 0 || scale < -(u32::MAX as i64) { return None; diff --git a/src/uucore/src/lib/features/format/escape.rs b/src/uucore/src/lib/features/format/escape.rs index da6e691ea..cba03a8a6 100644 --- a/src/uucore/src/lib/features/format/escape.rs +++ b/src/uucore/src/lib/features/format/escape.rs @@ -35,8 +35,8 @@ enum Base { impl Base { fn as_base(&self) -> u8 { match self { - Base::Oct(_) => 8, - Base::Hex => 16, + Self::Oct(_) => 8, + Self::Hex => 16, } } diff --git a/src/uucore/src/lib/features/format/mod.rs b/src/uucore/src/lib/features/format/mod.rs index 532af34ef..1741340c4 100644 --- a/src/uucore/src/lib/features/format/mod.rs +++ b/src/uucore/src/lib/features/format/mod.rs @@ -84,8 +84,8 @@ impl From for FormatError { } impl From for FormatError { - fn from(value: NonUtf8OsStrError) -> FormatError { - FormatError::InvalidEncoding(value) + fn from(value: NonUtf8OsStrError) -> Self { + Self::InvalidEncoding(value) } } diff --git a/src/uucore/src/lib/features/fsext.rs b/src/uucore/src/lib/features/fsext.rs index fa770723e..6d851f1fe 100644 --- a/src/uucore/src/lib/features/fsext.rs +++ b/src/uucore/src/lib/features/fsext.rs @@ -130,10 +130,10 @@ impl From<&str> for MetadataTimeField { /// not supported), and the default branch should not be reached. fn from(value: &str) -> Self { match value { - "ctime" | "status" => MetadataTimeField::Change, - "access" | "atime" | "use" => MetadataTimeField::Access, - "mtime" | "modification" => MetadataTimeField::Modification, - "birth" | "creation" => MetadataTimeField::Birth, + "ctime" | "status" => Self::Change, + "access" | "atime" | "use" => Self::Access, + "mtime" | "modification" => Self::Modification, + "birth" | "creation" => Self::Birth, // below should never happen as clap already restricts the values. _ => unreachable!("Invalid metadata time field."), } diff --git a/src/uucore/src/lib/features/parser/num_parser.rs b/src/uucore/src/lib/features/parser/num_parser.rs index 5f7d89538..178cd578f 100644 --- a/src/uucore/src/lib/features/parser/num_parser.rs +++ b/src/uucore/src/lib/features/parser/num_parser.rs @@ -156,12 +156,10 @@ where } match self { - ExtendedParserError::NotNumeric => ExtendedParserError::NotNumeric, - ExtendedParserError::PartialMatch(v, rest) => { - ExtendedParserError::PartialMatch(extract(f(v)), rest) - } - ExtendedParserError::Overflow(v) => ExtendedParserError::Overflow(extract(f(v))), - ExtendedParserError::Underflow(v) => ExtendedParserError::Underflow(extract(f(v))), + Self::NotNumeric => ExtendedParserError::NotNumeric, + Self::PartialMatch(v, rest) => ExtendedParserError::PartialMatch(extract(f(v)), rest), + Self::Overflow(v) => ExtendedParserError::Overflow(extract(f(v))), + Self::Underflow(v) => ExtendedParserError::Underflow(extract(f(v))), } } } @@ -179,7 +177,7 @@ pub trait ExtendedParser { impl ExtendedParser for i64 { /// Parse a number as i64. No fractional part is allowed. - fn extended_parse(input: &str) -> Result> { + fn extended_parse(input: &str) -> Result> { fn into_i64(ebd: ExtendedBigDecimal) -> Result> { match ebd { ExtendedBigDecimal::BigDecimal(bd) => { @@ -214,7 +212,7 @@ impl ExtendedParser for i64 { impl ExtendedParser for u64 { /// Parse a number as u64. No fractional part is allowed. - fn extended_parse(input: &str) -> Result> { + fn extended_parse(input: &str) -> Result> { fn into_u64(ebd: ExtendedBigDecimal) -> Result> { match ebd { ExtendedBigDecimal::BigDecimal(bd) => { @@ -251,7 +249,7 @@ impl ExtendedParser for u64 { impl ExtendedParser for f64 { /// Parse a number as f64 - fn extended_parse(input: &str) -> Result> { + fn extended_parse(input: &str) -> Result> { fn into_f64(ebd: ExtendedBigDecimal) -> Result> { // TODO: _Some_ of this is generic, so this should probably be implemented as an ExtendedBigDecimal trait (ToPrimitive). let v = match ebd { @@ -283,9 +281,7 @@ impl ExtendedParser for f64 { impl ExtendedParser for ExtendedBigDecimal { /// Parse a number as an ExtendedBigDecimal - fn extended_parse( - input: &str, - ) -> Result> { + fn extended_parse(input: &str) -> Result> { parse(input, ParseTarget::Decimal, &[]) } } diff --git a/src/uucore/src/lib/features/parser/parse_time.rs b/src/uucore/src/lib/features/parser/parse_time.rs index 60412d713..4abf85a1d 100644 --- a/src/uucore/src/lib/features/parser/parse_time.rs +++ b/src/uucore/src/lib/features/parser/parse_time.rs @@ -13,9 +13,9 @@ use crate::{ extendedbigdecimal::ExtendedBigDecimal, parser::num_parser::{self, ExtendedParserError, ParseTarget}, }; -use num_traits::Signed; use num_traits::ToPrimitive; use num_traits::Zero; +use num_traits::{FromPrimitive, Signed}; use std::time::Duration; /// Parse a duration from a string. @@ -86,6 +86,10 @@ pub fn from_str(string: &str, allow_suffixes: bool) -> Result // potentially expensive to-nanoseconds conversion return Ok(Duration::MAX); } + // early return if number is too small (< 1 ns) + if !bd.is_zero() && bd < bigdecimal::BigDecimal::from_f64(0.0000000001).unwrap() { + return Ok(NANOSECOND_DURATION); + } bd } ExtendedBigDecimal::MinusZero => 0.into(), @@ -165,6 +169,10 @@ mod tests { from_str("1e-92233720368547758080", false), Ok(NANOSECOND_DURATION) ); + assert_eq!( + from_str("0x6p-4376646810043701", false), + Ok(NANOSECOND_DURATION) + ); // nanoseconds underflow (in Duration, false) assert_eq!(from_str("0.0000000001", false), Ok(NANOSECOND_DURATION)); assert_eq!(from_str("1e-10", false), Ok(NANOSECOND_DURATION)); diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index f915d13dc..a67c27e36 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -5,7 +5,7 @@ //! Common functions to manage permissions -// spell-checker:ignore (jargon) TOCTOU fchownat +// spell-checker:ignore (jargon) TOCTOU fchownat fchown use crate::display::Quotable; use crate::error::{UResult, USimpleError, strip_errno}; @@ -18,10 +18,10 @@ use libc::{gid_t, uid_t}; use options::traverse; use std::ffi::OsString; -#[cfg(not(all(target_os = "linux", feature = "safe-traversal")))] +#[cfg(not(target_os = "linux"))] use walkdir::WalkDir; -#[cfg(all(target_os = "linux", feature = "safe-traversal"))] +#[cfg(target_os = "linux")] use crate::features::safe_traversal::DirFd; use std::ffi::CString; @@ -307,14 +307,43 @@ impl ChownExecutor { } let ret = if self.matched(meta.uid(), meta.gid()) { - match wrap_chown( + // Use safe syscalls for root directory to prevent TOCTOU attacks on Linux + #[cfg(target_os = "linux")] + let chown_result = if path.is_dir() { + // For directories on Linux, use safe traversal from the start + match DirFd::open(path) { + Ok(dir_fd) => self + .safe_chown_dir(&dir_fd, path, &meta) + .map(|_| String::new()), + Err(_e) => { + // Don't show error here - let safe_dive_into handle directory traversal errors + // This prevents duplicate error messages + Ok(String::new()) + } + } + } else { + // For non-directories (files, symlinks), use the regular wrap_chown method + wrap_chown( + path, + &meta, + self.dest_uid, + self.dest_gid, + self.dereference, + self.verbosity.clone(), + ) + }; + + #[cfg(not(target_os = "linux"))] + let chown_result = wrap_chown( path, &meta, self.dest_uid, self.dest_gid, self.dereference, self.verbosity.clone(), - ) { + ); + + match chown_result { Ok(n) => { if !n.is_empty() { show_error!("{n}"); @@ -338,11 +367,11 @@ impl ChownExecutor { }; if self.recursive { - #[cfg(all(target_os = "linux", feature = "safe-traversal"))] + #[cfg(target_os = "linux")] { ret | self.safe_dive_into(&root) } - #[cfg(not(all(target_os = "linux", feature = "safe-traversal")))] + #[cfg(not(target_os = "linux"))] { ret | self.dive_into(&root) } @@ -351,7 +380,56 @@ impl ChownExecutor { } } - #[cfg(all(target_os = "linux", feature = "safe-traversal"))] + #[cfg(target_os = "linux")] + fn safe_chown_dir(&self, dir_fd: &DirFd, path: &Path, meta: &Metadata) -> Result<(), String> { + let dest_uid = self.dest_uid.unwrap_or_else(|| meta.uid()); + let dest_gid = self.dest_gid.unwrap_or_else(|| meta.gid()); + + // Use fchown (safe) to change the directory's ownership + if let Err(e) = dir_fd.fchown(self.dest_uid, self.dest_gid) { + let mut error_msg = format!( + "changing {} of {}: {}", + if self.verbosity.groups_only { + "group" + } else { + "ownership" + }, + path.quote(), + e + ); + + if self.verbosity.level == VerbosityLevel::Verbose { + error_msg = if self.verbosity.groups_only { + let gid = meta.gid(); + format!( + "{error_msg}\nfailed to change group of {} from {} to {}", + path.quote(), + entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()), + entries::gid2grp(dest_gid).unwrap_or_else(|_| dest_gid.to_string()) + ) + } else { + let uid = meta.uid(); + let gid = meta.gid(); + format!( + "{error_msg}\nfailed to change ownership of {} from {}:{} to {}:{}", + path.quote(), + entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()), + entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()), + entries::uid2usr(dest_uid).unwrap_or_else(|_| dest_uid.to_string()), + entries::gid2grp(dest_gid).unwrap_or_else(|_| dest_gid.to_string()) + ) + }; + } + + return Err(error_msg); + } + + // Report the change if verbose (similar to wrap_chown) + self.report_ownership_change_success(path, meta.uid(), meta.gid()); + Ok(()) + } + + #[cfg(target_os = "linux")] fn safe_dive_into>(&self, root: P) -> i32 { let root = root.as_ref(); @@ -377,7 +455,7 @@ impl ChownExecutor { ret } - #[cfg(all(target_os = "linux", feature = "safe-traversal"))] + #[cfg(target_os = "linux")] fn safe_traverse_dir(&self, dir_fd: &DirFd, dir_path: &Path, ret: &mut i32) { // Read directory entries let entries = match dir_fd.read_dir() { @@ -482,7 +560,7 @@ impl ChownExecutor { } } - #[cfg(not(all(target_os = "linux", feature = "safe-traversal")))] + #[cfg(not(target_os = "linux"))] #[allow(clippy::cognitive_complexity)] fn dive_into>(&self, root: P) -> i32 { let root = root.as_ref(); @@ -619,7 +697,7 @@ impl ChownExecutor { } /// Try to open directory with error reporting - #[cfg(all(target_os = "linux", feature = "safe-traversal"))] + #[cfg(target_os = "linux")] fn try_open_dir(&self, path: &Path) -> Option { DirFd::open(path) .map_err(|e| { @@ -632,7 +710,7 @@ impl ChownExecutor { /// Report ownership change with proper verbose output /// Returns 0 on success - #[cfg(all(target_os = "linux", feature = "safe-traversal"))] + #[cfg(target_os = "linux")] fn report_ownership_change_success( &self, path: &Path, diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index 405f90120..a405ea5d9 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -11,8 +11,6 @@ // spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile // spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod -#![cfg(target_os = "linux")] - #[cfg(test)] use std::os::unix::ffi::OsStringExt; @@ -24,6 +22,7 @@ use std::path::Path; use nix::dir::Dir; use nix::fcntl::{OFlag, openat}; +use nix::libc; use nix::sys::stat::{FchmodatFlags, FileStat, Mode, fchmodat, fstatat}; use nix::unistd::{Gid, Uid, UnlinkatFlags, fchown, fchownat, unlinkat}; @@ -67,7 +66,7 @@ pub enum SafeTraversalError { impl From for io::Error { fn from(err: SafeTraversalError) -> Self { match err { - SafeTraversalError::PathContainsNull => io::Error::new( + SafeTraversalError::PathContainsNull => Self::new( io::ErrorKind::InvalidInput, translate!("safe-traversal-error-path-contains-null"), ), @@ -118,7 +117,7 @@ impl DirFd { } })?; - Ok(DirFd { fd }) + Ok(Self { fd }) } /// Open a subdirectory relative to this directory @@ -134,7 +133,7 @@ impl DirFd { } })?; - Ok(DirFd { fd }) + Ok(Self { fd }) } /// Get raw stat data for a file relative to this directory @@ -285,7 +284,7 @@ impl DirFd { } // SAFETY: We've verified fd >= 0, and the caller is transferring ownership let owned_fd = unsafe { OwnedFd::from_raw_fd(fd) }; - Ok(DirFd { fd: owned_fd }) + Ok(Self { fd: owned_fd }) } } @@ -346,23 +345,23 @@ pub enum FileType { impl FileType { pub fn from_mode(mode: libc::mode_t) -> Self { match mode & libc::S_IFMT { - libc::S_IFDIR => FileType::Directory, - libc::S_IFREG => FileType::RegularFile, - libc::S_IFLNK => FileType::Symlink, - _ => FileType::Other, + libc::S_IFDIR => Self::Directory, + libc::S_IFREG => Self::RegularFile, + libc::S_IFLNK => Self::Symlink, + _ => Self::Other, } } pub fn is_directory(&self) -> bool { - matches!(self, FileType::Directory) + matches!(self, Self::Directory) } pub fn is_regular_file(&self) -> bool { - matches!(self, FileType::RegularFile) + matches!(self, Self::RegularFile) } pub fn is_symlink(&self) -> bool { - matches!(self, FileType::Symlink) + matches!(self, Self::Symlink) } } diff --git a/src/uucore/src/lib/features/selinux.rs b/src/uucore/src/lib/features/selinux.rs index 1f2b6452c..e5bdf8ebc 100644 --- a/src/uucore/src/lib/features/selinux.rs +++ b/src/uucore/src/lib/features/selinux.rs @@ -31,7 +31,7 @@ pub enum SeLinuxError { } impl From for i32 { - fn from(error: SeLinuxError) -> i32 { + fn from(error: SeLinuxError) -> Self { match error { SeLinuxError::SELinuxNotEnabled => 1, SeLinuxError::FileOpenFailure(_) => 2, diff --git a/src/uucore/src/lib/features/systemd_logind.rs b/src/uucore/src/lib/features/systemd_logind.rs index baf4881f4..a59db3b1c 100644 --- a/src/uucore/src/lib/features/systemd_logind.rs +++ b/src/uucore/src/lib/features/systemd_logind.rs @@ -524,7 +524,7 @@ pub struct SystemdUtmpxCompat { impl SystemdUtmpxCompat { /// Create new instance from a SystemdLoginRecord pub fn new(record: SystemdLoginRecord) -> Self { - SystemdUtmpxCompat { record } + Self { record } } /// A.K.A. ut.ut_type @@ -596,7 +596,7 @@ impl SystemdUtmpxIter { /// Create new instance and read records from systemd-logind pub fn new() -> UResult { let records = read_login_records()?; - Ok(SystemdUtmpxIter { + Ok(Self { records, current_index: 0, }) @@ -604,7 +604,7 @@ impl SystemdUtmpxIter { /// Create empty iterator (for when systemd initialization fails) pub fn empty() -> Self { - SystemdUtmpxIter { + Self { records: Vec::new(), current_index: 0, } diff --git a/src/uucore/src/lib/features/utmpx.rs b/src/uucore/src/lib/features/utmpx.rs index 3b84a17d3..3c62ba9bc 100644 --- a/src/uucore/src/lib/features/utmpx.rs +++ b/src/uucore/src/lib/features/utmpx.rs @@ -412,63 +412,63 @@ impl UtmpxRecord { /// A.K.A. ut.ut_type pub fn record_type(&self) -> i16 { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.record_type(), + Self::Traditional(utmpx) => utmpx.record_type(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.record_type(), + Self::Systemd(systemd) => systemd.record_type(), } } /// A.K.A. ut.ut_pid pub fn pid(&self) -> i32 { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.pid(), + Self::Traditional(utmpx) => utmpx.pid(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.pid(), + Self::Systemd(systemd) => systemd.pid(), } } /// A.K.A. ut.ut_id pub fn terminal_suffix(&self) -> String { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.terminal_suffix(), + Self::Traditional(utmpx) => utmpx.terminal_suffix(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.terminal_suffix(), + Self::Systemd(systemd) => systemd.terminal_suffix(), } } /// A.K.A. ut.ut_user pub fn user(&self) -> String { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.user(), + Self::Traditional(utmpx) => utmpx.user(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.user(), + Self::Systemd(systemd) => systemd.user(), } } /// A.K.A. ut.ut_host pub fn host(&self) -> String { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.host(), + Self::Traditional(utmpx) => utmpx.host(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.host(), + Self::Systemd(systemd) => systemd.host(), } } /// A.K.A. ut.ut_line pub fn tty_device(&self) -> String { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.tty_device(), + Self::Traditional(utmpx) => utmpx.tty_device(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.tty_device(), + Self::Systemd(systemd) => systemd.tty_device(), } } /// A.K.A. ut.ut_tv pub fn login_time(&self) -> time::OffsetDateTime { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.login_time(), + Self::Traditional(utmpx) => utmpx.login_time(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.login_time(), + Self::Systemd(systemd) => systemd.login_time(), } } @@ -477,27 +477,27 @@ impl UtmpxRecord { /// Return (e_termination, e_exit) pub fn exit_status(&self) -> (i16, i16) { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.exit_status(), + Self::Traditional(utmpx) => utmpx.exit_status(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.exit_status(), + Self::Systemd(systemd) => systemd.exit_status(), } } /// check if the record is a user process pub fn is_user_process(&self) -> bool { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.is_user_process(), + Self::Traditional(utmpx) => utmpx.is_user_process(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.is_user_process(), + Self::Systemd(systemd) => systemd.is_user_process(), } } /// Canonicalize host name using DNS pub fn canon_host(&self) -> IOResult { match self { - UtmpxRecord::Traditional(utmpx) => utmpx.canon_host(), + Self::Traditional(utmpx) => utmpx.canon_host(), #[cfg(feature = "feat_systemd_logind")] - UtmpxRecord::Systemd(systemd) => systemd.canon_host(), + Self::Systemd(systemd) => systemd.canon_host(), } } } diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 91c9f001a..47da8296f 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -92,7 +92,7 @@ pub use crate::features::perms; pub use crate::features::pipes; #[cfg(all(unix, feature = "process"))] pub use crate::features::process; -#[cfg(all(target_os = "linux", feature = "safe-traversal"))] +#[cfg(target_os = "linux")] pub use crate::features::safe_traversal; #[cfg(all(unix, not(target_os = "fuchsia"), feature = "signals"))] pub use crate::features::signals; @@ -559,19 +559,19 @@ pub enum CharByte { impl From for CharByte { fn from(value: char) -> Self { - CharByte::Char(value) + Self::Char(value) } } impl From for CharByte { fn from(value: u8) -> Self { - CharByte::Byte(value) + Self::Byte(value) } } impl From<&u8> for CharByte { fn from(value: &u8) -> Self { - CharByte::Byte(*value) + Self::Byte(*value) } } @@ -588,7 +588,7 @@ impl Iterator for Utf8ChunkIterator<'_> { } impl<'a> From> for Utf8ChunkIterator<'a> { - fn from(chk: Utf8Chunk<'a>) -> Utf8ChunkIterator<'a> { + fn from(chk: Utf8Chunk<'a>) -> Self { Self { iter: Box::new( chk.valid() @@ -609,7 +609,7 @@ pub struct CharByteIterator<'a> { impl<'a> CharByteIterator<'a> { /// Make a `CharByteIterator` from a byte slice. /// [`CharByteIterator`] - pub fn new(input: &'a [u8]) -> CharByteIterator<'a> { + pub fn new(input: &'a [u8]) -> Self { Self { iter: Box::new(input.utf8_chunks().flat_map(Utf8ChunkIterator::from)), } diff --git a/src/uucore/src/lib/mods/clap_localization.rs b/src/uucore/src/lib/mods/clap_localization.rs index 82b38e6af..5a54bf7c3 100644 --- a/src/uucore/src/lib/mods/clap_localization.rs +++ b/src/uucore/src/lib/mods/clap_localization.rs @@ -31,9 +31,9 @@ pub enum Color { impl Color { fn code(self) -> &'static str { match self { - Color::Red => "31", - Color::Yellow => "33", - Color::Green => "32", + Self::Red => "31", + Self::Yellow => "33", + Self::Green => "32", } } } diff --git a/src/uucore/src/lib/mods/locale.rs b/src/uucore/src/lib/mods/locale.rs index 09b5bbf33..97606eeca 100644 --- a/src/uucore/src/lib/mods/locale.rs +++ b/src/uucore/src/lib/mods/locale.rs @@ -42,7 +42,7 @@ pub enum LocalizationError { impl From for LocalizationError { fn from(error: std::io::Error) -> Self { - LocalizationError::Io { + Self::Io { source: error, path: PathBuf::from(""), } diff --git a/tests/by-util/test_basenc.rs b/tests/by-util/test_basenc.rs index 52acd35d4..f02de772b 100644 --- a/tests/by-util/test_basenc.rs +++ b/tests/by-util/test_basenc.rs @@ -185,21 +185,55 @@ fn test_base2lsbf_decode() { } #[test] -fn test_choose_last_encoding_z85() { +fn test_z85_decode() { new_ucmd!() - .args(&[ - "--base2lsbf", - "--base2msbf", - "--base16", - "--base32hex", - "--base64url", - "--base32", - "--base64", - "--z85", - ]) - .pipe_in("Hello, World") + .args(&["--z85", "-d"]) + .pipe_in("nm=QNz.92jz/PV8") .succeeds() - .stdout_only("nm=QNz.92jz/PV8\n"); + .stdout_only("Hello, World"); +} + +#[test] +fn test_base58() { + new_ucmd!() + .arg("--base58") + .pipe_in("Hello, World!") + .succeeds() + .stdout_only("72k1xXWG59fYdzSNoA\n"); +} + +#[test] +fn test_base58_decode() { + new_ucmd!() + .args(&["--base58", "-d"]) + .pipe_in("72k1xXWG59fYdzSNoA") + .succeeds() + .stdout_only("Hello, World!"); +} + +#[test] +fn test_base58_large_file_no_chunking() { + // Regression test: base58 must process entire input as one big integer, + // not in 1024-byte chunks. This test ensures files >1024 bytes work correctly. + let (at, mut ucmd) = at_and_ucmd!(); + let filename = "large_file.txt"; + + // spell-checker:disable + let input = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. ".repeat(50); + // spell-checker:enable + at.write(filename, &input); + + let result = ucmd.arg("--base58").arg(filename).succeeds(); + let encoded = result.stdout_str(); + + // Verify the output ends with the expected suffix (matches GNU basenc output) + // spell-checker:disable + assert!( + encoded + .trim_end() + .ends_with("ZNRRacEnhrY83ZEYkpwWVZNFK5DFRasr\nw693NsNGtiQ9fYAj") + ); + // spell-checker:enable } #[test] @@ -238,6 +272,15 @@ fn test_choose_last_encoding_base2lsbf() { .stdout_only("00110110110011100100011001100110\n"); } +#[test] +fn test_choose_last_encoding_base58() { + new_ucmd!() + .args(&["--base64", "--base32", "--base16", "--z85", "--base58"]) + .pipe_in("Hello!") + .succeeds() + .stdout_only("d3yC1LKr\n"); +} + #[test] fn test_base32_decode_repeated() { new_ucmd!() diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 618d789a8..cd4f8ee73 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -7090,3 +7090,49 @@ fn test_cp_recursive_files_ending_in_backslash() { ts.ucmd().args(&["-r", "a", "b"]).succeeds(); assert!(at.file_exists("b/foo\\")); } + +#[test] +fn test_cp_no_preserve_target_directory() { + /* Expected result: + ├── a + │ └── b + │ └── c + │ └── d + │ └── f1 + ├── d + │ └── f1 + └── e + ├── b + │ └── c + │ └── d + │ ├── c + │ │ └── d + │ │ └── f1 + │ └── f1 + ├── d + │ └── f1 + ├── f2 + └── f3 + */ + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.mkdir_all("a/b/c/d"); + at.touch("a/b/c/d/f1"); + ts.ucmd().args(&["-rT", "a", "e"]).succeeds(); + at.touch("e/f2"); + ts.ucmd().args(&["-rT", "a/", "e/"]).succeeds(); + at.touch("e/f3"); + ts.ucmd().args(&["-rvT", "a/b/c", "e/"]).succeeds(); + ts.ucmd().args(&["-rvT", "a/b/", "e/b/c/d/"]).succeeds(); + ts.ucmd().args(&["-rT", "a/b/c", "."]).succeeds(); + assert!(!at.dir_exists("e/a")); + assert!(at.file_exists("e/b/c/d/f1")); + assert!(at.file_exists("e/b/c/d/c/d/f1")); + assert!(!at.dir_exists("e/c")); + assert!(!at.dir_exists("e/c/d/b")); + assert!(at.file_exists("e/d/f1")); + assert!(at.file_exists("./d/f1")); + assert!(at.file_exists("e/f2")); + assert!(at.file_exists("e/f3")); +} diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index 71f9b5716..d5a7bbfbb 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -793,6 +793,52 @@ fn test_symlink_remove_existing_same_src_and_dest() { assert_eq!(at.read("a"), "sample"); } +#[test] +fn test_force_same_file_detected_after_canonicalization() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file", "hello"); + + ucmd.args(&["-f", "file", "./file"]) + .fails_with_code(1) + .stderr_contains("are the same file"); + + assert!(at.file_exists("file")); + assert_eq!(at.read("file"), "hello"); +} + +#[test] +#[cfg(not(target_os = "android"))] +fn test_force_ln_existing_hard_link_entry() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.write("file", "hardlink\n"); + at.mkdir("dir"); + + scene.ucmd().args(&["file", "dir"]).succeeds().no_stderr(); + assert!(at.file_exists("dir/file")); + + scene + .ucmd() + .args(&["-f", "file", "dir"]) + .succeeds() + .no_stderr(); + + assert!(at.file_exists("file")); + assert!(at.file_exists("dir/file")); + assert_eq!(at.read("file"), "hardlink\n"); + assert_eq!(at.read("dir/file"), "hardlink\n"); + + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let source_inode = at.metadata("file").ino(); + let target_inode = at.metadata("dir/file").ino(); + assert_eq!(source_inode, target_inode); + } +} + #[test] #[cfg(not(target_os = "android"))] fn test_ln_seen_file() { diff --git a/tests/by-util/test_ptx.rs b/tests/by-util/test_ptx.rs index 917cd047a..3ff36a1c6 100644 --- a/tests/by-util/test_ptx.rs +++ b/tests/by-util/test_ptx.rs @@ -57,6 +57,30 @@ fn test_truncation_no_extra_space_in_after() { .stdout_contains(".xx \"\" \"Rust\" \"is/\" \"\""); } +#[test] +fn gnu_ext_disabled_reference_calculation() { + let input = "Hello World Rust is good language"; + let expected_output = concat!( + r#".xx "language" "" "Hello World Rust is good" "" ":1""#, + "\n", + r#".xx "" "Hello World" "Rust is good language" "" ":1""#, + "\n", + r#".xx "" "Hello" "World Rust is good language" "" ":1""#, + "\n", + r#".xx "" "Hello World Rust is" "good language" "" ":1""#, + "\n", + r#".xx "" "Hello World Rust" "is good language" "" ":1""#, + "\n", + r#".xx "" "Hello World Rust is good" "language" "" ":1""#, + "\n", + ); + new_ucmd!() + .args(&["-G", "-A"]) + .pipe_in(input) + .succeeds() + .stdout_only(expected_output); +} + #[test] fn gnu_ext_disabled_rightward_no_ref() { new_ucmd!() diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index db31ab876..9f8803865 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -1078,3 +1078,74 @@ fn test_rm_recursive_long_path_safe_traversal() { // Verify the directory is completely removed assert!(!at.dir_exists("rm_deep")); } + +#[cfg(all(not(windows), feature = "chmod"))] +#[test] +fn test_rm_directory_not_executable() { + // Test from GNU rm/rm2.sh + // Exercise code paths when directories have no execute permission + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create directory structure: a/0, a/1/2, a/2, a/3, b/3 + at.mkdir_all("a/0"); + at.mkdir_all("a/1/2"); + at.mkdir("a/2"); + at.mkdir("a/3"); + at.mkdir_all("b/3"); + + // Remove execute permission from a/1 and b + scene.ccmd("chmod").arg("u-x").arg("a/1").succeeds(); + scene.ccmd("chmod").arg("u-x").arg("b").succeeds(); + + // Try to remove both directories recursively - this should fail + let result = scene.ucmd().args(&["-rf", "a", "b"]).fails(); + + // Check for expected error messages + // When directories don't have execute permission, we get "Permission denied" + // when trying to access subdirectories + let stderr = result.stderr_str(); + assert!(stderr.contains("rm: cannot remove 'a/1/2': Permission denied")); + assert!(stderr.contains("rm: cannot remove 'b/3': Permission denied")); + + // Check which directories still exist + assert!(!at.dir_exists("a/0")); // Should be removed + assert!(at.dir_exists("a/1")); // Should still exist (no execute permission) + assert!(!at.dir_exists("a/2")); // Should be removed + assert!(!at.dir_exists("a/3")); // Should be removed + + // Restore execute permission to check b/3 + scene.ccmd("chmod").arg("u+x").arg("b").succeeds(); + assert!(at.dir_exists("b/3")); // Should still exist +} + +#[cfg(all(not(windows), feature = "chmod"))] +#[test] +fn test_rm_directory_not_writable() { + // Test from GNU rm/rm1.sh + // Exercise code paths when directories have no write permission + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create directory structure: b/a/p, b/c, b/d + at.mkdir_all("b/a/p"); + at.mkdir("b/c"); + at.mkdir("b/d"); + + // Remove write permission from b/a + scene.ccmd("chmod").arg("ug-w").arg("b/a").succeeds(); + + // Try to remove b recursively - this should fail + let result = scene.ucmd().args(&["-rf", "b"]).fails(); + + // Check for expected error message + // When the parent directory (b/a) doesn't have write permission, + // we get "Permission denied" when trying to remove the subdirectory + let stderr = result.stderr_str(); + assert!(stderr.contains("rm: cannot remove 'b/a/p': Permission denied")); + + // Check which directories still exist + assert!(at.dir_exists("b/a/p")); // Should still exist (parent not writable) + assert!(!at.dir_exists("b/c")); // Should be removed + assert!(!at.dir_exists("b/d")); // Should be removed +} diff --git a/tests/by-util/test_tee.rs b/tests/by-util/test_tee.rs index 10596f02c..ba6993371 100644 --- a/tests/by-util/test_tee.rs +++ b/tests/by-util/test_tee.rs @@ -623,3 +623,93 @@ mod linux_only { assert!(result.stderr_str().contains("No space left on device")); } } + +// Additional cross-platform tee tests to cover GNU compatibility around --output-error +#[test] +fn test_output_error_flag_without_value_defaults_warn_nopipe() { + // When --output-error is present without an explicit value, it should default to warn-nopipe + // We can't easily simulate a broken pipe across all platforms here, but we can ensure + // the flag is accepted without error and basic tee functionality still works. + let (at, mut ucmd) = at_and_ucmd!(); + let file_out = "tee_output_error_default.txt"; + let content = "abc"; + + let result = ucmd + .arg("--output-error") + .arg(file_out) + .pipe_in(content) + .succeeds(); + + result.stdout_is(content); + assert!(at.file_exists(file_out)); + assert_eq!(at.read(file_out), content); +} +// Unix-only: presence-only --output-error should not crash on broken pipe. +// Current implementation may exit zero; we only assert the process exits to avoid flakiness. +// TODO: When semantics are aligned with GNU warn-nopipe, strengthen assertions here. +#[cfg(all(unix, not(target_os = "freebsd")))] +#[test] +fn test_output_error_presence_only_broken_pipe_unix() { + use std::fs::File; + use std::os::unix::io::FromRawFd; + + unsafe { + let mut fds: [libc::c_int; 2] = [0, 0]; + assert_eq!(libc::pipe(fds.as_mut_ptr()), 0, "Failed to create pipe"); + // Close the read end to simulate a broken pipe on stdout + let _read_end = File::from_raw_fd(fds[0]); + let write_end = File::from_raw_fd(fds[1]); + + let content = (0..10_000).map(|_| "x").collect::(); + let result = new_ucmd!() + .arg("--output-error") // presence-only flag + .set_stdout(write_end) + .pipe_in(content.as_bytes()) + .run(); + + // Assert that a status was produced (i.e., process exited) and no crash occurred. + assert!(result.try_exit_status().is_some(), "process did not exit"); + } +} + +// Skip on FreeBSD due to repeated CI hangs in FreeBSD VM (see PR #8684) +#[cfg(all(unix, not(target_os = "freebsd")))] +#[test] +fn test_broken_pipe_early_termination_stdout_only() { + use std::fs::File; + use std::os::unix::io::FromRawFd; + + // Create a broken stdout by creating a pipe and dropping the read end + unsafe { + let mut fds: [libc::c_int; 2] = [0, 0]; + assert_eq!(libc::pipe(fds.as_mut_ptr()), 0, "Failed to create pipe"); + // Close the read end immediately to simulate a broken pipe + let _read_end = File::from_raw_fd(fds[0]); + let write_end = File::from_raw_fd(fds[1]); + + let content = (0..10_000).map(|_| "x").collect::(); + let mut proc = new_ucmd!(); + let result = proc + .set_stdout(write_end) + .ignore_stdin_write_error() + .pipe_in(content.as_bytes()) + .run(); + + // GNU tee exits nonzero on broken pipe unless configured otherwise; implementation + // details vary by mode, but we should not panic and should return an exit status. + // Assert that a status was produced (i.e., process exited) and no crash occurred. + assert!(result.try_exit_status().is_some(), "process did not exit"); + } +} + +#[test] +fn test_write_failure_reports_error_and_nonzero_exit() { + // Simulate a file open failure which should be reported via show_error and cause a failure + let (at, mut ucmd) = at_and_ucmd!(); + // Create a directory and try to use it as an output file (open will fail) + at.mkdir("out_dir"); + + let result = ucmd.arg("out_dir").pipe_in("data").fails(); + + assert!(!result.stderr_str().is_empty()); +} diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index e6752dcea..b04b32203 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -192,8 +192,7 @@ fn test_kill_subprocess() { "trap 'echo inside_trap' TERM; sleep 30", ]) .fails_with_code(124) - .stdout_contains("inside_trap") - .stderr_contains("Terminated"); + .stdout_contains("inside_trap"); } #[test] diff --git a/tests/by-util/test_unexpand.rs b/tests/by-util/test_unexpand.rs index 4439a3fc0..0f2a6d464 100644 --- a/tests/by-util/test_unexpand.rs +++ b/tests/by-util/test_unexpand.rs @@ -2,9 +2,10 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +// // spell-checker:ignore contenta -use uutests::at_and_ucmd; -use uutests::new_ucmd; + +use uutests::{at_and_ucmd, new_ucmd}; #[test] fn test_invalid_arg() { @@ -281,3 +282,16 @@ fn test_one_nonexisting_file() { .fails() .stderr_contains("asdf.txt: No such file or directory"); } + +#[test] +#[cfg(target_os = "linux")] +fn test_non_utf8_filename() { + use std::os::unix::ffi::OsStringExt; + + let (at, mut ucmd) = at_and_ucmd!(); + + let filename = std::ffi::OsString::from_vec(vec![0xFF, 0xFE]); + std::fs::write(at.plus(&filename), b" a\n").unwrap(); + + ucmd.arg(&filename).succeeds().stdout_is("\ta\n"); +} diff --git a/util/build-gnu.sh b/util/build-gnu.sh index db5832141..734088252 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -243,6 +243,10 @@ sed -i -e "s|removed directory 'a/'|removed directory 'a'|g" tests/rm/v-slash.sh # 'rel' doesn't exist. Our implementation is giving a better message. sed -i -e "s|rm: cannot remove 'rel': Permission denied|rm: cannot remove 'rel': No such file or directory|g" tests/rm/inaccessible.sh +# Our implementation shows "Directory not empty" for directories that can't be accessed due to lack of execute permissions +# This is actually more accurate than "Permission denied" since the real issue is that we can't empty the directory +sed -i -e "s|rm: cannot remove 'a/1': Permission denied|rm: cannot remove 'a/1/2': Permission denied|g" -e "s|rm: cannot remove 'b': Permission denied|rm: cannot remove 'a': Directory not empty\nrm: cannot remove 'b/3': Permission denied|g" tests/rm/rm2.sh + # overlay-headers.sh test intends to check for inotify events, # however there's a bug because `---dis` is an alias for: `---disable-inotify` sed -i -e "s|---dis ||g" tests/tail/overlay-headers.sh diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh new file mode 100755 index 000000000..ed3c5a78e --- /dev/null +++ b/util/check-safe-traversal.sh @@ -0,0 +1,227 @@ +#!/bin/bash +# +# Check that utilities are using safe traversal (openat family syscalls) +# to prevent TOCTOU race conditions +# + +set -e + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +TEMP_DIR=$(mktemp -d) + +# Function to exit immediately on error +fail_immediately() { + echo "❌ FAILED: $1" + echo "" + echo "Debug information available in: $TEMP_DIR/strace_*.log" + exit 1 +} + +cleanup() { + rm -rf "$TEMP_DIR" +} +trap cleanup EXIT + +echo "=== Safe Traversal Verification ===" + +# Assume binaries are already built (for CI usage) +# Prefer individual binaries for more accurate testing +if [ -f "$PROJECT_ROOT/target/release/rm" ]; then + echo "Using individual binaries" + USE_MULTICALL=0 +elif [ -f "$PROJECT_ROOT/target/release/coreutils" ]; then + echo "Using multicall binary" + USE_MULTICALL=1 + COREUTILS_BIN="$PROJECT_ROOT/target/release/coreutils" +else + echo "Error: No binaries found. Please build first with 'cargo build --release'" + exit 1 +fi + +cd "$TEMP_DIR" + +# Create test directory structure +mkdir -p test_dir/sub1/sub2/sub3 +echo "test1" > test_dir/file1.txt +echo "test2" > test_dir/sub1/file2.txt +echo "test3" > test_dir/sub1/sub2/file3.txt +echo "test4" > test_dir/sub1/sub2/sub3/file4.txt + +check_utility() { + local util="$1" + local trace_syscalls="$2" + local expected_syscalls="$3" + local test_args="$4" + local test_name="$5" + + echo "" + echo "Testing $util ($test_name)..." + + local strace_log="strace_${util}_${test_name}.log" + + # Choose binary to use + if [ "$USE_MULTICALL" -eq 1 ]; then + local util_cmd="$COREUTILS_BIN $util" + else + local util_path="$PROJECT_ROOT/target/release/$util" + if [ ! -f "$util_path" ]; then + fail_immediately "$util binary not found at $util_path" + fi + local util_cmd="$util_path" + fi + + # Run utility under strace + strace -f -e trace="$trace_syscalls" -o "$strace_log" \ + $util_cmd $test_args 2>/dev/null || true + cat $strace_log + # Check for expected safe syscalls + local found_safe=0 + for syscall in $expected_syscalls; do + if grep -q "$syscall" "$strace_log"; then + echo "✓ Found $syscall() (safe traversal)" + found_safe=$((found_safe + 1)) + else + fail_immediately "Missing $syscall() (safe traversal not active for $util)" + fi + done + + # Count detailed syscall statistics + local openat_count unlinkat_count fchmodat_count fchownat_count newfstatat_count renameat_count + local unlink_count rmdir_count chmod_count chown_count safe_ops unsafe_ops + + openat_count=$(grep -c "openat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + unlinkat_count=$(grep -c "unlinkat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + fchmodat_count=$(grep -c "fchmodat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + fchownat_count=$(grep -c "fchownat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + newfstatat_count=$(grep -c "newfstatat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + renameat_count=$(grep -c "renameat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + + # Count old unsafe syscalls (exclude the trace line prefix) + unlink_count=$(grep -cE "\bunlink\(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + rmdir_count=$(grep -cE "\brmdir\(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + chmod_count=$(grep -cE "\bchmod\(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + chown_count=$(grep -cE "\b(chown|lchown)\(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + + # Ensure all variables are integers + [ -z "$openat_count" ] && openat_count=0 + [ -z "$unlinkat_count" ] && unlinkat_count=0 + [ -z "$fchmodat_count" ] && fchmodat_count=0 + [ -z "$fchownat_count" ] && fchownat_count=0 + [ -z "$newfstatat_count" ] && newfstatat_count=0 + [ -z "$renameat_count" ] && renameat_count=0 + [ -z "$unlink_count" ] && unlink_count=0 + [ -z "$rmdir_count" ] && rmdir_count=0 + [ -z "$chmod_count" ] && chmod_count=0 + [ -z "$chown_count" ] && chown_count=0 + + # Calculate totals + safe_ops=$((openat_count + unlinkat_count + fchmodat_count + fchownat_count + newfstatat_count + renameat_count)) + unsafe_ops=$((unlink_count + rmdir_count + chmod_count + chown_count)) + + echo " Strace statistics:" + echo " Safe syscalls: openat=$openat_count unlinkat=$unlinkat_count fchmodat=$fchmodat_count fchownat=$fchownat_count newfstatat=$newfstatat_count renameat=$renameat_count" + echo " Unsafe syscalls: unlink=$unlink_count rmdir=$rmdir_count chmod=$chmod_count chown/lchown=$chown_count" + echo " Total: safe=$safe_ops unsafe=$unsafe_ops" + + # For rm specifically, we expect unlinkat instead of unlink/rmdir for file operations + # Note: A single rmdir() for the root directory is acceptable because: + # 1. The root directory path is provided by the user (not discovered during traversal) + # 2. There's no TOCTOU race - we're not resolving paths during recursive operations + # 3. After safe traversal removes all contents via unlinkat(), rmdir() is safe for the empty root + if [ "$util" = "rm" ]; then + if [ "$unlinkat_count" -gt 0 ] && [ "$unlink_count" -eq 0 ] && [ "$rmdir_count" -le 1 ]; then + echo "✓ Using safe syscalls (unlinkat for traversal)" + if [ "$rmdir_count" -eq 1 ]; then + echo " Note: Single rmdir() for root directory is acceptable" + fi + elif [ "$unlink_count" -gt 0 ] || [ "$rmdir_count" -gt 1 ]; then + fail_immediately "$util is UNSAFE: Using unlink/rmdir for file operations (unlink=$unlink_count rmdir=$rmdir_count unlinkat=$unlinkat_count) - vulnerable to TOCTOU attacks" + else + echo "⚠ No file removal operations detected" + fi + elif [ "$safe_ops" -gt 0 ] && [ "$unsafe_ops" -eq 0 ]; then + echo "✓ Using only safe syscalls" + elif [ "$safe_ops" -gt 0 ] && [ "$safe_ops" -ge "$unsafe_ops" ]; then + echo "✓ Using primarily safe syscalls" + elif [ "$found_safe" -gt 0 ]; then + echo "⚠ Some safe syscalls found but mixed with unsafe ops" + else + fail_immediately "$util is not using safe traversal" + fi +} + +# Get list of available utilities +if [ "$USE_MULTICALL" -eq 1 ]; then + AVAILABLE_UTILS=$($COREUTILS_BIN --list) +else + AVAILABLE_UTILS="" + for util in rm chmod chown chgrp du mv; do + if [ -f "$PROJECT_ROOT/target/release/$util" ]; then + AVAILABLE_UTILS="$AVAILABLE_UTILS $util" + fi + done +fi + +# Test rm - should use openat, unlinkat, newfstatat +if echo "$AVAILABLE_UTILS" | grep -q "rm"; then + cp -r test_dir test_rm + check_utility "rm" "openat,unlinkat,newfstatat,unlink,rmdir" "openat" "-rf test_rm" "recursive_remove" +fi + +# Test chmod - should use openat, fchmodat, newfstatat +if echo "$AVAILABLE_UTILS" | grep -q "chmod"; then + cp -r test_dir test_chmod + check_utility "chmod" "openat,fchmodat,newfstatat,chmod" "openat fchmodat" "-R 755 test_chmod" "recursive_chmod" +fi + +# Test chown - should use openat, fchownat, newfstatat +if echo "$AVAILABLE_UTILS" | grep -q "chown"; then + cp -r test_dir test_chown + USER_ID=$(id -u) + GROUP_ID=$(id -g) + check_utility "chown" "openat,fchownat,newfstatat,chown,lchown" "openat fchownat" "-R $USER_ID:$GROUP_ID test_chown" "recursive_chown" +fi + +# Test chgrp - should use openat, fchownat, newfstatat +if echo "$AVAILABLE_UTILS" | grep -q "chgrp"; then + cp -r test_dir test_chgrp + check_utility "chgrp" "openat,fchownat,newfstatat,chown,lchown" "openat fchownat" "-R $GROUP_ID test_chgrp" "recursive_chgrp" +fi + +# Test du - should use openat, newfstatat +if echo "$AVAILABLE_UTILS" | grep -q "du"; then + cp -r test_dir test_du + check_utility "du" "openat,newfstatat,stat,lstat" "openat" "-a test_du" "directory_usage" +fi + +# Test mv - should use openat, renameat for directory moves +if echo "$AVAILABLE_UTILS" | grep -q "mv"; then + mkdir -p test_mv_src/sub + echo "test" > test_mv_src/file.txt + echo "test" > test_mv_src/sub/file2.txt + check_utility "mv" "openat,renameat,newfstatat,rename" "openat" "test_mv_src test_mv_dst" "move_directory" +fi + +echo "" +echo "✓ Basic safe traversal verification completed" +echo "" +echo "=== Additional Safety Checks ===" + +# Check for dangerous patterns across all logs +echo "Checking for dangerous path resolution patterns..." + +# Check that we're not doing excessive path resolutions (sign of TOCTOU vulnerability) +echo "Checking path resolution frequency..." +for log in strace_*.log; do + if [ -f "$log" ]; then + path_resolutions=$(grep -c "test_" "$log" 2>/dev/null || echo "0") + if [ "$path_resolutions" -gt 20 ]; then + echo "⚠ $log: High path resolution count ($path_resolutions) - potential TOCTOU risk" + fi + fi +done + +echo "" +echo "=== Summary ===" +echo "All utilities are using safe traversal correctly!"