simpleqt opened a new pull request, #923:
URL: https://github.com/apache/commons-io/pull/923
# Fix UnsynchronizedByteArrayInputStream.skip() past end of stream
## Problem
`UnsynchronizedByteArrayInputStream.skip(long)` clamps the **return** value
to the bytes available, but advances the internal position by the **requested**
count (`offset = Math.addExact(offset, Math.toIntExact(n))`). Skipping past the
end of stream therefore leaves `offset` beyond `eod` (end of data). Concrete
consequences, reproducible on `master` (606e55d6b):
```java
UnsynchronizedByteArrayInputStream in =
new UnsynchronizedByteArrayInputStream(new byte[]{'a','b','c'});
in.skip(5); // returns 3 (correct), but leaves offset = 5
in.skip(1); // returns -2 -> InputStream#skip contract says "the
actual number of bytes skipped"; must be >= 0 (JDK ByteArrayInputStream returns
0)
UnsynchronizedByteArrayInputStream in2 =
new UnsynchronizedByteArrayInputStream(new byte[]{'a','b','c'});
in2.skip(Long.MAX_VALUE); // throws java.lang.ArithmeticException: integer
overflow
// (Math.toIntExact(Long.MAX_VALUE)); JDK
ByteArrayInputStream
// clamps to the available count and returns 3
```
The negative return value is the same value then handed back by anything
that skips through such a stream (e.g. `IOUtils.copyLarge(..., inputOffset,
...)` calls `skipFully` on wrapped streams), which makes the failure mode hard
to diagnose downstream.
## Root cause
In `skip(long)`:
```java
long actualSkip = eod - offset;
if (n < actualSkip) {
actualSkip = n;
}
offset = Math.addExact(offset, Math.toIntExact(n)); // advances by requested
n, not actualSkip
return actualSkip;
```
After an overshoot, `eod - offset` is negative on the next call, the `n <
actualSkip` clamp does not trigger, and the negative value is returned.
`Math.toIntExact(n)` additionally throws for skip counts above
`Integer.MAX_VALUE`.
## Fix
Advance the position by the clamped, non-negative actual skip count only
(parity with `java.io.ByteArrayInputStream`):
```java
final long actualSkip = Math.min(n, Math.max(0L, (long) eod - offset));
if (actualSkip > 0) {
offset = Math.addExact(offset, Math.toIntExact(actualSkip));
}
return actualSkip;
```
`actualSkip` is bounded by the available bytes (an `int`), so
`toIntExact`/`addExact` can no longer overflow. Single-byte reads, bulk reads,
`available()`, `mark()`/`reset()` and the existing `skip(0)`/negative-`n`
behavior are unchanged.
## Regression test
`UnsynchronizedByteArrayInputStreamTest.testSkipPastEndOfStream()` (src/test
style, JUnit 5): asserts the return values of a skip past EOF, a skip at EOF,
and `skip(Long.MAX_VALUE)`.
Scoped run (`mvn -B -ntp -Dtest=UnsynchronizedByteArrayInputStreamTest
test`):
- Base `master` + test (second worktree at 606e55d6b): **Tests run: 17,
Failures: 1, Errors: 0** — `testSkipPastEndOfStream:373 expected: <0> but was:
<-996>`
- With fix: **Tests run: 17, Failures: 0, Errors: 0, Skipped: 0** — BUILD
SUCCESS
`mvn checkstyle:check` (repo config `src/conf/checkstyle.xml`, tests
included): BUILD SUCCESS. `mvn apache-rat:check`: BUILD SUCCESS, "No issues
found."
## AI disclosure
Per the ASF Generative Tooling Guidance: this change was produced with the
assistance of a generative AI coding tool (defect search, patch drafting, and
test writing); a human contributor reviewed the change and will submit it under
their own account (DCO sign-off below).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
