apple / swift-foundation

The Foundation project
Apache License 2.0
2.28k stars 135 forks source link

Limit JSONDecoder's fallback Double path to lossless integer range #682

Closed oscbyspro closed 1 week ago

oscbyspro commented 2 weeks ago

Hello again, friends!

This patch is the sequel to (#667) that fixes JSONDecoder's Int64 clamping-esque behavior (#613). 

It also fixes a bug in _parseJSON5Integer(_:isHex:), which forgot to drop the sign for decimal inputs.

itingliu commented 2 weeks ago

@swift-ci please test

oscbyspro commented 1 week ago

Just some clarification: this solution :tm: only prevents floating-point errors >=(±1).

The fallback Double path still introduces lossy behavior for smaller JSON values that cannot be represented by Double. A proper solution would validate the input String in the parsing step. I suppose ICU doesn't provide integer validation APIs?  

try! JSONDecoder().decode(Int64.self, from: "9007199254740991.0".data(using: .utf8)!) // 2^53 - 1, OK
try! JSONDecoder().decode(Int64.self, from: "9007199254740990.5".data(using: .utf8)!) // 2^53 - 2, :(
try! JSONDecoder().decode(Int64.self, from: "9007199254740990.0".data(using: .utf8)!) // 2^53 - 2, OK
itingliu commented 1 week ago

LGTM. Thanks!