fix: remove implicit narrowing conversion in CipherInputStream.skip - #522
Closed
sharmabikram wants to merge 1 commit into
Closed
sharmabikram wants to merge 1 commit into
sharmabikram wants to merge 1 commit into
Conversation
The compound assignment `currentPosition += n` narrowed the long parameter to int implicitly, which GitHub code scanning flags. The buffered remainder is bounded by the buffer size and always fits in an int, so clamp the requested skip to that range as an int before the assignment. Behavior is unchanged for all inputs (in-range, zero, negative, and values beyond Integer.MAX_VALUE). Adds CipherInputStreamTest covering skip within the buffer, clamping to the buffered remainder, zero, negative, and Long.MAX_VALUE inputs.
Contributor
Author
|
Closing — will resubmit without a fork. |
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
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.
Issue
GitHub code scanning flags an implicit narrowing conversion in
CipherInputStream.skip(long)(alert code-scanning/3). The compound assignmentcurrentPosition += nisint += long, which implicitly narrows thelongback toint.Fix
The buffered remainder (
maxPosition - currentPosition) is bounded by the input buffer size (512 bytes by default) and always fits in anint. The requested skip is now clamped to that range as an int before the assignment, so:long -> intnarrowing on the compound assignment, andcurrentPositioncannot overflow.Behavior is unchanged for every input:
nwithin buffered remaindern, advancen, advancen> buffered remaindern == 0n < 0n>Integer.MAX_VALUETests
Adds
CipherInputStreamTest(5 cases, JUnit 5) covering skip within the buffer, clamping to the buffered remainder, zero, negative, andLong.MAX_VALUEinputs.mvn -Dtest=CipherInputStreamTest test->Tests run: 5, Failures: 0, Errors: 0.Notes
The pre-existing SME assessment on the internal tracking ticket is that this is a low-severity, benign finding (
currentPositionnever approachesInteger.MAX_VALUEin practice). This change resolves the static-analysis alert with a behavior-preserving refactor and adds test coverage where there previously was none forskip.