fix(ptx): reject --gap-size/--width values exceeding isize::MAX#13216
fix(ptx): reject --gap-size/--width values exceeding isize::MAX#13216koopatroopa787 wants to merge 2 commits into
Conversation
Values above isize::MAX are silently cast to a negative isize when the output-chunk sizing arithmetic does `value as isize`, wrapping in release builds (producing garbage output or capacity overflow) and panicking in debug builds (attempt to subtract with overflow). GNU ptx rejects any such value up front with "invalid gap width" / "invalid line width" and exits 1. Match that behaviour by validating the parsed usize against isize::MAX immediately after parsing, before the value is stored in Config. Fixes uutils#13184
|
GNU testsuite comparison: |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
Gentle ping — CI is green (the 4.67% CodSpeed |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR fixes a crash/overflow bug in ptx when users pass --gap-size or --width values greater than isize::MAX, aligning behavior with GNU ptx by rejecting those inputs early.
Changes:
- Add
PtxErrorvariants for invalid gap width and line width values. - Validate parsed
--widthand--gap-sizeagainstisize::MAXbefore storing inConfig. - Add regression tests covering values just above
isize::MAXfor both flags.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
tests/by-util/test_ptx.rs |
Adds regression tests ensuring oversized --gap-size/--width inputs are rejected. |
src/uu/ptx/src/ptx.rs |
Introduces new error variants and clamps accepted numeric ranges to <= isize::MAX. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let s = matches.get_one::<String>(options::WIDTH).expect(err_msg); | ||
| let v: usize = s.parse().map_err(PtxError::ParseError)?; | ||
| if v > isize::MAX as usize { | ||
| return Err(PtxError::InvalidLineWidth(s.clone()).into()); | ||
| } | ||
| config.line_width = v; |
| let s = matches.get_one::<String>(options::GAP_SIZE).expect(err_msg); | ||
| let v: usize = s.parse().map_err(PtxError::ParseError)?; | ||
| if v > isize::MAX as usize { | ||
| return Err(PtxError::InvalidGapWidth(s.clone()).into()); | ||
| } | ||
| config.gap_size = v; |
| let too_big = format!("{}", isize::MAX as u64 + 1); | ||
| new_ucmd!() | ||
| .args(&["--gap-size", &too_big]) | ||
| .pipe_in("hello world\n") | ||
| .fails() | ||
| .stderr_contains("invalid gap width"); |
| let too_big = format!("{}", isize::MAX as u64 + 1); | ||
| new_ucmd!() | ||
| .args(&["--width", &too_big]) | ||
| .pipe_in("hello world\n") | ||
| .fails() | ||
| .stderr_contains("invalid line width"); |
| let s = matches.get_one::<String>(options::WIDTH).expect(err_msg); | ||
| let v: usize = s.parse().map_err(PtxError::ParseError)?; | ||
| if v > isize::MAX as usize { | ||
| return Err(PtxError::InvalidLineWidth(s.clone()).into()); | ||
| } | ||
| config.line_width = v; |
| let s = matches.get_one::<String>(options::GAP_SIZE).expect(err_msg); | ||
| let v: usize = s.parse().map_err(PtxError::ParseError)?; | ||
| if v > isize::MAX as usize { | ||
| return Err(PtxError::InvalidGapWidth(s.clone()).into()); | ||
| } | ||
| config.gap_size = v; |
Fixes #13184.
Bug
ptx --gap-size Nwith N >isize::MAXpanics (debug) or produces silent garbage /capacity overflow(release) because the output-chunk sizing math casts the value withas isize, which wraps negative on overflow:GNU
ptxrejects any value aboveisize::MAXup front:Fix
Add two new
PtxErrorvariants (InvalidGapWidth,InvalidLineWidth) and validate the parsedusizeagainstisize::MAX as usizeimmediately after parsing, before the value is stored inConfig. This matches GNU's exact error messages and exit code.Testing
Added
test_gap_size_above_isize_max_is_rejectedandtest_line_width_above_isize_max_is_rejected. All 45 ptx tests pass;cargo clippy -D warningsandcargo fmt --checkclean.