Preserve exact multipleOf operands

[PHJ2000]

Fixes #1286.

Preserve exact multipleOf operands so a large odd integer such as 9007199254740993 is rejected as a multiple of 2. Default JSON/YAML validation readers also preserve decimal literals instead of rounding them before validation.

The private readers keep a DoubleNode when its decimal value round-trips exactly and use BigDecimal otherwise. Public mapper factories and caller-supplied mapper settings remain unchanged. Numeric type selection also works with location-aware readers. Bounds preserve FloatNode decimal text, and loose numeric strings with large exponents are compared and checked for divisibility without expanding exponent-sized powers. Protected conversion hooks remain honored.

Compatibility details:

• Integral/BigDecimal zero and finite negative multipleOf divisors raise SchemaException. Floating-point zero and non-finite divisors retain their ignored behavior, including custom-reader underflow to zero.
• Non-integral or non-finite maxItems now uses the same unrestrictive fallback as the other maximum counts. Out-of-range limits retain their original value in errors; minima above Integer.MAX_VALUE still reject a count of Integer.MAX_VALUE.
• Default BigDecimal nodes normalize insignificant trailing zeroes for Jackson 3 tree equality; values remain exact.

Validation: mvn -B -ntp verify passed on Java 17: 8,740 tests reported, 30 skipped, no failures or errors. The update adds 93 regressions covering bounds, count limits, large exponents, conversion hooks, and JSON/YAML string/stream readers with and without locations. NumericReaderBenchmark adds parsing workloads alongside the existing integral-validation benchmark.

Corresponding PR: #1291.

[Copilot]

Copilot review overview

🟢 Approval recommended

The focused implementation addresses the precision defect while retaining existing floating-point behavior and includes comprehensive regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Preserves exact numeric operands during multipleOf validation, preventing precision loss for large integers and extreme decimals.

Changes:

• Uses exact BigDecimal representations for integral and decimal operands.
• Adds regression coverage for large integers, BigInteger, and extreme decimal values.

File	Description
MultipleOfValidator.java	Preserves exact divisors and dividends.
MultipleOfValidatorTest.java	Adds precision regression tests.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[stevehu]

Code review(xhigh · 8 findings)
src/main/java/com/networknt/schema/keyword/MultipleOfValidator.java
● 66 [correctness] The new exact divisor branch drops the old limit that came from converting through double. A BigDecimal divisor with a huge negative exponent is now used as-is, and divideAndRemainder then does work that grows with the exponent difference.
● 98 [security] An attacker who controls the instance can make getDividend/validate burn CPU for a long time with a short string (type-loose mode) or a short BigDecimal literal. The huge exponent goes straight into divideAndRemainder. This already happens before the PR, but it is in the functions this PR rewrites.
● 94 [correctness] Issue #1286 is only fixed for integer literals. With the default mapper, an integer-valued float literal is still a DoubleNode, so it gets rounded through double on both the dividend and divisor sides.
● 66 [correctness] The new integer branch calls stripTrailingZeros() on values that already have scale 0. The error message then shows round integer divisors in scientific notation, because line 50 uses divisor.toString().
● 67 [correctness] The new exact branch's guard is signum() != 0, so it accepts negative divisors. The spec requires multipleOf to be strictly greater than 0.
● 94 [simplification] The isIntegralNumber() || isBigDecimal() ? decimalValue() : BigDecimal.valueOf(doubleValue()) split is unnecessary. For DoubleNode and FloatNode, decimalValue() already returns exactly BigDecimal.valueOf(doubleValue()).
● 47 [efficiency] The most common case, an int or long divisor with an int or long instance, still allocates two BigDecimals and runs divideAndRemainder on every validation. A long fast path would avoid all of that.
src/test/java/com/networknt/schema/MultipleOfValidatorTest.java
● 191 [test-coverage] No test checks the error message for the new exact divisor branch. The decimal edge cases also use hand-built DecimalNodes instead of a USE_BIG_DECIMAL_FOR_FLOATS reader, so the configuration that actually produces such divisors is never exercised.

