Check ignored error status of I/O calls (CodeQL java/ignored-error-status-of-call)#233
Open
vharseko wants to merge 3 commits into
Open
Check ignored error status of I/O calls (CodeQL java/ignored-error-status-of-call)#233vharseko wants to merge 3 commits into
vharseko wants to merge 3 commits into
Conversation
…atus-of-call)
Nine call sites discarded the return value of a stream read/skip or of
File.renameTo, so a short read, an incomplete skip, or a failed rename passed
silently. Consume and act on each result:
- StreamLoader.next: read the record key/value with DataInputStream.readFully
instead of read(), so a short read on a buffered/socket stream can no longer
hand a partially filled key or value to the import handler (latent
corruption bug).
- Value.ValueObjectInputStream.readFully: honour the DataInput.readFully
contract - the local read() transfers exactly the requested length or
throws, so a short count now raises EOFException.
- IOMeter.main: loop until the requested byte count is fully skipped, since
InputStream.skip may skip fewer bytes than asked.
- BackupTask.rename: throw IOException when File.renameTo returns false rather
than reporting success for a rename that did not happen.
- WDSSO.parseToken and DerValue.init: use the byte count returned by
ByteArrayInputStream.read so a truncated SPNEGO/Kerberos token or DER value
fails closed instead of being compared against uninitialised trailing bytes.
For every well-formed input the reads/skips/renames already completed fully, so
behaviour is unchanged for existing callers; only truncated or failing I/O is
now surfaced.
Also add the missing 3A Systems Portions header line to StreamLoader.java,
BackupTask.java, WDSSO.java and DerValue.java (Value.java and IOMeter.java
already carry it).
maximthomas
approved these changes
Jul 8, 2026
The test builds a three-transaction deadlock cycle (ts1 <- ts3 <- ts2 <- ts1) and assumed a specific thread would win the deadlock-detection race: that t3 (which closes the cycle) reports UNCOMMITTED and that t2 then clears to 0 once ts2 aborts. But deadlock detection is distributed -- every waiter re-checks for a cycle every SHORT_TIMEOUT (10 ms) -- so any transaction on the cycle may report the deadlock, and more than one may do so before the cycle is broken. Under load a second thread detected the deadlock too, so result2 was UNCOMMITTED instead of 0 (seen on ubuntu-latest JDK 17, expected:<0> but was:<9223372036854775807>). Assert the invariants that always hold instead of pinning the outcome to a particular thread: the deadlock is detected by at least one participant, ts3's wait on ts2 ends by either detecting the deadlock or observing ts2's abort, and each waiter blocks until the cycle forms (~1000 ms).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves all nine open
java/ignored-error-status-of-callCodeQL alerts. Eachsite threw away the result of a stream
read/skipor ofFile.renameTo, so ashort read, an incomplete skip, or a failed rename went unnoticed.
StreamLoader.java(139, 141)DataInputStream.read(buf, off, len)for record key/valuereadFullyValue.java(4894, 4899)readinsidereadFullyoverridesEOFExceptionon a short readIOMeter.java(455)InputStream.skipin the dumpmainBackupTask.java(263)File.renameToIOExceptionwhen it returnsfalseWDSSO.java(288, 326)ByteArrayInputStream.readfor SPNEGO / Kerberos OIDDerValue.java(91)ByteArrayInputStream.readfor DER contentWhy it is safe
For every well-formed input these calls already transferred the full amount:
StreamLoaderreads from aDataInputStream;readFullyreads exactly therequested bytes or throws
EOFException— for a complete stream the bytecount is identical to before, but a short read (buffered / socket source) can
no longer deliver a half-filled key or value.
Value.ValueObjectInputStream.readalready transfers exactlylengthbytesor throws
IOException, so the added!= lengthguard only documents andenforces the existing
DataInput.readFullycontract.IOMeter.mainis a diagnostic dumper; the skip loop reaches the same offset,just correctly when
skipreturns fewer bytes than requested.BackupTask.renameonly reaches the newthrowwhenrenameToactuallyfails — previously that failure was reported as success.
WDSSO/DerValuea valid token fills the OID / DER buffers completely, sothe guard is
trueand the comparison result is unchanged; a truncatedtoken now fails closed instead of being compared against zero-filled trailing
bytes.
So the change is behaviour-preserving for all existing (valid) inputs and only
surfaces genuinely truncated or failed I/O.
Copyright
Adds the missing
Portions Copyrighted 2026 3A Systems, LLCheader line toStreamLoader.java,BackupTask.java,WDSSO.javaandDerValue.java(
Value.javaandIOMeter.javaalready carry it). The@Checkstyle:ignoreForcount in the two ForgeRock CDDL headers is bumped by one to keep covering the
extended header.
Testing
mvn -o -pl persistit/core,commons/auth-filters/authn-filter/jaspi-modules/iwa-module -am compile— BUILD SUCCESS.