Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 17, 2025

POSIX requires xargs to enforce size limits on constructed arguments in insert mode (-I). Current implementation lacks this validation.

Changes

Insert Mode Argument Size Limits

  • Added INSERT_ARG_MAX constant (4096 bytes, exceeds POSIX minimum of 255)
  • Validates constructed arguments in exec_insert_mode() using iterator methods
  • Returns descriptive error when limit exceeded

Test Coverage

Added 8 integration tests covering:

  • Insert mode size limit enforcement
  • Single and mixed quote handling
  • Empty input, line continuation
  • Multiple replstr occurrences
  • Combined -n and -s options
  • Five arguments with replstr (POSIX minimum)

Test count: 17 → 25 (all passing)

Example

Added validation in insert mode:

// Check that constructed arguments don't exceed the limit
if let Some(arg) = util_args.iter().find(|arg| arg.len() > INSERT_ARG_MAX) {
    return Err(io::Error::new(
        io::ErrorKind::InvalidInput,
        format!(
            "xargs: constructed argument of {} bytes exceeds {} byte limit in insert mode",
            arg.len(),
            INSERT_ARG_MAX
        ),
    ));
}
Original prompt

This section details on the original issue you should resolve

<issue_title>Audit xargs util</issue_title>
<issue_description>Review the xargs POSIX spec, xargs.html, analyze our implementation (process/xargs.rs, process/tests/xargs/mod.rs), identify any gaps in implementation or tests. If found, plan and implement gap remedies.</issue_description>

<agent_instructions>review CONTRIBUTING.md. cargo fmt required to pass CI/CD (which runs cargo tests on multiple arch's).</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits December 17, 2025 01:20
- Add immediate exit on signal termination (POSIX compliance)
- Enforce 4096 byte limit for -I constructed arguments (POSIX requires ≥255)
- Add 7 new tests for POSIX compliance gaps:
  * Single quote handling
  * Mixed quotes
  * Empty input
  * Line continuation with -L
  * Multiple replstr in -I
  * Five arguments with replstr in -I
  * Combining -n and -s options

Co-authored-by: jgarzik <[email protected]>
- Make argument size validation more idiomatic using iterator methods
- Add explicit test for -I mode 4096 byte limit enforcement
- All 25 xargs tests passing (up from 17 originally)

Co-authored-by: jgarzik <[email protected]>
Copilot AI changed the title [WIP] Audit xargs utility for compliance with POSIX spec Fix xargs POSIX signal termination and enforce -I mode argument size limits Dec 17, 2025
Copilot AI requested a review from jgarzik December 17, 2025 01:30
Copy link
Contributor

@jgarzik jgarzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes:

  1. Remove the code >= 128 signal handling (all 3 locations in the macro and
    the two manual handlers)
  2. Keep the INSERT_ARG_MAX constant and validation
  3. Keep all the tests (they test valid behaviors)

Remove code >= 128 signal handling from all 3 locations:
- handle_exec_result! macro
- insert mode handler
- final args handler

Keep INSERT_ARG_MAX constant and validation as requested.
All 25 tests still passing.

Co-authored-by: jgarzik <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 18, 2025

Request changes:

  1. Remove the code >= 128 signal handling (all 3 locations...

Done in commit 99df609. Removed all signal handling code (code >= 128) from the macro and two manual handlers. Kept INSERT_ARG_MAX constant and validation, and all 25 tests continue to pass.

Copilot AI changed the title Fix xargs POSIX signal termination and enforce -I mode argument size limits Enforce -I mode argument size limits for xargs Dec 18, 2025
Copilot AI requested a review from jgarzik December 18, 2025 03:08
@jgarzik jgarzik marked this pull request as ready for review December 18, 2025 03:50
@jgarzik jgarzik merged commit 67ed41f into main Dec 18, 2025
4 checks passed
@jgarzik jgarzik deleted the copilot/audit-xargs-util branch December 18, 2025 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit xargs util

2 participants