Skip to content

Commit

Permalink
Merge pull request #935 from tbkka/tbkka-jsonString-gardening
Browse files Browse the repository at this point in the history 
JSON improvements:  test both string- and data-based APIs, fix inconsistencies
  • Loading branch information
@tbkka
tbkka authored Jan 6, 2020
2 parents d108fa9 + e4811ab commit cb82b11
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 22 deletions.
31 changes: 17 additions & 14 deletions Sources/SwiftProtobuf/Message+JSONAdditions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ extension Message {
public func jsonString(
options: JSONEncodingOptions = JSONEncodingOptions()
) throws -> String {
if let m = self as? _CustomJSONCodable {
return try m.encodedJSONString(options: options)
}
let data = try jsonUTF8Data(options: options)
return String(data: data, encoding: String.Encoding.utf8)!
}
Expand Down Expand Up @@ -90,23 +93,23 @@ extension Message {
) throws {
self.init()
try jsonUTF8Data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
if body.count > 0 {
var decoder = JSONDecoder(source: body, options: options)
if !decoder.scanner.skipOptionalNull() {
try decoder.decodeFullObject(message: &self)
} else if Self.self is _CustomJSONCodable.Type {
if let message = try (Self.self as! _CustomJSONCodable.Type)
.decodedFromJSONNull() {
self = message as! Self
} else {
throw JSONDecodingError.illegalNull
}
// Empty input is valid for binary, but not for JSON.
guard body.count > 0 else {
throw JSONDecodingError.truncated
}
var decoder = JSONDecoder(source: body, options: options)
if decoder.scanner.skipOptionalNull() {
if let customCodable = Self.self as? _CustomJSONCodable.Type,
let message = try customCodable.decodedFromJSONNull() {
self = message as! Self
} else {
throw JSONDecodingError.illegalNull
}
if !decoder.scanner.complete {
throw JSONDecodingError.trailingGarbage
}
} else {
try decoder.decodeFullObject(message: &self)
}
if !decoder.scanner.complete {
throw JSONDecodingError.trailingGarbage
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion Tests/LinuxMain.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ extension Test_JSON {
("testRepeatedInt32", testRepeatedInt32),
("testRepeatedString", testRepeatedString),
("testRepeatedNestedMessage", testRepeatedNestedMessage),
("testOneof", testOneof)
("testOneof", testOneof),
("testEmptyMessage", testEmptyMessage)
]
}

Expand Down
51 changes: 49 additions & 2 deletions Tests/SwiftProtobufTests/TestHelpers.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,22 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
} catch let e {
XCTFail("Failed to serialize JSON: \(e)\n \(configured)", file: file, line: line)
}

do {
let encodedData = try configured.jsonUTF8Data()
let encodedOptString = String(data: encodedData, encoding: String.Encoding.utf8)
XCTAssertNotNil(encodedOptString)
let encodedString = encodedOptString!
XCTAssert(expected == encodedString, "Did not encode correctly: got \(encodedString)", file: file, line: line)
do {
let decoded = try MessageTestType(jsonUTF8Data: encodedData)
XCTAssert(decoded == configured, "Encode/decode cycle should generate equal object: \(decoded) != \(configured)", file: file, line: line)
} catch {
XCTFail("Encode/decode cycle should not throw error decoding: \(encodedString), but it threw \(error)", file: file, line: line)
}
} catch let e {
XCTFail("Failed to serialize JSON: \(e)\n \(configured)", file: file, line: line)
}
}

/// Verify the preferred encoding/decoding of a particular object.
Expand Down Expand Up @@ -200,8 +216,8 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
do {
let encoded = try decoded.jsonString()
do {
let redecoded = try MessageTestType(jsonString: json)
XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded)", file: file, line: line)
let redecoded = try MessageTestType(jsonString: encoded)
XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded) from \(encoded)", file: file, line: line)
XCTAssertEqual(decoded, redecoded, file: file, line: line)
} catch {
XCTFail("Swift should have recoded/redecoded without error: \(encoded)", file: file, line: line)
Expand All @@ -213,6 +229,29 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
XCTFail("Swift should have decoded without error but got \(e): \(json)", file: file, line: line)
return
}

do {
let jsonData = json.data(using: String.Encoding.utf8)!
let decoded: MessageTestType = try MessageTestType(jsonUTF8Data: jsonData)
XCTAssert(check(decoded), "Condition failed for \(decoded) from binary \(json)", file: file, line: line)

do {
let encoded = try decoded.jsonUTF8Data()
let encodedString = String(data: encoded, encoding: String.Encoding.utf8)!
do {
let redecoded = try MessageTestType(jsonUTF8Data: encoded)
XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded) from binary \(encodedString)", file: file, line: line)
XCTAssertEqual(decoded, redecoded, file: file, line: line)
} catch {
XCTFail("Swift should have recoded/redecoded without error: \(encodedString)", file: file, line: line)
}
} catch let e {
XCTFail("Swift should have recoded without error but got \(e)\n \(decoded)", file: file, line: line)
}
} catch let e {
XCTFail("Swift should have decoded without error but got \(e): \(json)", file: file, line: line)
return
}
}