I found 8 issues in PR #1290. The fix works for integer literals, but there are two problems worth raising before merge:

• New slowdown from extreme divisors: a divisor like 1e-20000000 used to be ignored and now makes each validation take about 40 seconds. This only happens when the schema is read with BigDecimal floats or built in code.
• Incomplete fix: 9007199254740993.0 is still accepted as a multiple of 2 with the default parser.

I also flagged an existing slowdown in the same functions that untrusted instance data can trigger: with typeLoose enabled, the string instance "1e5000000" took 3.8 seconds to validate.

I ran each of these on the PR branch to get the timings and behavior. I used a temporary worktree and branch, both now removed, and your feat/validation-execution-limits checkout was not touched.

[PHJ2000]

Thanks for the review. Both PRs have been updated (55e08f1 on master, d4a390c on 2.x).

1. Small divisors: divisibility uses modular powers of ten instead of constructing an expanded quotient. The large-exponent divisor case has a timeout regression test.
2. Large instance exponents: the same calculation handles DecimalNode and loose-type string inputs, with tests for both.
3. Decimal literals: the default JSON and YAML readers now enable USE_BIG_DECIMAL_FOR_FLOATS. 9007199254740993.0 is rejected as a multiple of 2 without custom configuration. Tests cover both operands, exponent notation, strings and streams. Custom mappers remain caller-configurable.
4. Error messages: integral divisors retain their plain integer representation, so 100 is reported as 100, not 1E+2.
5. Non-positive divisors: zero and negative values now throw SchemaException with multipleOf must be greater than zero. Tests cover integer, decimal and BigInteger values.
6. Conversion: getDividend uses node.decimalValue(), retaining each branch’s non-finite handling.
7. Integral fast path: int/long operands use a cached long divisor and %, avoiding per-instance decimal conversion. Tests cover zero, negative instances and long boundaries. MultipleOfBenchmark is included: local JMH measurements on pre-parsed valid inputs changed from 43/46 to 29/28 ns/op (int/long, Java 17), and 102/114 to 90/89 ns/op (Java 8). Each measurement used 2 forks, 3 warmup and 3 measurement iterations of 500 ms; these are workload-specific results.
8. Coverage: message assertions, default and configured decimal readers, extreme scales, and 2,000 comparisons against BigDecimal remainder are covered.

Full mvn -B verify passed: master on Java 17, 8,631 tests with 30 skipped; 2.x on Java 8, 8,545 tests with 15 skipped. The README documents exact default parsing and rejection of non-positive divisors.

[stevehu]

