Repository navigation
openapi3: compare multipleOf on exact rationals, not fixed-precision decimals - #1252
Merged
Merged
Conversation
…decimals Both sides of the multipleOf check were formatted with "%.10f" before being parsed into big.Rat. That fixed precision is deliberate — getkin#1068 introduced it to absorb float64 representation error and getkin#1194 carried it across when decimal128 was removed — but the tolerance is absolute, so it is only correct inside a narrow band of magnitudes. Above roughly 1e6 the format demands more significant digits than a float64 carries, so it prints the binary error instead of the value: 999999999.99 becomes "999999999.9900000095" and is rejected even though the same schema declares it as the maximum. Below roughly 5e-11 every operand formats to "0.0000000000", which makes any value a multiple of anything and turns a small multipleOf into a zero denominator, so validating a document that merely declares multipleOf: 1e-12 panics with a division by zero. multipleOf: 0 and a negative multipleOf reach the same code and were never rejected. Compare exact rationals instead. big.Rat.SetFloat64 is exact, so there is no formatting step, no parse that can silently fail, and no denominator that can collapse. The tolerance becomes relative to the value: accept when the quotient is an integer, or when the nearest candidate multiple sits within |value| * 2^-51 of the value, which is a couple of units in the last place. Measuring against the value rather than against the quotient matters — a quotient-relative window grows without bound and would exceed one half around 2^49, accepting everything — and the candidate multiples are derived through big.Int so no quotient can overflow. Behaviour changes, all covered by tests: large magnitudes now validate; a multipleOf below the old decimal grid, including subnormals, now validates instead of panicking; multipleOf: 0 and negatives are reported instead of panicking or accepting everything; a value far below the multipleOf, such as 1e-300 against multipleOf: 0.01, is now rejected rather than treated as zero; and the window is now tied to float64 precision rather than to ten decimal places, so a value that differs from a multiple by more than a few ULP, such as 1.00000000004 against multipleOf: 1, is now rejected where the old grid rounded it in. Computed floats such as 0.1+0.2 against multipleOf: 0.1 are unaffected, which is why the new tests route them through a //go:noinline helper: written as a literal the compiler folds the expression exactly and the case would test nothing. Two paths are deliberately left alone because SchemaError renders the schema and the value as JSON and NaN has no JSON encoding, so reporting them would panic while formatting the error. A non-finite multipleOf is treated as no constraint, and a non-finite value keeps its current outcome. Both are noted in comments. Fixes getkin#1109
Collaborator
|
Thank you very much! |
This was referenced Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1109.
What breaks
multipleOfformats both operands with"%.10f"before parsing them intobig.Rat. That fixed precision is deliberate — #1068 introduced it to absorbfloat64 representation error, and #1194 carried it across when
decimal128wasremoved — but the tolerance is absolute, so it is only correct inside a narrow
band of magnitudes.
Above roughly
1e6, ten fractional digits behind a large integer part demandmore significant digits than a float64 carries, so the format prints the binary
error rather than the value.
999999999.99becomes"999999999.9900000095"andis rejected, even though the reporter's schema declares that same number as its
maximum:Below roughly
5e-11, every operand formats to"0.0000000000". A valuethen becomes a multiple of anything, and a small
multipleOfcollapses thedenominator to zero, so
big.Rat.Quopanics. Validating a document that merelydeclares a small
multipleOftakes the process down:multipleOf: 0panics the same way, and a negativemultipleOfaccepted everynumber.
Schema.Validatehas no check that would stop such a document fromloading, so this is reachable from an ordinary specification.
The change
Compare exact rationals instead of formatted decimals.
big.Rat.SetFloat64isexact, so there is no formatting step, no parse that can fail silently, and no
denominator that can collapse to zero.
The tolerance becomes relative to the value: accept when the quotient is an
integer, or when the nearest candidate multiple sits within
|value| * 2^-51ofthe value — a couple of units in the last place.
Measuring against the value rather than against the quotient matters. A
quotient-relative window grows without bound and would pass one half around
2^49, after which every number validates;2^50againstmultipleOf: 3is aconcrete case, and there is a regression test for it. The candidate multiples are
derived through
big.Int, so no quotient can overflow.Behaviour changes
999999999.99withmultipleOf: 0.01multipleOfbelow the old decimal grid, e.g.1e-12, incl. subnormalsmultipleOf: 0multipleOf must be greater than zeromultipleOfmultipleOf must be greater than zero1e-300withmultipleOf: 0.011.00000000004withmultipleOf: 1The last row is the one worth flagging: the window is now tied to float64
precision rather than to ten decimal places, so a value that differs from a
multiple by more than a few ULP is rejected where the old grid rounded it in.
Computed floats such as
0.1+0.2againstmultipleOf: 0.1are unaffected.One limit is inherent rather than chosen. When
multipleOfis smaller than thevalue's own ULP —
1e16againstmultipleOf: 3, where consecutive float64values are 2 apart — the question is not decidable from the stored number, and
the check accepts. No relative tolerance can both absorb one ULP at
0.3andreject a one-unit gap at
1e16.Two cases are deliberately left alone, with comments saying why: a non-finite
multipleOfis treated as no constraint, and a non-finite value keeps itscurrent outcome through the exported
VisitJSONNumber.SchemaError.Error()marshals both the value and the schema as JSON, and NaN has no JSON encoding, so
reporting either would panic while formatting the error — a worse outcome than
what is there now. Making
SchemaErrorrenderable for non-finite input is aseparate change. Note that
VisitJSONalready rejects non-finite input beforeany keyword runs; only the exported number entry point reaches this code.
Testing
TestIssue817gains the values from the report. A newTestSchemaMultipleOfAcrossMagnitudescovers the invariant by scenario: largemagnitudes, preserved tolerance for computed floats, a
multipleOfsmaller thanthe value's precision including a subnormal, a value far below the
multipleOf,zero, non-finite input,
multipleOfbounds, a no-panic case for every previouslycrashing input, and an assertion that the failure still names
multipleOfinSchemaError.SchemaField.The computed-float cases go through a
//go:noinlinehelper on purpose. Writtenas a literal,
0.1+0.2is folded exactly at compile time and the case wouldassert nothing.
go test ./openapi3/,go vet ./..., andgofmt -l .are clean.go test ./...is clean exceptopenapi2conv/TestIssue440, which failsidentically on an unpatched checkout of the same base
(
invalid character '.' looking for beginning of value) and is unrelated.