Skip to content

Commit

Permalink
Allow TextFormat to map extreme doubles to infinity.
Browse files Browse the repository at this point in the history
The TextFormat spec says this is required, JSON doesn't, so for now all JSON to
keep out existing behavior but bring TextFormat in line with that spec.
  • Loading branch information
thomasvl committed May 2, 2024
1 parent 0cb8176 commit 1804a61
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 9 deletions.
17 changes: 11 additions & 6 deletions Sources/SwiftProtobuf/DoubleParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal class DoubleParser {
return utf8ToDouble(bytes: UnsafeRawBufferPointer(rebasing: bytes[start..<end]))
}

func utf8ToDouble(bytes: UnsafeRawBufferPointer) -> Double? {
func utf8ToDouble(bytes: UnsafeRawBufferPointer, finiteOnly: Bool = true) -> Double? {
// Reject unreasonably long or short UTF8 number
if work.count <= bytes.count || bytes.count < 1 {
return nil
Expand All @@ -50,18 +50,23 @@ internal class DoubleParser {
var e: UnsafeMutablePointer<Int8>? = work.baseAddress
let d = strtod(work.baseAddress!, &e)

// Fail if strtod() did not consume everything we expected
// or if strtod() thought the number was out of range.
// Fail if strtod() did not consume everything we expected.
guard e == work.baseAddress! + bytes.count else {
return nil
}

// If strtod() thought the number was out of range, it will return
// a non-finite number...
//
// NOTE: TextFormat specifically calls out handling for overflows
// for float/double fields:
// TextFormat specifically calls out handling for overflows for
// float/double fields:
// https://protobuf.dev/reference/protobuf/textformat-spec/#value
//
// > Overflows are treated as infinity or -infinity.
//
// But the JSON protobuf spec doesn't mention anything:
// https://protobuf.dev/programming-guides/proto3/#json
if e != work.baseAddress! + bytes.count || !d.isFinite {
if finiteOnly && !d.isFinite {
return nil
}
return d
Expand Down
8 changes: 6 additions & 2 deletions Sources/SwiftProtobuf/TextFormatScanner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,9 @@ internal struct TextFormatScanner {
asciiUpperE: // 0...9, ., +, -, e, E
p += 1
case asciiLowerF, asciiUpperF: // f or F
let d = doubleParser.utf8ToDouble(bytes: UnsafeRawBufferPointer(start: start, count: p - start))
let d = doubleParser.utf8ToDouble(bytes: UnsafeRawBufferPointer(start: start,
count: p - start),
finiteOnly: false)
// Just skip the 'f'/'F'
p += 1
skipWhitespace()
Expand All @@ -835,7 +837,9 @@ internal struct TextFormatScanner {
break loop
}
}
let d = doubleParser.utf8ToDouble(bytes: UnsafeRawBufferPointer(start: start, count: p - start))
let d = doubleParser.utf8ToDouble(bytes: UnsafeRawBufferPointer(start: start,
count: p - start),
finiteOnly: false)
skipWhitespace()
return d
}
Expand Down
10 changes: 10 additions & 0 deletions Tests/SwiftProtobufTests/Test_TextFormatDecodingOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,16 @@ final class Test_TextFormatDecodingOptions: XCTestCase {
// This would overload a int, but as a floating point value it will map to "inf".
assertDecodeIgnoringUnknownsSucceeds("float", "999999999999999999999999999999999999")

// Things that round to infinity or zero, but should parse ok.
assertDecodeIgnoringUnknownsSucceeds("float", "1e50")
assertDecodeIgnoringUnknownsSucceeds("float", "-1e50")
assertDecodeIgnoringUnknownsSucceeds("float", "1e-50")
assertDecodeIgnoringUnknownsSucceeds("float", "-1e-50")
assertDecodeIgnoringUnknownsSucceeds("double", "1e9999")
assertDecodeIgnoringUnknownsSucceeds("double", "-1e9999")
assertDecodeIgnoringUnknownsSucceeds("double", "1e-9999")
assertDecodeIgnoringUnknownsSucceeds("double", "-1e-9999")

assertDecodeIgnoringUnknownsSucceeds("float", "nan")
assertDecodeIgnoringUnknownsSucceeds("float", "inf")
assertDecodeIgnoringUnknownsSucceeds("float", "-inf")
Expand Down
29 changes: 28 additions & 1 deletion Tests/SwiftProtobufTests/Test_TextFormat_proto3.swift
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,13 @@ final class Test_TextFormat_proto3: XCTestCase, PBTestHelpers {
(o: MessageTestType) in
return o.optionalFloat == -Float.infinity
}
// Too-large of values round to infinity
assertTextFormatDecodeSucceeds("optional_float: 1e50\n") {(o: MessageTestType) in
return o.optionalFloat == Float.infinity
}
assertTextFormatDecodeSucceeds("optional_float: -1e50\n") {(o: MessageTestType) in
return o.optionalFloat == -Float.infinity
}
// Too-small values round to zero (not currently checked by conformance)
assertTextFormatDecodeSucceeds("optional_float: 1e-50\n") {
(o: MessageTestType) in
Expand All @@ -366,7 +373,7 @@ final class Test_TextFormat_proto3: XCTestCase, PBTestHelpers {
XCTAssertEqual(msg.optionalFloat, 0.0)
XCTAssertEqual(msg.optionalFloat.sign, .minus)
} catch {
XCTFail("Shouldn't have throws on decode: \(error)")
XCTFail("Shouldn't have thrown on decode: \(error)")
}
// protobuf conformance requires subnormals to be handled
assertTextFormatDecodeSucceeds("optional_float: 1.17549e-39\n") {
Expand Down Expand Up @@ -493,6 +500,26 @@ final class Test_TextFormat_proto3: XCTestCase, PBTestHelpers {
assertTextFormatDecodeSucceeds("12: 1.0\n") {(o: MessageTestType) in
return o.optionalDouble == 1.0
}
// Too-large of values round to infinity
assertTextFormatDecodeSucceeds("optional_double: 1e9999\n") {(o: MessageTestType) in
return o.optionalDouble == Double.infinity
}
assertTextFormatDecodeSucceeds("optional_double: -1e9999\n") {(o: MessageTestType) in
return o.optionalDouble == -Double.infinity
}
// Too-small values round to zero (not currently checked by conformance)
assertTextFormatDecodeSucceeds("optional_double: 1e-9999\n") {
(o: MessageTestType) in
return o.optionalDouble == 0.0 && o.optionalDouble.sign == .plus
}
// The negative version won't really round trip, because -0.0 for proto3 counts as not set.
do {
let msg = try MessageTestType(textFormatString: "optional_double: -1e-9999\n")
XCTAssertEqual(msg.optionalDouble, 0.0)
XCTAssertEqual(msg.optionalDouble.sign, .minus)
} catch {
XCTFail("Shouldn't have thrown on decode: \(error)")
}
assertTextFormatDecodeSucceeds("optional_double: INFINITY\n") {(o: MessageTestType) in
return o.optionalDouble == Double.infinity
}
Expand Down

0 comments on commit 1804a61

Please sign in to comment.