Throw JsonSyntaxException for null element in primitive array - #3118
jdymitarai wants to merge 1 commit into
Conversation
|
Thanks! There is already an existing recent PR which fixes that issue (as well as a few related ones): #3108 Would it be ok for you if that existing PR was preferred over your one? Maybe it would be helpful if you had a look at that PR and verify that it covers everything your PR covers. The tests there are not as extensive, but maybe they suffice? |
|
Hi @Marcono1234, Thanks for pointing that out! I took a close look at #3108 — it looks great and addresses the primitive array null issue cleanly along with the related collection adapters (TreeSet, ArrayDeque, PriorityQueue) and float/double number parsing. I'm completely fine with preferring #3108 over this PR! The implementation approach in ArrayTypeAdapter is essentially identical. The only minor difference was that our tests in this PR explicitly asserted against all 8 Java primitive array types (int[], boolean[], double[], long[], char[], byte[], short[], float[]), but since componentType.isPrimitive() handles all 8 uniformly in ArrayTypeAdapter, the coverage in #3108 is more than sufficient. Feel free to close this PR in favor of #3108. Thanks again for your review and stewardship of Gson! |
|
Closing in favor of #3108 as discussed above. Thank you! |
Purpose
Throw
JsonSyntaxExceptioninstead of a rawIllegalArgumentExceptionwhen a JSON array with anullelement is deserialized into a primitive array.Description
Deserializing a JSON array that contains a
nullelement into any primitive array (e.g.int[],boolean[],double[],long[],char[],byte[],short[],float[]) currently causes reflection (Array.set(array, i, null)) to throw an uncaughtjava.lang.IllegalArgumentException:Because
IllegalArgumentExceptionis neither wrapped nor caught, it terminates execution with a raw runtime exception and lacks any JSON document path context.This PR adds a null check in
ArrayTypeAdapter.readwhencomponentType.isPrimitive():JsonSyntaxException("null is not a valid " + componentType.getName() + "[] element; at path " + in.getPreviousPath()), directly mirroring the precedent established forAtomicLongArrayin Throw JsonSyntaxException instead of NullPointerException for a null AtomicLongArray element #3038 and Fix some adapter exceptions not including JSON document location #3096.$[1]or$[0][1]).Integer[],String[], and outer dimensions of multidimensional arrays likeint[][]) unaffected so they can continue to containnullelements as valid Java objects.ArrayTestcovering all primitive types, boxed arrays, and multidimensional arrays.Checklist
ArrayTestfor situations which failed previously and are now fixed