Code review(xhigh · 14 findings)
src/main/java/com/networknt/schema/keyword/MultipleOfValidator.java
● 76 [correctness] dividend.stripTrailingZeros() throws ArithmeticException("Overflow") when the dividend's unscaled value has trailing zeros and its scale is near Integer.MIN_VALUE, so validate() crashes instead of reporting a result.
● 76 [security] On JDK 17 (the project baseline) and Java 8 (2.x), stripTrailingZeros() strips one zero per BigInteger division. That is quadratic in the number of trailing zeros, so a type-loose numeric string reintroduces the instance-controlled CPU burn from the previous review.
● 111 [correctness] With a custom double-based mapper, which the README says remains supported, a positive divisor literal that underflows to 0.0 now throws SchemaException("multipleOf must be greater than zero") at schema load.
● 142 [correctness] In type-loose mode, new BigDecimal(node.asString()) throws NumberFormatException for numeric strings whose exponent exceeds the int range. Strings.isNumeric accepts these strings, so the exception escapes validate().
● 43 [simplification] longDivisor and divisorText are derived from schemaNode, not from the computed divisor, and the int/long fast path skips getDividend. Overrides of the protected getDivisor/getDividend are bypassed, and integral DecimalNode divisors (2.0, 1e2, now the default node type for those literals) miss the fast path and still render as 1E+2.
● 103 [simplification] getDivisor keeps two branches with duplicated throws even though DoubleNode.decimalValue() already equals BigDecimal.valueOf(doubleValue()) (getDividend relies on this). divisor != 0 on line 114 is now dead after the <= 0 throw.
● 83 [efficiency] this.divisor.unscaledValue().abs() allocates a new BigInteger (two when compact) on every decimal-path validation, even though it depends only on the divisor.
src/main/java/com/networknt/schema/serialization/JsonMapperFactory.java
● 31 [correctness] With USE_BIG_DECIMAL_FOR_FLOATS on by default, untrusted instances now reach EnumValidator and UniqueItemsValidator as arbitrary-scale DecimalNodes. Their existing stripTrailingZeros() calls overflow and throw. The same applies to YamlMapperFactory.
● 31 [compatibility] JsonMapperFactory.getInstance() and YamlMapperFactory.getInstance() are public, and doc/quickstart.md and doc/walkers.md tell users to call them. Enabling USE_BIG_DECIMAL_FOR_FLOATS on the singleton changes every caller's untyped deserialization, not just schema/instance node reading.
● 31 [correctness] Large integral literals above double range used to parse as Infinity and fail type: integer. They now parse as integral DecimalNodes, pass the type check, and hit node.asLong() in the integer branches of Minimum/Maximum/ExclusiveMinimum/ExclusiveMaximum, which throws JsonNodeException.
● 32 [efficiency] Making DecimalNode the default for every float roughly doubles retained memory for float-heavy instances and slows numeric validation. Every default-reader user pays this cost for a multipleOf precision fix.
● 32 [compatibility] Switching schema literals to DecimalNode changes user-visible messages for exponent-notated numbers, and makes previously loadable schemas with out-of-int-range counts fail to load.
src/test/java/com/networknt/schema/MultipleOfValidatorTest.java
● 255 [test-coverage] The extreme-scale tests only use unscaled values with no trailing zeros (3, 1), so the stripTrailingZeros overflow and the type-loose trailing-zero cost are untested. Nothing exercises enum/uniqueItems or integer min/max against the new default DecimalNodes.
● 234 [test-coverage] exactDecimalsFromReader and largeExponentsFromReaderAndLooseType build a custom USE_BIG_DECIMAL_FOR_FLOATS mapper that is now identical to the default. They no longer test a separate configuration, and their rows duplicate exactDecimalsWithDefaultReaders. The plain-double custom mapper, which the README still supports, has no test.

I reviewed PR #1290 again after the updates, and three new problems block merge. Most of the eight items from the last review are fixed, but the new code and the switch to BigDecimal default readers bring in new crashes and a slowdown. The 14 findings are in the review panel; I reproduced the behavior findings on the PR branch and compared them with master.

Blocking:

