fix: JsonTreeWriter accepts finite BigDecimal/BigInteger like JsonWriter

[andrewstellman]

Summary

JsonTreeWriter.value(Number) rejects a finite BigDecimal or BigInteger that JsonWriter.value(Number) accepts. The two writer surfaces should agree on what constitutes a valid JSON number, but JsonTreeWriter's finiteness check uses doubleValue() — which overflows to Infinity for finite values like BigDecimal("1E400"). JsonWriter already has a whitelist (alwaysCreatesValidJsonNumber) that skips the doubleValue oracle for BigDecimal/BigInteger/AtomicInteger/AtomicLong. This PR mirrors that guard in JsonTreeWriter.

Reproduction

Gson gson = new GsonBuilder().setStrictness(Strictness.STRICT).create();
BigDecimal finiteHuge = new BigDecimal("1E400");

gson.toJson(finiteHuge, Number.class);      // OK — returns "1E+400"
gson.toJsonTree(finiteHuge, Number.class);  // throws IllegalArgumentException
                                            //   "JSON forbids NaN and infinities"

finiteHuge is mathematically finite, the streaming serializer accepts it, but toJsonTree rejects it because:

// JsonTreeWriter.value(Number), pre-fix:
double d = value.doubleValue();
if (Double.isNaN(d) || Double.isInfinite(d)) {
  throw new IllegalArgumentException("JSON forbids NaN and infinities: " + value);
}

BigDecimal("1E400").doubleValue() returns Double.POSITIVE_INFINITY even though the BigDecimal itself is finite.

Root cause

doubleValue() is the wrong oracle for BigDecimal/BigInteger finiteness — those types can represent values outside double's range. JsonWriter.value(Number) already handles this correctly via the alwaysCreatesValidJsonNumber whitelist (JsonWriter.java:730-741), which skips the VALID_JSON_NUMBER_PATTERN validation for known-always-valid number classes. The tree writer never got the equivalent guard.

Fix

internal/bind/JsonTreeWriter.java:218-226 — skip the doubleValue() finiteness check when the value is a BigDecimal or BigInteger. Both classes always produce valid JSON number toString() output, so the check is only needed for Float/Double and arbitrary Number subclasses where doubleValue() is a meaningful oracle.

The new condition is if (!isLenient() && !(value instanceof BigDecimal) && !(value instanceof BigInteger)). AtomicInteger/AtomicLong aren't included because their doubleValue() is exact and finite by construction.

Testing

New regression test Bug001RegressionTest.treeWriterAcceptsFiniteBigDecimalLikeStreamWriter in gson/src/test/java/com/google/gson/regression/:

• Red (before fix): treeOk = false, stringOk = true → assertion fails.
• Green (after fix): both paths accept the value → assertion passes.

Existing test suites pass with the change: JsonTreeWriterTest, ToNumberPolicyTest. The fix doesn't relax the lenient/strict semantics for Float/Double or arbitrary number subclasses — only BigDecimal/BigInteger get the new skip.

Provenance

Discovered by Quality Playbook v1.5.8, an AI-assisted code review skill. The regression test was generated and TDD-verified (red on clean HEAD → green after fix) automatically before this PR was authored. The bug is a write-surface parallel-implementation hazard — same class as #3006 (which fixed the read-side dup-key counterpart).

Related

• Fix duplicate key detection when first value is null #3006 — read-side dup-key detection fix (same area, opposite side)

[Marcono1234]

Good find! Thanks for providing a fix for this.

---

Feel free to consider my review comments only as suggestions; I am not a direct project member.

[Marcono1234]

This test naming might be a bit arbitrary / not very expressive. Might be better to move the new test method to the existing JsonTreeWriterTest?

[andrewstellman]

Yep, thanks!

[Marcono1234]

Not sure if that relevant, but with this null check the assertion below would also succeed if one implementation returns null and the other throws an exception. That would probably not be desired.

Maybe omit the try-catch and just have two assertThat(...).isNotNull(), indicating that both implementations should support it without throwing (that would also match the test method name which says "... accepts ...")?

[andrewstellman]

You're right, pushed tighter version.