fix: throw JsonSyntaxException for null elements in AtomicIntegerArray - #3126
priyadarshnisundararajan wants to merge 1 commit into
Conversation
The ATOMIC_INTEGER_ARRAY type adapter called in.nextInt() directly and only caught NumberFormatException, so a null array element escaped as a raw IllegalStateException. Mirror the AtomicLongArray fix from google#3038: peek for a NULL token and throw JsonSyntaxException with the element path. Fixes google#3047
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Thanks for the pull request! However, have you verified that the issue you are trying to fix here even existed in the first place, and have you read the discussion in #3047? Even the author of that issue confirmed in the end that the claimed issue does not actually exist: #3047 (comment) |
|
You're right, thank you for checking. I verified against current |
What
new Gson().fromJson("[1,null,3]", AtomicIntegerArray.class)threw a rawIllegalStateException("Expected an int but was NULL") instead ofJsonSyntaxException.Why
In
TypeAdapters.ATOMIC_INTEGER_ARRAY.read(),in.nextInt()was called directly inside a try block that only caughtNumberFormatException. A JSON null token makesnextInt()throwIllegalStateException, which escaped uncaught.Fix
Mirrors the already-merged
AtomicLongArrayfix from PR #3038: peek for aNULLtoken before reading each element and throwJsonSyntaxException("null is not a valid AtomicIntegerArray element; at path " + in.getPath()).in.getPath()(notgetPreviousPath()) is used because the null token has not been consumed yet, which yields the identical$[1]path format as theAtomicLongArrayfix.Fixes #3047
Tests
Added
testAtomicIntegerArrayWithNullElementtoJavaUtilConcurrentAtomicTest, assertingJsonSyntaxExceptionwith messagenull is not a valid AtomicIntegerArray element; at path $[1](mirrorstestAtomicLongArrayWithNullElement). Ran the fullJavaUtilConcurrentAtomicTestclass: 9 tests, all passing (8 existing + 1 new). Maven could not resolve build extensions over this network (TLS interception on repo.maven.apache.org), so tests were compiled and run directly with javac/java 21 using jars fetched from Maven Central (junit 4.13.2, truth 1.4.5, guava 33.7.0-jre + transitive deps).