iemejia opened a new pull request, #3967: URL: https://github.com/apache/avro/pull/3967
## What changes were proposed in this pull request? The binary decoders mishandled `InputStream.skip()`. Per its [contract](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/io/InputStream.html#skip(long)), `skip()` may return `0` without being at end of stream, and negative returns are not specified. This PR fixes three related problems: 1. **Spurious EOF (the reported issue).** `BinaryDecoder.InputStreamByteSource.skipSourceBytes`/`trySkipBytes` treated two consecutive `0` returns as EOF (`EOFException`), so a stream that legitimately returns `0` from `skip()` could fail spuriously. It also carried a dead "negative return" branch (AVRO-4048). 2. **Over-skip / stream corruption.** `trySkipBytes` requested the original full length on every iteration (`in.skip(length)`) instead of the remaining amount (`in.skip(leftToSkip)`). On a partial skip this re-requests too much and can advance the underlying stream **past** the intended position, returning a skipped-count larger than requested. 3. **`DirectBinaryDecoder.doSkipBytes`** treated `skip() <= 0` as immediate EOF, sharing the same spurious-EOF problem. ## How was this patch fixed? Replace the fragile "two zeros = EOF" / negative-branch heuristics with a single-byte `read()` probe: when `skip()` returns a non-positive value, read one byte to tell a genuine EOF (`read() == -1`) apart from a transient inability to skip, then continue. This removes the dead negative branch, eliminates the spurious EOF, and cannot infinite-loop. `trySkipBytes` now skips only the remaining count (`leftToSkip`). The same probe is applied to `DirectBinaryDecoder.doSkipBytes`. ## How was this patch tested? New `TestBinaryDecoderSkip`: - `bufferedSkipFixedWithZeroSkipStream` / `directSkipFixedWithZeroSkipStream` — a stream whose `skip()` always returns `0` no longer causes a spurious `EOFException`; the decoder is positioned exactly after the skipped bytes. - `bufferedSkipFixedPastEndThrows` / `directSkipFixedPastEndThrows` — skipping past the real end still raises `EOFException`. - `inputStreamSkipDoesNotOverSkip` — a partial-skip stream is skipped by exactly the requested count (no over-skip). Verified the tests fail against the current code (over-skip returns 6 instead of 5; the zero-skip cases throw `EOFException`) and pass with the fix. No regressions across `TestBinaryDecoder` (68), `TestValidatingIO` (972), `TestBlockingIO` (376), and `TestDataFile`, under the default, custom-coders, and without-fast-reader surefire profiles. -- 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]
