fix: uniqueItems compares numbers by value, not by node type - #1273
Conversation
uniqueItems put each item into a HashSet<JsonNode> and relied on JsonNode
equality, which is type-sensitive: IntNode(1) does not equal
DoubleNode(1.0). Two mathematically equal numbers written differently were
therefore accepted as distinct items, and [1, 1.0] validated.
The suite's case for this rule is [1.0, 1.0, 1] ("numbers are unique if
mathematically unequal"). It passes either way, because the two identical
decimals are caught before an integer is ever compared against a decimal,
so the gap stayed hidden.
Items are now compared by a derived key. Numbers reduce to a
scale-independent BigDecimal, so 1, 1.0 and 1.00 are one value, and
10000000000 matches 1.0e10. Only numbers are folded together: a number and
a boolean stay distinct, as do a number and its string form, which the
suite requires with nested [1] vs [true] and [0] vs [false]. Objects and
arrays are walked so the rule holds wherever the number sits, and map
equality ignores property order.
Tests cover the integer/decimal pairs, differing scales, a large value in
exponent form, nesting inside an object and an array, and the
number-versus-boolean and number-versus-string cases that must stay
unique. Four of them fail against the previous implementation.
Full suite: 8487 tests, 0 failures.
a4d8dff to
162a870
Compare
There was a problem hiding this comment.
Pull request overview
Fixes uniqueItems numeric comparison semantics so JSON numbers are deduplicated by mathematical value (e.g., 1, 1.0, 1.00, 1e0), aligning behavior with the JSON Schema specification and closing a gap not covered by the upstream JSON-Schema-Test-Suite case ordering.
Changes:
- Update
UniqueItemsValidatorto compare items using a derived comparison key rather thanJsonNodetype-sensitive equality, normalizing numeric values viaBigDecimal.stripTrailingZeros(). - Add a dedicated JUnit test suite covering integer/decimal equivalence (including exponent forms) and ensuring numbers do not collapse into booleans/strings, including nested structures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/com/networknt/schema/keyword/UniqueItemsValidator.java | Switches uniqueItems deduplication from JsonNode equality to derived comparison keys that normalize numeric values and walk arrays/objects. |
| src/test/java/com/networknt/schema/UniqueItemsNumericEqualityTest.java | Adds targeted tests to catch integer-vs-decimal equality cases and preserve distinctness vs booleans/strings, including nested scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (node.isNumber()) { | ||
| return node.decimalValue().stripTrailingZeros(); | ||
| } |
There was a problem hiding this comment.
Confirmed and fixed in 06dbe98.
It is reachable through the public API, not only in theory: with a mapper built with ALLOW_NON_NUMERIC_NUMBERS, validating [NaN, NaN] against {"uniqueItems": true} raised
JsonNodeException: 'DoubleNode' method `decimalValue()` cannot convert value NaN to `java.math.BigDecimal`: value non-Finite ('NaN')
instead of returning a result. Same for Infinity and -Infinity.
Guarded with JsonNodeTypes.isNonFiniteNumber, following MultipleOfValidator.getDividend. Those values fall through to comparing as nodes, which is what every item did before this PR folded numbers by value, so their behaviour is unchanged — [NaN, NaN] is still a duplicate, and a finite number beside a non-finite one is still folded ([Infinity, 1, 1.0] is not unique).
Test added covering all three values in both directions; it throws without the guard. Full suite is 8488 tests, 0 failures.
decimalValue() has no BigDecimal to return for NaN, Infinity or -Infinity and throws, so folding numbers by value made uniqueItems raise instead of producing a validation result for an instance a mapper had been configured to read with ALLOW_NON_NUMERIC_NUMBERS. Guard with JsonNodeTypes.isNonFiniteNumber, as MultipleOfValidator.getDividend already does. Those values fall through to comparing as nodes, which is what every item did before numbers were folded, so their behaviour is unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/main/java/com/networknt/schema/keyword/UniqueItemsValidator.java:79
- The Javadoc block for comparisonKey() has mis-indented lines (missing leading spaces before
*), which can break Javadoc formatting and may fail style checks. Align the*prefixes consistently with the rest of the comment.
* NaN, Infinity and -Infinity have no BigDecimal form, so they are left out
* of that and keep comparing as nodes, as every item did before.
*
* Only numbers are folded together. A number and a boolean stay distinct,
* as do a number and its string form, which the suite requires: nested [1]
The bug
UniqueItemsValidatorputs each item into aHashSet<JsonNode>and relies onJsonNodeequality, which is type-sensitive —IntNode(1)does not equalDoubleNode(1.0). Two mathematically equal numbers written differently are therefore accepted as distinct items.The spec is explicit that numbers compare by mathematical value.
Why the suite didn't catch it
The official case for this rule is:
That passes either way. The two identical decimals are caught first, so an integer is never actually compared against a decimal, and the gap stays hidden. The tests here put the integer next to the decimal directly.
The change
Items are compared by a derived key rather than by node identity. Numbers reduce to a scale-independent
BigDecimal, so1,1.0and1.00are one value.Only numbers are folded together, which the suite constrains tightly:
[[[1], "foo"], [[true], "foo"]]and[[[0], "foo"], [[false], "foo"]]are both required to be unique[1, "1"]is uniqueObjects and arrays are walked so the rule holds wherever the number sits (
[{"a":1},{"a":1.0}]is a duplicate), and map equality ignores property order.Tests
UniqueItemsNumericEqualityTest, 8 cases: the integer/decimal pairs, differing scales, a large value in exponent form, nesting inside an object and an array, and the number-versus-boolean and number-versus-string cases that must remain unique.Four fail against the previous implementation; the four asserting things stay distinct pass both before and after, so the tests pin the rule rather than the implementation.
Full suite after the change: 8487 tests, 0 failures — including the complete vendored JSON-Schema-Test-Suite.
Built and tested with JDK 17 and Maven 3.9.9. The file uses CRLF endings and I kept them, so the diff is only the lines I touched.