Repository navigation
Conversation
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.
Fixes #1042.
Check the map size before allocating the interleaved key/value array in both
MessageUnpacker.unpackValue()overloads. Pair counts from2^30through2^31 - 1now throw the existingMessageSizeException, with the declared count available throughgetSize(), instead of overflowingsize * 2and throwingNegativeArraySizeException. Larger unsigned headers retain their existing size exception.The streaming
unpackMapHeader()range is preserved. This change addresses the integer overflow during value materialization; it does not introduce a general memory limit or gradual allocation for #657.Validation
The same 21 new AirSpec cases produced four expected errors before the fix and passed afterward; the other 17 cases passed on both versions. They cover both overloads, the overflow boundary, existing unsigned-size rejection, ordinary fixmap/map16/map32 values, nested maps, and streaming headers.
All 21 cases pass with both default and universal buffers on actual Java 8, 11, 17, 21 and 24 test JVMs.
scalafmtCheckAllandjcheckStylepass.The existing pending entry is the malformed UTF-8 string case in
MessagePackTest.Broader local results:
The remaining universal Java 21 error is the unchanged
DirectBufferAccessTest, which accessesMessageBuffer.unsafewhen universal mode leaves it null. Of the 27 unchanged Jackson failures, 26 are null-pointer failures associated with the existing mixture of Jupiter test methods and JUnit 4 fixture lifecycle annotations. The remaining failure istestMessagePackParserDirectly(): its parser-location assertion expects0and receives-1, independently of those fixtures. These limitations were also documented in #1037 and were rechecked for this change. The stock sbt task does not discover the Jackson JUnit tests, so its zero-test result is not counted as a successful Jackson run.AirSpec’s stock XML repeats leaf names for some nested tests. Additional full core runs used a verification-only copy of its event reporter that includes the actual parent test context in each name. Those reports contain 215 distinct testcase names and preserve the outcomes above; assertions, test execution, status and errors were unchanged. The 21 new regression cases and the focused Java-version matrix also passed with the unmodified reporter. This is not a claim that every project test is green. Remote CI has not run for this unpublished branch.
Portable reproduction commands and build notes
Run sbt itself on JDK 21. Set
TEST_JAVA_HOMEto the JDK whose compiler/test runtime is being checked. The local evidence additionally recorded the forked JVM identity; that diagnostic did not transform classes or change test behavior.TEST_JAVA_HOME=/path/to/jdk21 ./sbt -batch \ 'msgpack-core/testOnly org.msgpack.core.MapSizeLimitTest' ./sbt -batch scalafmtCheckAll jcheckStyleFor the core suite, use
testOnly *to select every class explicitly; sbt 2'stestusestestQuickand may reuse previous results:The initial Java 8 attempt encountered
NoSuchMethodErrorfrom classes linked against newer covariant NIO methods. The exact origin of that bytecode or cache reuse was not established. An explicit session setting followed by recompilation recovered the Java 8 runs:For Jackson, obtain its
Test/fullClasspathfrom sbt and run JUnit Platform Console Standalone 1.14.4. The normal class-name scan covers 76 entries; the additional example class contributes one entry:The report-only verification input comes from AirSpec 2026.2.4. Its sole change qualifies the emitted event name with
parentContext.testNameafter the test has completed. It was supplied through a local sbt session setting, is outside this patch, and does not change test selection or expectations. The original reports and this diagnostic source are retained separately.The fix and tests are limited to
MessageUnpackerand the newMapSizeLimitTest. Build settings and the unrelated fixtures remain outside this patch. #1026 also touchesMessageUnpacker; its current depth-limit change leaves these two overflowing allocations in place.