Emit shortest round-trip decimal for numbers (RFC 8785 §3.2.2.3) - #7
Open
dngr2 wants to merge 1 commit into
Open
Emit shortest round-trip decimal for numbers (RFC 8785 §3.2.2.3)#7dngr2 wants to merge 1 commit into
dngr2 wants to merge 1 commit into
Conversation
The number serializer formatted values with "%.15E", which yields 16 significant digits. IEEE-754 doubles need up to 17 to round-trip, so any value requiring the 17th digit was canonicalized to a string that parses back to a *different* double -- a direct violation of RFC 8785 §3.2.2.3, which requires the ECMAScript shortest form. For example 0.1 + 0.2 became "0.3". Large integers were also coerced through the 16-digit float path and lost precision (e.g. 295147905179352830000 -> ...800000). Take the shortest digit sequence from Float#to_s (which already produces the shortest round-tripping representation) instead, and normalize it into the single-leading-digit form the existing exponent logic expects. The number_spec cases previously commented out as "Outside Ruby Range" were never outside Ruby's range -- they failed only because of the 16-digit limit. They are restored and pass, along with new round-trip regressions.
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.
The bug
Numeric#to_json_c14nserializes numbers with"%.15E" % num, which produces 16 significant digits. IEEE-754 doubles need up to 17 significant digits to round-trip. Any value that needs the 17th digit is canonicalized to a string that parses back to a different double — a direct violation of RFC 8785 §3.2.2.3, which mandates the ECMAScript ("shortest") number form.The canonical example:
Because a canonical form that doesn't round-trip defeats the entire purpose of canonicalization (two parties hashing "the same" value get different bytes), this bites real doubles constantly. Testing the current code against Node's
String(x)(the ES6 reference the RFC points to) over ~20k random doubles, 9,152 diverged. A few:0.1 + 0.20.30.300000000000000041.7976931348623157e308(Float::MAX)1.797693134862316e+3081.7976931348623157e+3085e-324(Float::MINsubnormal)4.940656458412465e-3245e-324123456789012345680.0123456789012345700123456789012345680Large integers are affected too — they're coerced through the same 16-digit float path:
This is exactly why
spec/number_spec.rbhad a block of cases commented out as "Outside Ruby Range". They aren't outside Ruby's range at all (5e-324,1e+23,Float::MAXare all perfectly representable) — they were commented out because%.15Ecouldn't produce their correct canonical form.The fix
Ruby's
Float#to_salready emits the shortest decimal that round-trips. This PR takes the digit sequence fromFloat#to_sinstead of%.15E, and normalizes it into the single-leading-digit form the existing exponent-placement logic already expects. That logic (positional vs. exponential thresholds,+/-exponent formatting,-0→0) is unchanged.Verification
String(x)over the same ~20k-double battery (edge cases + subnormals + random bit patterns): 0 divergences (was 9,152). End-to-end, a mixed nested object canonicalizes byte-identically to the referencecanonicalize.js.number_speccase still passes; theRangeErrorcases still raise.0.1 + 0.2,123456789012345680.0).rspec spec/number_spec.rb: 24 examples, 0 failures.