func assertTextFormatDecodeSucceeds(_ text: String, file: XCTestFileArgType = #file, line: UInt = #line, check: (MessageTestType) throws -> Bool) {
Expand Down Expand Up @@ -278,6 +317,14 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
} catch {
// Yay! It failed!
}

let jsonData = json.data(using: String.Encoding.utf8)!
do {
let _ = try MessageTestType(jsonUTF8Data: jsonData, options: options)
XCTFail("Swift decode should have failed for binary: \(json)", file: file, line: line)
} catch {
// Yay! It failed again!
}
}

func assertTextFormatDecodeFails(_ text: String, file: XCTestFileArgType = #file, line: UInt = #line) {
Expand Down
38 changes: 33 additions & 5 deletions Tests/SwiftProtobufTests/Test_JSON.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,19 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeSucceeds("{\"optionalDouble\":\"1.5e+1\"}") {$0.optionalDouble == 15}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"15e-1\"}") {$0.optionalDouble == 1.5}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"1.0e0\"}") {$0.optionalDouble == 1.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"-0\"}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"0\"}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":-0}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":0}") {$0.optionalDouble == 0.0}
// We preserve signed zero when decoding
let d1 = try MessageTestType(jsonString: "{\"optionalDouble\":\"-0\"}")
XCTAssertEqual(d1.optionalDouble, 0.0)
XCTAssertEqual(d1.optionalDouble.sign, .minus)
let d2 = try MessageTestType(jsonString: "{\"optionalDouble\":-0}")
XCTAssertEqual(d2.optionalDouble, 0.0)
XCTAssertEqual(d2.optionalDouble.sign, .minus)
// But re-encoding treats the field as defaulted, so the sign gets lost
assertJSONDecodeSucceeds("{\"optionalDouble\":\"-0\"}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":-0}") {$0.optionalDouble == 0.0}

// Malformed numbers should fail
assertJSONDecodeFails("{\"optionalDouble\":Infinity}")
assertJSONDecodeFails("{\"optionalDouble\":-Infinity}") // Must be quoted
Expand Down Expand Up @@ -472,10 +481,13 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertRoundTripJSON {$0.optionalDouble = 2.22507385850720138309e-308}
}

func testOptionalFloat() {
func testOptionalFloat() throws {
assertJSONEncode("{\"optionalFloat\":1.0}") {(o: inout MessageTestType) in
o.optionalFloat = 1.0
}
assertJSONEncode("{\"optionalFloat\":-1.0}") {(o: inout MessageTestType) in
o.optionalFloat = -1.0
}
assertJSONEncode("{\"optionalFloat\":\"Infinity\"}") {(o: inout MessageTestType) in
o.optionalFloat = Float.infinity
}
Expand All @@ -485,6 +497,7 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeSucceeds("{\"optionalFloat\":\"Inf\"}") {$0.optionalFloat == Float.infinity}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-Inf\"}") {$0.optionalFloat == -Float.infinity}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1\"}") {$0.optionalFloat == 1}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-1\"}") {$0.optionalFloat == -1}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1.0\"}") {$0.optionalFloat == 1.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1.5\"}") {$0.optionalFloat == 1.5}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1.5e1\"}") {$0.optionalFloat == 15}
Expand All @@ -499,8 +512,16 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeSucceeds("{\"optionalFloat\":1.0e0}") {$0.optionalFloat == 1.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"0\"}") {$0.optionalFloat == 0.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":0}") {$0.optionalFloat == 0.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-0\"}") {$0.optionalFloat == -0.0 && $0.optionalFloat.sign == .minus}
assertJSONDecodeSucceeds("{\"optionalFloat\":-0}") {$0.optionalFloat == 0 && $0.optionalFloat.sign == .minus}
// We preserve signed zero when decoding
let d1 = try MessageTestType(jsonString: "{\"optionalFloat\":\"-0\"}")
XCTAssertEqual(d1.optionalFloat, 0.0)
XCTAssertEqual(d1.optionalFloat.sign, .minus)
let d2 = try MessageTestType(jsonString: "{\"optionalFloat\":-0}")
XCTAssertEqual(d2.optionalFloat, 0.0)
XCTAssertEqual(d2.optionalFloat.sign, .minus)
// But re-encoding treats the field as defaulted, so the sign gets lost
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-0\"}") {$0.optionalFloat == 0.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":-0}") {$0.optionalFloat == 0.0}
// Malformed numbers should fail
assertJSONDecodeFails("{\"optionalFloat\":Infinity}")
assertJSONDecodeFails("{\"optionalFloat\":-Infinity}") // Must be quoted
Expand Down Expand Up @@ -874,6 +895,13 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeFails("{\"oneofUint32\":1,\"oneofString\":\"abc\"}")
assertJSONDecodeFails("{\"oneofString\":\"abc\",\"oneofUint32\":1}")
}

func testEmptyMessage() {
assertJSONDecodeSucceeds("{}") {MessageTestType -> Bool in true}
assertJSONDecodeFails("")
assertJSONDecodeFails("{")
assertJSONDecodeFails("}")
}
}


Expand Down

0 comments on commit cb82b11

Please sign in to comment.