Fix Output.longLength to accept long instead of int#1290
Open
raphaelroshan wants to merge 1 commit into
Open
Conversation
Output.longLength declared its value parameter as int, so longs outside int range were silently narrowed before delegating to varLongLength, yielding the wrong byte length (e.g. longLength(1L << 32) returned 1 though writeVarLong writes 5 bytes). Widen the parameter to long (source-compatible) and fix the stale @see reference on writeLong. Add a round-trip test asserting longLength matches the bytes written.
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect byte-length calculation for variable-length longs by widening Output.longLength’s value parameter from int to long, aligning it with varLongLength(long, boolean) and the bytes written by writeVarLong. It also updates related Javadoc and adds a regression test to validate length calculations across var-long boundaries.
Changes:
- Change
Output.longLength(int, boolean)toOutput.longLength(long, boolean)to avoid truncation and incorrect results for largelongvalues. - Fix stale Javadoc
@seereference onwriteLong(long, boolean). - Add a test that asserts
longLengthmatches the actual number of bytes written across representative var-long boundary values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/com/esotericsoftware/kryo/io/Output.java |
Widens longLength parameter to long and updates related Javadoc reference. |
test/com/esotericsoftware/kryo/io/InputOutputTest.java |
Adds a regression test covering var-long boundary values and negatives. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
596
to
600
| /** Returns the number of bytes that would be written with {@link #writeLong(long, boolean)}. */ | ||
| public int longLength (int value, boolean optimizePositive) { | ||
| public int longLength (long value, boolean optimizePositive) { | ||
| if (varEncoding) return varLongLength(value, optimizePositive); | ||
| return 8; | ||
| } |
Comment on lines
473
to
+477
| /** Reads a long using fixed or variable length encoding, depending on {@link #setVariableLengthEncoding(boolean)}. Use | ||
| * {@link #writeVarLong(long, boolean)} explicitly when writing values that should always use variable length encoding (eg | ||
| * values that appear many times). | ||
| * @return The number of bytes written. | ||
| * @see #longLength(int, boolean) */ | ||
| * @see #longLength(long, boolean) */ |
Comment on lines
+56
to
+58
| // Values spanning each var-long byte boundary, including values outside int range and negatives. | ||
| long[] values = {0L, 127L, 128L, 1L << 28, 1L << 32, 1L << 40, 1L << 48, 1L << 56, Long.MAX_VALUE, -1L, | ||
| Long.MIN_VALUE}; |
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.
The
valueparameter ofOutput.longLengthwas declaredint, so alongoutside int range was silently narrowed before delegating tovarLongLength(long, boolean), returning the wrong byte count. For examplelongLength(1L << 32, true)returns 1, butwriteVarLong(1L << 32, true)writes 5 bytes.Widen the parameter to
long(source-compatible — existingintcallers still compile), and fix the stale@see #longLength(int, boolean)onwriteLong. Added a round-trip test assertinglongLengthmatches the bytes actually written across var-long boundaries.