• New crash in multipleOf: the instance 100e2147483647 now makes validate() throw ArithmeticException. Master handled it normally.
• Same crash in enum and uniqueItems: these keywords now receive the same values because of the new default readers, and they throw the same exception.
• Slowdown is back: with type-loose mode on JDK 17 (the project's minimum Java version), a 200,000-digit string ending in zeros takes 8.2 s to validate; master takes 0.35 s. It doesn't show up on JDK 25, which is probably why the author's runs missed it.

Compatibility costs of the new default readers:

• JsonMapperFactory.getInstance() is public and the docs tell users to call it. It now returns BigDecimal instead of Double for decimal numbers in maps, so existing user code that casts to Double will break.
• Instance numbers above about 1e308 under an integer schema with a minimum or maximum now throw instead of failing the type check. Master already threw for 1e19 to 1e308.
• Error messages change: a schema value written as 1e2 now reads 1E+2 instead of 100.0.
• A document of 2M floats now uses about twice the memory (58 MB → 119 MB).

With a user-supplied mapper: a positive divisor like 1e-400 rounds to 0.0 and is now rejected as "multipleOf must be greater than zero". Master ignored it.

The rest are an error in type-loose mode that predates this PR, a few cleanups, and missing tests. I removed the temporary worktrees and the pr-1290 branch I created; your checkout wasn't touched.

[PHJ2000]

Thanks for the reproductions and the detailed review. I have addressed the three merge blockers and added regression coverage.

Updates are pushed in c84af35 (master, #1290) and 15cda47 (2.x, #1291).

• Fixed normalization overflow for extreme numbers such as 100e2147483647 in multipleOf, enum, and uniqueItems.
• Reduced repeated large-integer operations when processing numbers with many trailing zeros. In my local JDK 17 reproduction, validation of the 200,000-digit input went from approximately 10 seconds to 0.39 seconds.
• Restored the existing behavior of the public JSON/YAML mappers while preserving exact numeric values in the default readers used for validation. Ordinary decimals remain DoubleNodes when that representation preserves their decimal value. Locally measured retained memory for two million decimal values was approximately 56.8 MiB for both the baseline and the updated implementation.

I also addressed underflow with a user-supplied double-based mapper, protected-method overrides, bounds validation for large numbers, count limits outside the int range, and error messages. The README's Numeric Precision section is updated accordingly.

Exact numeric handling is needed beyond multipleOf. In an experiment that bypassed exact parsing for schemas without multipleOf or range constraints, six valid inputs covering const, enum, $ref, allOf, uniqueItems, and integer validation failed. The implementation therefore preserves numeric accuracy throughout the default reader while reducing the processing cost for ordinary numbers.

Full mvn verify, including the existing multipleOf tests, passed on both branches.

Performance on JDK 17 (10,000-number arrays): ordinary decimals were 7.5% faster and scientific notation 5.7% faster; integer parsing took 4.2% longer (95% CI +0.9% to +7.5%). Decimals with nine fractional digits averaged 4.9% longer (95% CI −0.3% to +10.0%). Component-isolation measurements point to parser wrapping as the source of the integer overhead in the current exact-reading implementation. Details below.

Performance measurements and full test results

The baseline is master commit 51dab248335c4e738652b7c88d5ed7b6bf9fb71c.

The following measurements cover parsing an array of 10,000 numbers on JDK 17, pinned to logical CPU 0 with taskset -c 0, with a fixed 512 MiB heap, four one-second warmup iterations, and four one-second measurement iterations. The JMH GC profiler was enabled. Each version was measured in six independent JVM forks, with execution order alternated across rounds. Confidence intervals are 95% intervals for the paired relative changes across rounds.

Input	Baseline → updated	Mean time change	95% confidence interval
Integers	105.81 → 110.26 µs	+4.2%	+0.9% to +7.5%
Ordinary decimals	195.24 → 180.59 µs	−7.5%	−11.1% to −3.8%
Decimals with nine fractional digits	237.00 → 248.55 µs	+4.9%	−0.3% to +10.0%
Scientific notation	230.31 → 217.04 µs	−5.7%	−8.7% to −2.8%

Ordinary decimals and scientific notation improved, while integer parsing showed additional cost. For decimals with nine fractional digits, the observed mean increased, with a confidence interval that includes zero.

On 2.x, nextFieldName() now delegates to the underlying parser's fast path. On JDK 8, stream parsing of 2,000 objects averaged 167.47 µs before this delegation change and 163.44 µs afterward. The mean relative change was −2.4%, with a 95% confidence interval of −4.9% to +0.2%: a small improvement trend within measurement uncertainty.

Branch	JDK	Reported test count	Failures / errors	Skipped
master	17	8,647	0 / 0	30
2.x	8	8,561	0 / 0	15

Please take another look at the updated changes.

[stevehu]

Code review(xhigh · 14 findings)
src/main/java/com/networknt/schema/keyword/MaximumValidator.java
● 103 [correctness] Min/max comparisons switched from new BigDecimal(node.asString()) to decimalValue(). In Jackson 3.2.3, FloatNode.decimalValue() widens the float to double first (f2d then BigDecimal.valueOf), so 0.1f compares as 0.10000000149011612 instead of "0.1". The same change appears in Minimum, ExclusiveMaximum and ExclusiveMinimum, on both the schema side and the instance side.
● 104 [correctness] The lines this commit edits in minimum, maximum, exclusiveMinimum and exclusiveMaximum still call new BigDecimal(node.asString()) on type-loose numeric strings. A NumberFormatException for an out-of-range exponent escapes validate(). The same commit catches exactly this case in MultipleOfValidator.
● 70 [simplification] Inside the else if (node.isString()) branch, the new node.isNumber() ? node.decimalValue() : ... ternary can never take its first arm, because a string node is never a number.
src/main/java/com/networknt/schema/serialization/ExactMapperFactory.java
● 183 [correctness] With locationAware(), JsonNodes.readTree replaces ExactNumberNodeFactory with LocationJsonNodeFactory. That factory never converts back to double, so a literal on the BIG_DECIMAL path always becomes a DecimalNode, while short literals still become DoubleNode. Numerically equal values can therefore end up as different node classes, and nested object equality fails.
● 55 [correctness] The default readers disable STRIP_TRAILING_BIGDECIMAL_ZEROES, and DecimalNode.equals uses BigDecimal.equals, which compares scale. Two values that cannot be represented as doubles, are numerically equal, and differ only in trailing zeros therefore produce unequal nodes.
● 170 [robustness] getDoubleValue() returns compactValue whenever compact is true, but only getNumberTypeFP() resets the flag. A caller that reads a later float token's double without calling getNumberTypeFP() first gets the previous token's value.
● 198 [efficiency] Float literals that miss the compact fast path now cost several conversions each, where the base reader did one double parse. This covers anything over 17 chars or over 15 significant digits, which includes typical machine-serialized doubles. Each such literal goes through a BigDecimal parse, BigDecimal.doubleValue() (a string round trip for non-compact coefficients on JDK 17), Double.toString, a second BigDecimal parse, and compareTo.
src/main/java/com/networknt/schema/keyword/MultipleOfValidator.java
● 79 [correctness] A type-loose numeric string whose exponent overflows BigDecimal is always reported as invalid, even when the value is mathematically a multiple of the divisor.
src/main/java/com/networknt/schema/keyword/MaxItemsValidator.java
● 38 [correctness] In a touched constructor, maxItems still falls back to max = 0 for non-integral or non-finite values, while maxLength and maxProperties fall back to Integer.MAX_VALUE.
● 35 [correctness] The min* validators gained exceedsIntegerRange so their messages report the original schema value. The max* validators saturate negative out-of-range limits to Integer.MIN_VALUE and report that value instead.
src/main/java/com/networknt/schema/keyword/MinLengthValidator.java
● 37 [correctness] The new exceedsIntegerRange initializer dereferences schemaNode before the existing schemaNode != null guard on line 40. That makes the guard dead code and adds an NPE where the old code tolerated null.
README.md
● 578 [documentation] The README says "With the default validation readers, zero and negative divisors raise SchemaException". In fact, integer 0, negative integers and negative doubles throw with any reader. Only a double zero from a custom double mapper is ignored.
src/main/java/com/networknt/schema/keyword/MinimumValidator.java
● 110 [efficiency] The non-integer threshold calls schemaNode.decimalValue() inside crossesThreshold, so it rebuilds the schema's BigDecimal on every validation instead of computing it once in the constructor.
src/main/java/com/networknt/schema/keyword/MinItemsValidator.java
● 39 [reuse] The same saturating int conversion, canConvertToInt() ? intValue() : decimalValue().signum() > 0 ? MAX : MIN, is copied into six validators, and the exceedsIntegerRange expression into three. A count already saturated to Integer.MAX_VALUE makes the exceedsIntegerRange || branch in the conditions redundant.

I reviewed the PR owner's latest commit, c84af35 ("Fix numeric edge cases and preserve mapper compatibility"), and reported 14 findings. I checked Jackson's behaviour against the 3.1.1 sources and the actual 3.2.3 classes, but ran no tests.

The four that matter most:

1. Float values now compare wrongly in min/max checks. The bound validators switched from parsing the node's text to decimalValue(). In Jackson 3.2.3, a 32-bit float such as 0.1f then becomes 0.10000000149011612. Validating a tree built with valueToTree from a POJO with float fields against maximum: 0.1 now fails; it passed before.
2. Location-aware reading breaks equality inside objects. With locationAware(), the PR's number handling is bypassed for long literals. So 1.10 and 1.1000000000000000000 end up as different node types, and const or enum comparisons on objects fail where they matched before.
3. Huge exponents still crash min/max in type-loose mode. A numeric string like "1e2147483648" throws an exception out of minimum/maximum. The same commit fixes exactly this case for multipleOf only.
4. Equal high-precision decimals can compare unequal. The default reader keeps trailing zeros on values a double can't hold exactly, and Jackson's equality check counts those zeros. So 9007199254740993.0 and 9007199254740993.00 don't match inside a nested const.

The rest are lower priority: a few smaller edge cases, a README line that misstates when zero or negative multipleOf values throw, extra parsing cost for long machine-written doubles (the benchmark only covers integers), and some duplicated or dead code.

I also suspected the compact-double shortcut might be wrong on JDK 17 (the library's minimum Java version), but I couldn't test that here. The PR's 10,000-case random test reportedly passes on Java 17, so I left it out.

[PHJ2000]

Thanks for the detailed review. Updated in 1ffd27f, with the 2.x port in #1291 (59c4691).

• 1–3, 13: All four bounds now preserve FloatNode decimal text, compare loose numbers beyond BigDecimal's scale range, remove the unreachable ternaries, and cache finite thresholds during construction.
• 4–5: Type selection is in the parser, so ordinary and location-aware readers receive consistent node types. Remaining decimals use overflow-safe normalization for nested equality and hashes.
• 6: The compact-value cache is tied to the current input context and token offset, with a stateless fallback when locations are unavailable. Tests cover token/name/container advancement and parser sequences that restart at the same offset.
• 7: Canonical finite, nonzero Double.toString output avoids decimal parsing. Added JSON/YAML conversion-count regressions and NumericReaderBenchmark.
• 8: Loose multipleOf operands outside BigDecimal's range use coefficient/exponent arithmetic. The fallback carries the value selected by getDividend, including overrides that transform the original input, and does not swallow unrelated hook exceptions.
• 9–11, 14: CountLimit shares conversion, fallback and error arguments. Non-integral/non-finite maxItems now defaults to Integer.MAX_VALUE, matching the other maxima. Original out-of-range values appear in errors, and MinLength tolerates null again. The minimum-overflow distinction remains necessary: minItems=2147483648 must reject size()=2147483647; a regression checks that boundary without allocating billions of elements.
• 12: README now distinguishes exact-zero/finite-negative rejection from floating-point-zero and non-finite compatibility.

Full local verify passed: Java 17, 8,740 tests reported / 30 skipped; Java 8 on 2.x, 8,654 / 15 skipped. No failures or errors. The new 93 regressions cover the reviewed behavior and compatibility cases.

I also measured parsing arrays of 1,000 values before/after this update, using JMH on one pinned CPU, 2 forks, 3×300 ms warmup and 3×300 ms measurement per fork (microseconds per array):

Input	Java 17 before → after	Java 8 before → after
Short decimals (i.25)	26.89 → 24.32	24.20 → 26.49
Serialized doubles (Double.toString(Math.PI * (i+1)))	364.93 → 207.89	364.27 → 295.60
High-precision decimals	636.16 → 617.91	427.78 → 413.77

Java 17 allocation for serialized doubles fell from about 590 KB to 139 KB per array. Java 8's allocation profiler returned implausibly small values, so I am not using those allocation numbers. The short-decimal Java 8 timings have overlapping JMH error ranges; this small local run does not establish a general performance guarantee.