Skip to content

rdar://131793235 (Invalid JSON string causes precondition failure) #754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ internal func __NSStringToDecimal(
from: string.utf8,
decimalSeparator: ".".utf8,
matchEntireString: false
)
).asOptional
processedLength.pointee = parsed.processedLength
if let parsedResult = parsed.result {
result.pointee = parsedResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ internal import _ForSwiftFoundation
extension Decimal : CustomStringConvertible {
public init?(string: __shared String, locale: __shared Locale? = nil) {
let decimalSeparator = locale?.decimalSeparator ?? "."
guard let value = Decimal._decimal(
guard case let .success(value, _) = Decimal._decimal(
from: string.utf8,
decimalSeparator: decimalSeparator.utf8,
matchEntireString: false
).result else {
) else {
return nil
}
self = value
Expand Down
107 changes: 61 additions & 46 deletions Sources/FoundationEssentials/Decimal/Decimal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ extension Decimal {
decimalSeparator: String.UTF8View,
matchEntireString: Bool
) -> (result: Decimal?, processedLength: Int) {
_decimal(from: stringView, decimalSeparator: decimalSeparator, matchEntireString: matchEntireString)
_decimal(from: stringView, decimalSeparator: decimalSeparator, matchEntireString: matchEntireString).asOptional
}
#endif
internal func _toString(with locale: Locale? = nil) -> String {
Expand Down Expand Up @@ -250,11 +250,26 @@ extension Decimal {
return String(buffer.reversed())
}

internal static func _decimal(
from stringView: String.UTF8View,
decimalSeparator: String.UTF8View,
internal enum DecimalParseResult {
case success(Decimal, processedLength: Int)
case parseFailure
case overlargeValue

var asOptional: (result: Decimal?, processedLength: Int) {
switch self {
case let .success(decimal, processedLength): (decimal, processedLength: processedLength)
default: (nil, processedLength: 0)
}
}
}

@_specialize(where UTF8Collection == String.UTF8View)
@_specialize(where UTF8Collection == BufferView<UInt8>)
internal static func _decimal<UTF8Collection: Collection>(
from utf8View: UTF8Collection,
decimalSeparator: String.UTF8View = ".".utf8,
matchEntireString: Bool
) -> (result: Decimal?, processedLength: Int) {
) -> DecimalParseResult where UTF8Collection.Element == UTF8.CodeUnit {
func multiplyBy10AndAdd(
_ decimal: Decimal,
number: UInt16
Expand All @@ -268,47 +283,47 @@ extension Decimal {
}
}

func skipWhiteSpaces(from index: String.UTF8View.Index) -> String.UTF8View.Index {
func skipWhiteSpaces(from index: UTF8Collection.Index) -> UTF8Collection.Index {
var i = index
while i != stringView.endIndex &&
Character(utf8Scalar: stringView[i]).isWhitespace {
stringView.formIndex(after: &i)
while i != utf8View.endIndex &&
Character(utf8Scalar: utf8View[i]).isWhitespace {
utf8View.formIndex(after: &i)
}
return i
}

func stringViewContainsDecimalSeparator(at index: String.UTF8View.Index) -> Bool {
func stringViewContainsDecimalSeparator(at index: UTF8Collection.Index) -> Bool {
for indexOffset in 0 ..< decimalSeparator.count {
let stringIndex = stringView.index(index, offsetBy: indexOffset)
let stringIndex = utf8View.index(index, offsetBy: indexOffset)
let decimalIndex = decimalSeparator.index(
decimalSeparator.startIndex,
offsetBy: indexOffset
)
if stringView[stringIndex] != decimalSeparator[decimalIndex] {
if utf8View[stringIndex] != decimalSeparator[decimalIndex] {
return false
}
}
return true
}

var result = Decimal()
var index = stringView.startIndex
var index = utf8View.startIndex
index = skipWhiteSpaces(from: index)
// Get the sign
if index != stringView.endIndex &&
(stringView[index] == UInt8._plus ||
stringView[index] == UInt8._minus) {
result._isNegative = (stringView[index] == UInt8._minus) ? 1 : 0
if index != utf8View.endIndex &&
(utf8View[index] == UInt8._plus ||
utf8View[index] == UInt8._minus) {
result._isNegative = (utf8View[index] == UInt8._minus) ? 1 : 0
// Advance over the sign
stringView.formIndex(after: &index)
utf8View.formIndex(after: &index)
}
// Build mantissa
var tooBigToFit = false

while index != stringView.endIndex,
let digitValue = stringView[index].digitValue {
while index != utf8View.endIndex,
let digitValue = utf8View[index].digitValue {
defer {
stringView.formIndex(after: &index)
utf8View.formIndex(after: &index)
}
// Multiply the value by 10 and add the current digit
func incrementExponent(_ decimal: inout Decimal) {
Expand All @@ -324,7 +339,7 @@ extension Decimal {
if tooBigToFit {
incrementExponent(&result)
if result.isNaN {
return (result: nil, processedLength: 0)
return .overlargeValue
}
continue
}
Expand All @@ -333,20 +348,20 @@ extension Decimal {
tooBigToFit = true
incrementExponent(&result)
if result.isNaN {
return (result: nil, processedLength: 0)
return .overlargeValue
}
continue
}
result = product
}
// Get the decimal point
if index != stringView.endIndex && stringViewContainsDecimalSeparator(at: index) {
stringView.formIndex(&index, offsetBy: decimalSeparator.count)
if index != utf8View.endIndex && stringViewContainsDecimalSeparator(at: index) {
utf8View.formIndex(&index, offsetBy: decimalSeparator.count)
// Continue to build the mantissa
while index != stringView.endIndex,
let digitValue = stringView[index].digitValue {
while index != utf8View.endIndex,
let digitValue = utf8View[index].digitValue {
defer {
stringView.formIndex(after: &index)
utf8View.formIndex(after: &index)
}
guard !tooBigToFit else {
continue
Expand All @@ -360,38 +375,38 @@ extension Decimal {
// Before decrementing the exponent, we need to check
// if it's still possible to decrement.
if result._exponent == Int8.min {
return (result: nil, processedLength: 0)
return .overlargeValue
}
result._exponent -= 1
}
}
// Get the exponent if any
if index != stringView.endIndex && (stringView[index] == UInt8._E || stringView[index] == UInt8._e) {
stringView.formIndex(after: &index)
if index != utf8View.endIndex && (utf8View[index] == UInt8._E || utf8View[index] == UInt8._e) {
utf8View.formIndex(after: &index)
var exponentIsNegative = false
var exponent = 0
// Get the exponent sign
if stringView[index] == UInt8._minus || stringView[index] == UInt8._plus {
exponentIsNegative = stringView[index] == UInt8._minus
stringView.formIndex(after: &index)
if utf8View[index] == UInt8._minus || utf8View[index] == UInt8._plus {
exponentIsNegative = utf8View[index] == UInt8._minus
utf8View.formIndex(after: &index)
}
// Build the exponent
while index != stringView.endIndex,
let digitValue = stringView[index].digitValue {
while index != utf8View.endIndex,
let digitValue = utf8View[index].digitValue {
exponent = 10 * exponent + digitValue
if exponent > 2 * Int(Int8.max) {
// Too big to fit
return (result: nil, processedLength: 0)
return .overlargeValue
}
stringView.formIndex(after: &index)
utf8View.formIndex(after: &index)
}
if exponentIsNegative {
exponent = -exponent
}
// Check to see if it will fit into the exponent field
exponent += Int(result._exponent)
if exponent > Int8.max || exponent < Int8.min {
return (result: nil, processedLength: 0)
return .overlargeValue
}
result._exponent = Int32(exponent)
}
Expand All @@ -401,27 +416,27 @@ extension Decimal {
if matchEntireString {
// Trim end spaces
index = skipWhiteSpaces(from: index)
guard index == stringView.endIndex else {
guard index == utf8View.endIndex else {
// Any unprocessed content means the string
// contains something not valid
return (result: nil, processedLength: 0)
return .parseFailure
}
}
if index == stringView.startIndex {
if index == utf8View.startIndex {
// If we weren't able to process any character
// the entire string isn't a valid decimal
return (result: nil, processedLength: 0)
return .parseFailure
}
result.compact()
let processedLength = stringView.distance(from: stringView.startIndex, to: index)
let processedLength = utf8View.distance(from: utf8View.startIndex, to: index)
// if we get to this point, and have NaN,
// then the input string was probably "-0"
// or some variation on that, and
// normalize that to zero.
if result.isNaN {
return (result: Decimal(0), processedLength: processedLength)
return .success(Decimal(0), processedLength: processedLength)
}
return (result: result, processedLength: processedLength)
return .success(result, processedLength: processedLength)
}
}

Expand Down
40 changes: 17 additions & 23 deletions Sources/FoundationEssentials/JSON/JSONDecoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -727,20 +727,27 @@ extension JSONDecoderImpl: Decoder {
// TODO: Proper handling of Infinity and NaN Decimal values.
return Decimal.quietNaN
} else {
let numberString = String(decoding: numberBuffer, as: UTF8.self)
if let decimal = Decimal(entire: numberString) {
return decimal
switch Decimal._decimal(from: numberBuffer, matchEntireString: true) {
case .success(let result, _):
return result
case .overlargeValue:
throw JSONError.numberIsNotRepresentableInSwift(parsed: String(decoding: numberBuffer, as: UTF8.self))
case .parseFailure:
throw JSON5Scanner.validateNumber(from: numberBuffer.suffix(from: digitsStartPtr), fullSource: fullSource)
}
throw JSON5Scanner.validateNumber(from: numberBuffer.suffix(from: digitsStartPtr), fullSource: fullSource)

}

} else {
let digitsStartPtr = try JSONScanner.prevalidateJSONNumber(from: numberBuffer, hasExponent: hasExponent, fullSource: fullSource)
let numberString = String(decoding: numberBuffer, as: UTF8.self)
if let decimal = Decimal(entire: numberString) {
return decimal
switch Decimal._decimal(from: numberBuffer, matchEntireString: true) {
case .success(let result, _):
return result
case .overlargeValue:
throw JSONError.numberIsNotRepresentableInSwift(parsed: String(decoding: numberBuffer, as: UTF8.self))
case .parseFailure:
throw JSONScanner.validateNumber(from: numberBuffer.suffix(from: digitsStartPtr), fullSource: fullSource)
}
throw JSONScanner.validateNumber(from: numberBuffer.suffix(from: digitsStartPtr), fullSource: fullSource)
}
}
}
Expand Down Expand Up @@ -996,8 +1003,8 @@ extension JSONDecoderImpl: Decoder {
}
}

let number = String(decoding: numberBuffer, as: Unicode.ASCII.self)
if let decimal = Decimal(entire: number) {
let decimalParseResult = Decimal._decimal(from: numberBuffer, matchEntireString: true).asOptional
if let decimal = decimalParseResult.result {
guard let value = T(decimal) else {
throw JSONError.numberIsNotRepresentableInSwift(parsed: String(decoding: numberBuffer, as: UTF8.self))
}
Expand Down Expand Up @@ -1072,19 +1079,6 @@ extension FixedWidthInteger {
}
}

extension Decimal {
init?(entire string: String) {
guard let value = Decimal._decimal(
from: string.utf8,
decimalSeparator: ".".utf8,
matchEntireString: true
).result else {
return nil
}
self = value
}
}

extension JSONDecoderImpl : SingleValueDecodingContainer {
func decodeNil() -> Bool {
switch topValue {
Expand Down
5 changes: 5 additions & 0 deletions Tests/FoundationEssentialsTests/JSONEncoderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2613,6 +2613,11 @@ extension JSONEncoderTests {
let testBigDecimal = TestBigDecimal()
_testRoundTrip(of: testBigDecimal)
}

func testOverlargeDecimal() {
// Check value too large fails to decode.
XCTAssertThrowsError(try JSONDecoder().decode(Decimal.self, from: "100e200".data(using: .utf8)!))
}
}

// MARK: - Framework-only tests
Expand Down