From 218356bbd7771d2f6cda8da9fb2d66f9739c2772 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Thu, 14 May 2020 16:32:45 -0700 Subject: [PATCH 01/17] First cut at support for proto2 extensions in JSON This is motivated purely by the existence of a new JSON conformance test. These changes are sufficient to pass that new conformance test. --- Sources/Conformance/main.swift | 11 +++- Sources/SwiftProtobuf/JSONDecoder.swift | 33 +++++++++- Sources/SwiftProtobuf/JSONEncoder.swift | 12 ++++ .../SwiftProtobuf/JSONEncodingVisitor.swift | 13 ++-- Sources/SwiftProtobuf/JSONScanner.swift | 31 +++++++--- .../SwiftProtobuf/Message+JSONAdditions.swift | 61 +++++++++++++++++++ .../TextFormatEncodingVisitor.swift | 6 ++ SwiftProtobuf.xcodeproj/project.pbxproj | 8 +++ Tests/LinuxMain.swift | 7 +++ Tests/SwiftProtobufTests/TestHelpers.swift | 21 ++++--- .../Test_JSON_Extensions.swift | 45 ++++++++++++++ 11 files changed, 217 insertions(+), 31 deletions(-) create mode 100644 Tests/SwiftProtobufTests/Test_JSON_Extensions.swift diff --git a/Sources/Conformance/main.swift b/Sources/Conformance/main.swift index e3741c04b..234001cbe 100644 --- a/Sources/Conformance/main.swift +++ b/Sources/Conformance/main.swift @@ -85,6 +85,7 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse { } let msgType: SwiftProtobuf.Message.Type + let extensions: SwiftProtobuf.SimpleExtensionMap switch request.messageType { case "": // Note: This case is here to cover using a old version of the conformance test @@ -92,8 +93,10 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse { fallthrough case ProtobufTestMessages_Proto3_TestAllTypesProto3.protoMessageName: msgType = ProtobufTestMessages_Proto3_TestAllTypesProto3.self + extensions = SwiftProtobuf.SimpleExtensionMap() case ProtobufTestMessages_Proto2_TestAllTypesProto2.protoMessageName: msgType = ProtobufTestMessages_Proto2_TestAllTypesProto2.self + extensions = ProtobufTestMessages_Proto2_TestMessagesProto2_Extensions default: response.runtimeError = "Unexpected message type: \(request.messageType)" return response @@ -103,7 +106,7 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse { switch request.payload { case .protobufPayload(let data)?: do { - testMessage = try msgType.init(serializedData: data) + testMessage = try msgType.init(serializedData: data, extensions: extensions) } catch let e { response.parseError = "Protobuf failed to parse: \(e)" return response @@ -112,7 +115,9 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse { var options = JSONDecodingOptions() options.ignoreUnknownFields = (request.testCategory == .jsonIgnoreUnknownParsingTest) do { - testMessage = try msgType.init(jsonString: json, options: options) + testMessage = try msgType.init(jsonString: json, + extensions: extensions, + options: options) } catch let e { response.parseError = "JSON failed to parse: \(e)" return response @@ -124,7 +129,7 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse { return response case .textPayload(let textFormat)?: do { - testMessage = try msgType.init(textFormatString: textFormat) + testMessage = try msgType.init(textFormatString: textFormat, extensions: extensions) } catch let e { response.parseError = "Protobuf failed to parse: \(e)" return response diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index 6c63f654c..d8a9e3ace 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -17,6 +17,8 @@ import Foundation internal struct JSONDecoder: Decoder { internal var scanner: JSONScanner internal var options: JSONDecodingOptions + internal var extensions: ExtensionMap? + internal var messageType: Message.Type? private var fieldCount = 0 private var isMapKey = false private var fieldNameMap: _NameMap? @@ -32,10 +34,21 @@ internal struct JSONDecoder: Decoder { ignoreUnknownFields: self.options.ignoreUnknownFields) } + internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions, + messageType: Message.Type, extensions: ExtensionMap?) { + self.options = options + self.scanner = JSONScanner(source: source, + messageDepthLimit: self.options.messageDepthLimit, + ignoreUnknownFields: self.options.ignoreUnknownFields) + self.messageType = messageType + self.extensions = extensions + } + private init(decoder: JSONDecoder) { // The scanner is copied over along with the options. scanner = decoder.scanner options = decoder.options + extensions = decoder.extensions } mutating func nextFieldNumber() throws -> Int? { @@ -45,7 +58,10 @@ internal struct JSONDecoder: Decoder { if fieldCount > 0 { try scanner.skipRequiredComma() } - if let fieldNumber = try scanner.nextFieldNumber(names: fieldNameMap!) { + let fieldNumber = try scanner.nextFieldNumber(names: fieldNameMap!, + messageType: messageType, + extensionMap: extensions) + if let fieldNumber = fieldNumber { fieldCount += 1 return fieldNumber } @@ -697,6 +713,19 @@ internal struct JSONDecoder: Decoder { messageType: Message.Type, fieldNumber: Int ) throws { - throw JSONDecodingError.schemaMismatch + if let ext = extensions?[messageType, fieldNumber] { + var fieldValue = values[fieldNumber] + if fieldValue != nil { + try fieldValue!.decodeExtensionField(decoder: &self) + } else { + fieldValue = try ext._protobuf_newField(decoder: &self) + } + if fieldValue != nil { + values[fieldNumber] = fieldValue + } else { + // This most likely indicates a bug in our extension support. + throw TextFormatDecodingError.internalExtensionError + } + } } } diff --git a/Sources/SwiftProtobuf/JSONEncoder.swift b/Sources/SwiftProtobuf/JSONEncoder.swift index 6247076da..0a2079337 100644 --- a/Sources/SwiftProtobuf/JSONEncoder.swift +++ b/Sources/SwiftProtobuf/JSONEncoder.swift @@ -128,6 +128,18 @@ internal struct JSONEncoder { separator = asciiComma } + /// Begin a new extension field + internal mutating func startExtensionField(name: String) { + if let s = separator { + data.append(s) + } + data.append(asciiDoubleQuote) + data.append(asciiOpenSquareBracket) + data.append(contentsOf: name.utf8) + append(staticText: "]\":") + separator = asciiComma + } + /// Append an open square bracket `[` to the JSON. internal mutating func startArray() { data.append(asciiOpenSquareBracket) diff --git a/Sources/SwiftProtobuf/JSONEncodingVisitor.swift b/Sources/SwiftProtobuf/JSONEncodingVisitor.swift index 85110ee58..61de3eeb8 100644 --- a/Sources/SwiftProtobuf/JSONEncodingVisitor.swift +++ b/Sources/SwiftProtobuf/JSONEncodingVisitor.swift @@ -19,6 +19,7 @@ internal struct JSONEncodingVisitor: Visitor { private var encoder = JSONEncoder() private var nameMap: _NameMap + private var extensions: ExtensionFieldValueSet? private let options: JSONEncodingOptions /// The JSON text produced by the visitor, as raw UTF8 bytes. @@ -49,6 +50,7 @@ internal struct JSONEncodingVisitor: Visitor { } else { throw JSONEncodingError.missingFieldNames } + self.extensions = (message as? ExtensibleMessage)?._protobuf_extensionFieldValues self.options = options } @@ -363,11 +365,6 @@ internal struct JSONEncodingVisitor: Visitor { encoder.append(text: "}") } - /// Called for each extension range. - mutating func visitExtensionFields(fields: ExtensionFieldValueSet, start: Int, end: Int) throws { - // JSON does not store extensions - } - /// Helper function that throws an error if the field number could not be /// resolved. private mutating func startField(for number: Int) throws { @@ -379,8 +376,10 @@ internal struct JSONEncodingVisitor: Visitor { name = nameMap.names(for: number)?.json } - if let nm = name { - encoder.startField(name: nm) + if let name = name { + encoder.startField(name: name) + } else if let name = extensions?[number]?.protobufExtension.fieldName { + encoder.startExtensionField(name: name) } else { throw JSONEncodingError.missingFieldNames } diff --git a/Sources/SwiftProtobuf/JSONScanner.swift b/Sources/SwiftProtobuf/JSONScanner.swift index e72003e59..cb154de01 100644 --- a/Sources/SwiftProtobuf/JSONScanner.swift +++ b/Sources/SwiftProtobuf/JSONScanner.swift @@ -1250,29 +1250,42 @@ internal struct JSONScanner { /// Throws if field name cannot be parsed. /// If it encounters an unknown field name, it silently skips /// the value and looks at the following field name. - internal mutating func nextFieldNumber(names: _NameMap) throws -> Int? { + internal mutating func nextFieldNumber( + names: _NameMap, + messageType: Message.Type?, + extensionMap: ExtensionMap? + ) throws -> Int? { while true { + var fieldName: String if let key = try nextOptionalKey() { // Fast path: We parsed it as UTF8 bytes... try skipRequiredCharacter(asciiColon) // : if let fieldNumber = names.number(forJSONName: key) { return fieldNumber } - if !ignoreUnknownFields { - let fieldName = utf8ToString(bytes: key.baseAddress!, count: key.count)! - throw JSONDecodingError.unknownField(fieldName) - } + fieldName = utf8ToString(bytes: key.baseAddress!, count: key.count)! } else { // Slow path: We parsed a String; lookups from String are slower. - let key = try nextQuotedString() + fieldName = try nextQuotedString() try skipRequiredCharacter(asciiColon) // : - if let fieldNumber = names.number(forJSONName: key) { + if let fieldNumber = names.number(forJSONName: fieldName) { return fieldNumber } - if !ignoreUnknownFields { - throw JSONDecodingError.unknownField(key) + } + if let extensions = extensionMap, + let messageType = messageType, + let first = fieldName.first, first == "[", + let last = fieldName.last, last == "]" + { + fieldName.removeFirst() + fieldName.removeLast() + if let fieldNumber = extensions.fieldNumberForProto(messageType: messageType, protoFieldName: fieldName) { + return fieldNumber } } + if !ignoreUnknownFields { + throw JSONDecodingError.unknownField(fieldName) + } // Unknown field, skip it and try to parse the next field name try skipValue() if skipOptionalObjectEnd() { diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift index c76261cf9..2d1ba02e3 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions.swift @@ -1,3 +1,4 @@ + // Sources/SwiftProtobuf/Message+JSONAdditions.swift - JSON format primitive types // // Copyright (c) 2014 - 2016 Apple Inc. and the project authors @@ -79,6 +80,28 @@ extension Message { } } + /// Creates a new message by decoding the given string containing a + /// serialized message in JSON format. + /// + /// - Parameter jsonString: The JSON-formatted string to decode. + /// - Parameter extensions: An ExtensionMap + /// - Parameter options: The JSONDecodingOptions to use. + /// - Throws: `JSONDecodingError` if decoding fails. + public init( + jsonString: String, + extensions: ExtensionMap?, + options: JSONDecodingOptions = JSONDecodingOptions() + ) throws { + if jsonString.isEmpty { + throw JSONDecodingError.truncated + } + if let data = jsonString.data(using: String.Encoding.utf8) { + try self.init(jsonUTF8Data: data, extensions: extensions, options: options) + } else { + throw JSONDecodingError.truncated + } + } + /// Creates a new message by decoding the given `Data` containing a /// serialized message in JSON format, interpreting the data as UTF-8 encoded /// text. @@ -113,5 +136,43 @@ extension Message { } } } + + /// Creates a new message by decoding the given `Data` containing a + /// serialized message in JSON format, interpreting the data as UTF-8 encoded + /// text. + /// + /// - Parameter jsonUTF8Data: The JSON-formatted data to decode, represented + /// as UTF-8 encoded text. + /// - Parameter extensions: The extension map to use with this decode + /// - Parameter options: The JSONDecodingOptions to use. + /// - Throws: `JSONDecodingError` if decoding fails. + public init( + jsonUTF8Data: Data, + extensions: ExtensionMap?, + options: JSONDecodingOptions = JSONDecodingOptions() + ) throws { + self.init() + try jsonUTF8Data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in + // 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, + messageType: Self.self, extensions: extensions) + 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 + } + } else { + try decoder.decodeFullObject(message: &self) + } + if !decoder.scanner.complete { + throw JSONDecodingError.trailingGarbage + } + } + } } diff --git a/Sources/SwiftProtobuf/TextFormatEncodingVisitor.swift b/Sources/SwiftProtobuf/TextFormatEncodingVisitor.swift index 2ebe0f448..49d33b35d 100644 --- a/Sources/SwiftProtobuf/TextFormatEncodingVisitor.swift +++ b/Sources/SwiftProtobuf/TextFormatEncodingVisitor.swift @@ -63,6 +63,12 @@ internal struct TextFormatEncodingVisitor: Visitor { self.options = options } + // TODO: This largely duplicates emitFieldName() below. + // But, it's slower so we don't want to just have emitFieldName() use + // formatFieldName(). Also, we need to measure whether the optimization + // this provides to repeated fields is worth the effort; consider just + // removing this and having repeated fields just re-run emitFieldName() + // for each item. private func formatFieldName(lookingUp fieldNumber: Int) -> [UInt8] { var bytes = [UInt8]() if let protoName = nameMap?.names(for: fieldNumber)?.proto { diff --git a/SwiftProtobuf.xcodeproj/project.pbxproj b/SwiftProtobuf.xcodeproj/project.pbxproj index 34cd5ca21..695a287a0 100644 --- a/SwiftProtobuf.xcodeproj/project.pbxproj +++ b/SwiftProtobuf.xcodeproj/project.pbxproj @@ -10,6 +10,9 @@ 0353A1CE1E81623B00067996 /* any.pb.swift in Sources */ = {isa = PBXBuildFile; fileRef = F41BA43F1E7AE4AC004F6E95 /* any.pb.swift */; }; 0353A1CF1E81624F00067996 /* any.pb.swift in Sources */ = {isa = PBXBuildFile; fileRef = F41BA43F1E7AE4AC004F6E95 /* any.pb.swift */; }; 0353A1D01E81625400067996 /* any.pb.swift in Sources */ = {isa = PBXBuildFile; fileRef = F41BA43F1E7AE4AC004F6E95 /* any.pb.swift */; }; + 6583D6E4246E04A200353AF4 /* Test_JSON_Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6583D6E3246E04A200353AF4 /* Test_JSON_Extensions.swift */; }; + 6583D6E5246E04A200353AF4 /* Test_JSON_Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6583D6E3246E04A200353AF4 /* Test_JSON_Extensions.swift */; }; + 6583D6E6246E04A200353AF4 /* Test_JSON_Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6583D6E3246E04A200353AF4 /* Test_JSON_Extensions.swift */; }; 8DC1CA0D1E54B80800CA8A26 /* MathUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8DC1CA0C1E54B80800CA8A26 /* MathUtils.swift */; }; 8DC1CA0F1E54B81400CA8A26 /* TimeUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8DC1CA0E1E54B81400CA8A26 /* TimeUtils.swift */; }; 9C0B366B1E5FAB910094E128 /* JSONMapEncodingVisitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9C0B366A1E5FAB910094E128 /* JSONMapEncodingVisitor.swift */; }; @@ -717,6 +720,7 @@ 5A82E34B2455D26900EB0A70 /* SwiftProtobufTestSuite_iOS.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = SwiftProtobufTestSuite_iOS.xcconfig; path = SwiftProtobuf.xcodeproj/xcconfigs/SwiftProtobufTestSuite_iOS.xcconfig; sourceTree = ""; }; 5A82E34C2455D71400EB0A70 /* Base_Debug.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = Base_Debug.xcconfig; path = SwiftProtobuf.xcodeproj/xcconfigs/Base_Debug.xcconfig; sourceTree = ""; }; 5A82E34D2455D71400EB0A70 /* Base_Release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = Base_Release.xcconfig; path = SwiftProtobuf.xcodeproj/xcconfigs/Base_Release.xcconfig; sourceTree = ""; }; + 6583D6E3246E04A200353AF4 /* Test_JSON_Extensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Test_JSON_Extensions.swift; sourceTree = ""; }; 8DC1CA0C1E54B80800CA8A26 /* MathUtils.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MathUtils.swift; sourceTree = ""; }; 8DC1CA0E1E54B81400CA8A26 /* TimeUtils.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TimeUtils.swift; sourceTree = ""; }; 9C0B366A1E5FAB910094E128 /* JSONMapEncodingVisitor.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSONMapEncodingVisitor.swift; sourceTree = ""; }; @@ -1130,6 +1134,7 @@ F4539D241E688B000076251F /* Test_GroupWithGroups.swift */, DB2E0AF91EB24C7600F59319 /* Test_JSON_Array.swift */, __PBXFileRef_Tests/ProtobufTests/Test_JSON_Conformance.swift /* Test_JSON_Conformance.swift */, + 6583D6E3246E04A200353AF4 /* Test_JSON_Extensions.swift */, __PBXFileRef_Tests/ProtobufTests/Test_JSON_Group.swift /* Test_JSON_Group.swift */, __PBXFileRef_Tests/ProtobufTests/Test_JSON.swift /* Test_JSON.swift */, F4584DB21EDDC9D400803AB6 /* Test_JSONDecodingOptions.swift */, @@ -1425,6 +1430,7 @@ 9CC8CAB31EC512A0008EF45F /* generated_swift_names_fields.pb.swift in Sources */, F44F943B1DBFBB7500BC5B85 /* Test_BasicFields_Access_Proto3.swift in Sources */, F4151F781EFAD85B00EEA00B /* Test_BinaryDelimited.swift in Sources */, + 6583D6E5246E04A200353AF4 /* Test_JSON_Extensions.swift in Sources */, 9C8CDA271D7A28F600E207CA /* Test_Conformance.swift in Sources */, 9C8CDA291D7A28F600E207CA /* Test_Duration.swift in Sources */, 9C8CDA2A1D7A28F600E207CA /* Test_Empty.swift in Sources */, @@ -1724,6 +1730,7 @@ __src_cc_ref_Tests/ProtobufTests/Test_Duration.swift /* Test_Duration.swift in Sources */, __src_cc_ref_Tests/ProtobufTests/Test_Empty.swift /* Test_Empty.swift in Sources */, F4151F771EFAD85A00EEA00B /* Test_BinaryDelimited.swift in Sources */, + 6583D6E4246E04A200353AF4 /* Test_JSON_Extensions.swift in Sources */, F44F94431DBFE0BE00BC5B85 /* Test_OneofFields_Access_Proto3.swift in Sources */, __src_cc_ref_Tests/ProtobufTests/Test_Enum.swift /* Test_Enum.swift in Sources */, AA6CF6F51DB6D227007DF26B /* Test_Enum_Proto2.swift in Sources */, @@ -1934,6 +1941,7 @@ F44F93C81DAEA8C900BC5B85 /* Test_RecursiveMap.swift in Sources */, 9CC8CAB41EC512A0008EF45F /* generated_swift_names_fields.pb.swift in Sources */, F44F93D81DAEA8C900BC5B85 /* unittest_import_public_lite.pb.swift in Sources */, + 6583D6E6246E04A200353AF4 /* Test_JSON_Extensions.swift in Sources */, F4151F791EFAD85C00EEA00B /* Test_BinaryDelimited.swift in Sources */, F44F93E01DAEA8C900BC5B85 /* unittest_no_arena_import.pb.swift in Sources */, F44F93B81DAEA8C900BC5B85 /* Test_Empty.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index f5aad2fb5..5b046be1e 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -643,6 +643,12 @@ extension Test_JSON_Conformance { ] } +extension Test_JSON_Extensions { + static var allTests = [ + ("test_optionalInt32Extension", test_optionalInt32Extension) + ] +} + extension Test_JSON_Group { static var allTests = [ ("testOptionalGroup", testOptionalGroup), @@ -1186,6 +1192,7 @@ XCTMain( testCase(Test_JSONEncodingOptions.allTests), testCase(Test_JSON_Array.allTests), testCase(Test_JSON_Conformance.allTests), + testCase(Test_JSON_Extensions.allTests), testCase(Test_JSON_Group.allTests), testCase(Test_Map.allTests), testCase(Test_MapFields_Access_Proto2.allTests), diff --git a/Tests/SwiftProtobufTests/TestHelpers.swift b/Tests/SwiftProtobufTests/TestHelpers.swift index 9a8bf8c6c..50bc313a5 100644 --- a/Tests/SwiftProtobufTests/TestHelpers.swift +++ b/Tests/SwiftProtobufTests/TestHelpers.swift @@ -134,7 +134,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } - func assertJSONEncode(_ expected: String, file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout MessageTestType) -> Void) { + func assertJSONEncode(_ expected: String, extensions: ExtensionMap? = nil, file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout MessageTestType) -> Void) { let empty = MessageTestType() var configured = empty configure(&configured) @@ -143,7 +143,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable let encoded = try configured.jsonString() XCTAssert(expected == encoded, "Did not encode correctly: got \(encoded)", file: file, line: line) do { - let decoded = try MessageTestType(jsonString: encoded) + let decoded = try MessageTestType(jsonString: encoded, extensions: extensions) 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: \(encoded), but it threw \(error)", file: file, line: line) @@ -159,7 +159,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable let encodedString = encodedOptString! XCTAssert(expected == encodedString, "Did not encode correctly: got \(encodedString)", file: file, line: line) do { - let decoded = try MessageTestType(jsonUTF8Data: encodedData) + let decoded = try MessageTestType(jsonUTF8Data: encodedData, extensions: extensions) 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) @@ -208,15 +208,15 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } } - func assertJSONDecodeSucceeds(_ json: String, file: XCTestFileArgType = #file, line: UInt = #line, check: (MessageTestType) -> Bool) { + func assertJSONDecodeSucceeds(_ json: String, extensions: ExtensionMap? = nil, file: XCTestFileArgType = #file, line: UInt = #line, check: (MessageTestType) -> Bool) { do { - let decoded: MessageTestType = try MessageTestType(jsonString: json) + let decoded: MessageTestType = try MessageTestType(jsonString: json, extensions: extensions) XCTAssert(check(decoded), "Condition failed for \(decoded)", file: file, line: line) do { let encoded = try decoded.jsonString() do { - let redecoded = try MessageTestType(jsonString: encoded) + let redecoded = try MessageTestType(jsonString: encoded, extensions: extensions) XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded) from \(encoded)", file: file, line: line) XCTAssertEqual(decoded, redecoded, file: file, line: line) } catch { @@ -232,14 +232,14 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable do { let jsonData = json.data(using: String.Encoding.utf8)! - let decoded: MessageTestType = try MessageTestType(jsonUTF8Data: jsonData) + let decoded: MessageTestType = try MessageTestType(jsonUTF8Data: jsonData, extensions: extensions) 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) + let redecoded = try MessageTestType(jsonUTF8Data: encoded, extensions: extensions) XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded) from binary \(encodedString)", file: file, line: line) XCTAssertEqual(decoded, redecoded, file: file, line: line) } catch { @@ -307,12 +307,13 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable func assertJSONDecodeFails( _ json: String, + extensions: ExtensionMap? = nil, options: JSONDecodingOptions = JSONDecodingOptions(), file: XCTestFileArgType = #file, line: UInt = #line ) { do { - let _ = try MessageTestType(jsonString: json, options: options) + let _ = try MessageTestType(jsonString: json, extensions: extensions, options: options) XCTFail("Swift decode should have failed: \(json)", file: file, line: line) } catch { // Yay! It failed! @@ -320,7 +321,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable let jsonData = json.data(using: String.Encoding.utf8)! do { - let _ = try MessageTestType(jsonUTF8Data: jsonData, options: options) + let _ = try MessageTestType(jsonUTF8Data: jsonData, extensions: extensions, options: options) XCTFail("Swift decode should have failed for binary: \(json)", file: file, line: line) } catch { // Yay! It failed again! diff --git a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift new file mode 100644 index 000000000..fd0805cb8 --- /dev/null +++ b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift @@ -0,0 +1,45 @@ +// Tests/SwiftProtobufTests/Test_JSON_Extensions.swift - Exercise proto2 extensions +// +// Copyright (c) 2014 - 2016 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- +/// +/// Test support for Proto2 extensions in JSON +/// +// ----------------------------------------------------------------------------- + +import Foundation +import XCTest +import SwiftProtobuf + +class Test_JSON_Extensions: XCTestCase, PBTestHelpers { + typealias MessageTestType = ProtobufUnittest_TestAllExtensions + var extensions = SwiftProtobuf.SimpleExtensionMap() + + override func setUp() { + // Start with all the extensions from the unittest.proto file: + extensions = ProtobufUnittest_Unittest_Extensions + // Append another file's worth: + extensions.formUnion(ProtobufUnittest_UnittestCustomOptions_Extensions) + // Append an array of extensions + extensions.insert(contentsOf: + [ + Extensions_RepeatedExtensionGroup, + Extensions_ExtensionGroup + ] + ) + } + + func test_optionalInt32Extension() throws { + assertJSONEncode("{\"[protobuf_unittest.optional_int32_extension]\":17}", + extensions: extensions) { + (o: inout MessageTestType) in + o.ProtobufUnittest_optionalInt32Extension = 17 + } + } + +} From e3289523f5c1526394554c9f0b1cb3570f6c60ae Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Thu, 14 May 2020 17:00:08 -0700 Subject: [PATCH 02/17] JsonInput.FieldNameExtension is not failing in this branch --- Sources/Conformance/failure_list_swift.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/Conformance/failure_list_swift.txt b/Sources/Conformance/failure_list_swift.txt index 6726479ae..b97355a87 100644 --- a/Sources/Conformance/failure_list_swift.txt +++ b/Sources/Conformance/failure_list_swift.txt @@ -1,2 +1 @@ -# Tracking in https://github.com/apple/swift-protobuf/issues/993 -Recommended.Proto2.JsonInput.FieldNameExtension.Validator +# No failures From e56ffc39ffb37fd7daf35d875e510632fc9dcfd9 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Fri, 15 May 2020 17:44:58 -0700 Subject: [PATCH 03/17] Basic test for nested messages with extension fields --- Protos/unittest_swift_extension.proto | 1 + Reference/unittest_swift_extension.pb.swift | 21 +++++++++ Sources/SwiftProtobuf/JSONDecoder.swift | 47 +++++++++---------- .../SwiftProtobuf/JSONEncodingVisitor.swift | 20 ++++++-- .../SwiftProtobuf/Message+JSONAdditions.swift | 31 +----------- .../Message+JSONArrayAdditions.swift | 3 +- Tests/LinuxMain.swift | 7 +++ .../Test_JSON_Extensions.swift | 17 +++++++ .../unittest_swift_extension.pb.swift | 21 +++++++++ 9 files changed, 108 insertions(+), 60 deletions(-) diff --git a/Protos/unittest_swift_extension.proto b/Protos/unittest_swift_extension.proto index af6ff16bc..f20a937b4 100644 --- a/Protos/unittest_swift_extension.proto +++ b/Protos/unittest_swift_extension.proto @@ -53,6 +53,7 @@ message Msg2 { extend Msg1 { optional int32 a_b = 1; + optional Msg2 m2 = 2; } extend Msg2 { diff --git a/Reference/unittest_swift_extension.pb.swift b/Reference/unittest_swift_extension.pb.swift index 6e76a52a5..f26f5be47 100644 --- a/Reference/unittest_swift_extension.pb.swift +++ b/Reference/unittest_swift_extension.pb.swift @@ -236,6 +236,21 @@ extension ProtobufUnittest_Extend_Msg1 { mutating func clearProtobufUnittest_Extend_aB() { clearExtensionValue(ext: ProtobufUnittest_Extend_Extensions_a_b) } + + var ProtobufUnittest_Extend_m2: ProtobufUnittest_Extend_Msg2 { + get {return getExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2) ?? ProtobufUnittest_Extend_Msg2()} + set {setExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2, value: newValue)} + } + /// Returns true if extension `ProtobufUnittest_Extend_Extensions_m2` + /// has been explicitly set. + var hasProtobufUnittest_Extend_m2: Bool { + return hasExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2) + } + /// Clears the value of extension `ProtobufUnittest_Extend_Extensions_m2`. + /// Subsequent reads from it will return its default value. + mutating func clearProtobufUnittest_Extend_m2() { + clearExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2) + } } extension ProtobufUnittest_Extend_Msg2 { @@ -333,6 +348,7 @@ let ProtobufUnittest_Extend_UnittestSwiftExtension_Extensions: SwiftProtobuf.Sim ProtobufUnittest_Extend_Extensions_b, ProtobufUnittest_Extend_Extensions_C, ProtobufUnittest_Extend_Extensions_a_b, + ProtobufUnittest_Extend_Extensions_m2, ProtobufUnittest_Extend_Extensions_aB, ProtobufUnittest_Extend_Extensions_ext_a, ProtobufUnittest_Extend_Extensions_ext_b, @@ -359,6 +375,11 @@ let ProtobufUnittest_Extend_Extensions_a_b = SwiftProtobuf.MessageExtension, ProtobufUnittest_Extend_Msg1>( + _protobuf_fieldNumber: 2, + fieldName: "protobuf_unittest.extend.m2" +) + let ProtobufUnittest_Extend_Extensions_aB = SwiftProtobuf.MessageExtension, ProtobufUnittest_Extend_Msg2>( _protobuf_fieldNumber: 1, fieldName: "protobuf_unittest.extend.aB" diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index d8a9e3ace..b6755ff5c 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -27,13 +27,6 @@ internal struct JSONDecoder: Decoder { throw JSONDecodingError.conflictingOneOf } - internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions) { - self.options = options - self.scanner = JSONScanner(source: source, - messageDepthLimit: self.options.messageDepthLimit, - ignoreUnknownFields: self.options.ignoreUnknownFields) - } - internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions, messageType: Message.Type, extensions: ExtensionMap?) { self.options = options @@ -44,11 +37,12 @@ internal struct JSONDecoder: Decoder { self.extensions = extensions } - private init(decoder: JSONDecoder) { + private init(decoder: JSONDecoder, messageType: Message.Type) { // The scanner is copied over along with the options. - scanner = decoder.scanner - options = decoder.options - extensions = decoder.extensions + self.scanner = decoder.scanner + self.options = decoder.options + self.extensions = decoder.extensions + self.messageType = messageType } mutating func nextFieldNumber() throws -> Int? { @@ -545,7 +539,7 @@ internal struct JSONDecoder: Decoder { if value == nil { value = M() } - var subDecoder = JSONDecoder(decoder: self) + var subDecoder = JSONDecoder(decoder: self, messageType: M.self) try subDecoder.decodeFullObject(message: &value!) assert(scanner.recursionBudget == subDecoder.scanner.recursionBudget) scanner = subDecoder.scanner @@ -576,7 +570,7 @@ internal struct JSONDecoder: Decoder { } } else { var message = M() - var subDecoder = JSONDecoder(decoder: self) + var subDecoder = JSONDecoder(decoder: self, messageType: M.self) try subDecoder.decodeFullObject(message: &message) value.append(message) assert(scanner.recursionBudget == subDecoder.scanner.recursionBudget) @@ -713,19 +707,20 @@ internal struct JSONDecoder: Decoder { messageType: Message.Type, fieldNumber: Int ) throws { - if let ext = extensions?[messageType, fieldNumber] { - var fieldValue = values[fieldNumber] - if fieldValue != nil { - try fieldValue!.decodeExtensionField(decoder: &self) - } else { - fieldValue = try ext._protobuf_newField(decoder: &self) - } - if fieldValue != nil { - values[fieldNumber] = fieldValue - } else { - // This most likely indicates a bug in our extension support. - throw TextFormatDecodingError.internalExtensionError - } + guard let ext = extensions?[messageType, fieldNumber] else { + return + } + var fieldValue = values[fieldNumber] + if fieldValue != nil { + try fieldValue!.decodeExtensionField(decoder: &self) + } else { + fieldValue = try ext._protobuf_newField(decoder: &self) + } + if fieldValue != nil { + values[fieldNumber] = fieldValue + } else { + // This most likely indicates a bug in our extension support. + throw TextFormatDecodingError.internalExtensionError } } } diff --git a/Sources/SwiftProtobuf/JSONEncodingVisitor.swift b/Sources/SwiftProtobuf/JSONEncodingVisitor.swift index 61de3eeb8..6ceeea238 100644 --- a/Sources/SwiftProtobuf/JSONEncodingVisitor.swift +++ b/Sources/SwiftProtobuf/JSONEncodingVisitor.swift @@ -173,10 +173,17 @@ internal struct JSONEncodingVisitor: Visitor { encoder.append(text: json) } else if let newNameMap = (M.self as? _ProtoNameProviding.Type)?._protobuf_nameMap { encoder.startNestedObject() + // Preserve outer object's name and extension maps; restore them before returning let oldNameMap = self.nameMap + let oldExtensions = self.extensions + defer { + self.nameMap = oldNameMap + self.extensions = oldExtensions + } + // Install inner object's name and extension maps self.nameMap = newNameMap + self.extensions = (value as? ExtensibleMessage)?._protobuf_extensionFieldValues try value.traverse(visitor: &self) - self.nameMap = oldNameMap endObject() } else { throw JSONEncodingError.missingFieldNames @@ -300,17 +307,22 @@ internal struct JSONEncodingVisitor: Visitor { encoder.append(text: json) } } else if let newNameMap = (M.self as? _ProtoNameProviding.Type)?._protobuf_nameMap { - // Install inner object's name map + // Preserve name and extension maps for outer object let oldNameMap = self.nameMap + let oldExtensions = self.extensions + // Restore outer object's name and extension maps before returning + defer { + self.nameMap = oldNameMap + self.extensions = oldExtensions + } self.nameMap = newNameMap - // Restore outer object's name map before returning - defer { self.nameMap = oldNameMap } for v in value { if comma { encoder.comma() } comma = true encoder.startNestedObject() + self.extensions = (v as? ExtensibleMessage)?._protobuf_extensionFieldValues try v.traverse(visitor: &self) encoder.endObject() } diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift index 2d1ba02e3..b8e854e22 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions.swift @@ -70,14 +70,7 @@ extension Message { jsonString: String, options: JSONDecodingOptions = JSONDecodingOptions() ) throws { - if jsonString.isEmpty { - throw JSONDecodingError.truncated - } - if let data = jsonString.data(using: String.Encoding.utf8) { - try self.init(jsonUTF8Data: data, options: options) - } else { - throw JSONDecodingError.truncated - } + try self.init(jsonString: jsonString, extensions: nil, options: options) } /// Creates a new message by decoding the given string containing a @@ -114,27 +107,7 @@ extension Message { jsonUTF8Data: Data, options: JSONDecodingOptions = JSONDecodingOptions() ) throws { - self.init() - try jsonUTF8Data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in - // 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 - } - } else { - try decoder.decodeFullObject(message: &self) - } - if !decoder.scanner.complete { - throw JSONDecodingError.trailingGarbage - } - } + try self.init(jsonUTF8Data: jsonUTF8Data, extensions: nil, options: options) } /// Creates a new message by decoding the given `Data` containing a diff --git a/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift b/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift index ca21928da..a3272f3b1 100644 --- a/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift @@ -95,7 +95,8 @@ extension Message { var array = [Self]() if body.count > 0 { - var decoder = JSONDecoder(source: body, options: options) + var decoder = JSONDecoder(source: body, options: options, + messageType: Self.self, extensions: nil) try decoder.decodeRepeatedMessageField(value: &array) if !decoder.scanner.complete { throw JSONDecodingError.trailingGarbage diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 5b046be1e..dedef18cb 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -649,6 +649,12 @@ extension Test_JSON_Extensions { ] } +extension Test_JSON_RecursiveNested_Extensions { + static var allTests = [ + ("test_nestedMessage", test_nestedMessage) + ] +} + extension Test_JSON_Group { static var allTests = [ ("testOptionalGroup", testOptionalGroup), @@ -1193,6 +1199,7 @@ XCTMain( testCase(Test_JSON_Array.allTests), testCase(Test_JSON_Conformance.allTests), testCase(Test_JSON_Extensions.allTests), + testCase(Test_JSON_RecursiveNested_Extensions.allTests), testCase(Test_JSON_Group.allTests), testCase(Test_Map.allTests), testCase(Test_MapFields_Access_Proto2.allTests), diff --git a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift index fd0805cb8..a793f6571 100644 --- a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift +++ b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift @@ -41,5 +41,22 @@ class Test_JSON_Extensions: XCTestCase, PBTestHelpers { o.ProtobufUnittest_optionalInt32Extension = 17 } } +} +class Test_JSON_RecursiveNested_Extensions: XCTestCase, PBTestHelpers { + typealias MessageTestType = ProtobufUnittest_Extend_Msg1 + let extensions = ProtobufUnittest_Extend_UnittestSwiftExtension_Extensions + + func test_nestedMessage() throws { + assertJSONEncode("{\"[protobuf_unittest.extend.a_b]\":12}", + extensions: extensions) { + (o: inout MessageTestType) in + o.ProtobufUnittest_Extend_aB = 12 + } + + assertJSONDecodeSucceeds("{\"[protobuf_unittest.extend.m2]\":{\"[protobuf_unittest.extend.aB]\":23}}", extensions: extensions) { + $0.ProtobufUnittest_Extend_m2.ProtobufUnittest_Extend_aB == 23 + } + } + } diff --git a/Tests/SwiftProtobufTests/unittest_swift_extension.pb.swift b/Tests/SwiftProtobufTests/unittest_swift_extension.pb.swift index 6e76a52a5..f26f5be47 100644 --- a/Tests/SwiftProtobufTests/unittest_swift_extension.pb.swift +++ b/Tests/SwiftProtobufTests/unittest_swift_extension.pb.swift @@ -236,6 +236,21 @@ extension ProtobufUnittest_Extend_Msg1 { mutating func clearProtobufUnittest_Extend_aB() { clearExtensionValue(ext: ProtobufUnittest_Extend_Extensions_a_b) } + + var ProtobufUnittest_Extend_m2: ProtobufUnittest_Extend_Msg2 { + get {return getExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2) ?? ProtobufUnittest_Extend_Msg2()} + set {setExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2, value: newValue)} + } + /// Returns true if extension `ProtobufUnittest_Extend_Extensions_m2` + /// has been explicitly set. + var hasProtobufUnittest_Extend_m2: Bool { + return hasExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2) + } + /// Clears the value of extension `ProtobufUnittest_Extend_Extensions_m2`. + /// Subsequent reads from it will return its default value. + mutating func clearProtobufUnittest_Extend_m2() { + clearExtensionValue(ext: ProtobufUnittest_Extend_Extensions_m2) + } } extension ProtobufUnittest_Extend_Msg2 { @@ -333,6 +348,7 @@ let ProtobufUnittest_Extend_UnittestSwiftExtension_Extensions: SwiftProtobuf.Sim ProtobufUnittest_Extend_Extensions_b, ProtobufUnittest_Extend_Extensions_C, ProtobufUnittest_Extend_Extensions_a_b, + ProtobufUnittest_Extend_Extensions_m2, ProtobufUnittest_Extend_Extensions_aB, ProtobufUnittest_Extend_Extensions_ext_a, ProtobufUnittest_Extend_Extensions_ext_b, @@ -359,6 +375,11 @@ let ProtobufUnittest_Extend_Extensions_a_b = SwiftProtobuf.MessageExtension, ProtobufUnittest_Extend_Msg1>( + _protobuf_fieldNumber: 2, + fieldName: "protobuf_unittest.extend.m2" +) + let ProtobufUnittest_Extend_Extensions_aB = SwiftProtobuf.MessageExtension, ProtobufUnittest_Extend_Msg2>( _protobuf_fieldNumber: 1, fieldName: "protobuf_unittest.extend.aB" From cd033696977c1e5733ff85c1d75f376c5c97b6cf Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Tue, 26 May 2020 16:56:27 -0700 Subject: [PATCH 04/17] Just abort on invariant failures; don't try to recover --- Sources/SwiftProtobuf/JSONDecoder.swift | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index b6755ff5c..5758a771b 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -707,20 +707,15 @@ internal struct JSONDecoder: Decoder { messageType: Message.Type, fieldNumber: Int ) throws { - guard let ext = extensions?[messageType, fieldNumber] else { - return - } + // Force-unwrap: we can only get here if the extension exists. + let ext = extensions![messageType, fieldNumber]! + var fieldValue = values[fieldNumber] if fieldValue != nil { try fieldValue!.decodeExtensionField(decoder: &self) } else { fieldValue = try ext._protobuf_newField(decoder: &self) } - if fieldValue != nil { - values[fieldNumber] = fieldValue - } else { - // This most likely indicates a bug in our extension support. - throw TextFormatDecodingError.internalExtensionError - } + values[fieldNumber] = fieldValue! } } From 38274e875e27db04579a086f8695e8e689629e3e Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Tue, 2 Jun 2020 16:09:43 -0700 Subject: [PATCH 05/17] Any + JSON + extensions This extends the various Any methods to accept an extensions map. So you can now unpack an Any object holding JSON that contains extensions; you "just" have to provide the appropriate extension map. Note that implicit transcoding from JSON to binary (which occurs when you ask for `.serializedData()` from an Any that was decoded from JSON without explicitly unpacking the JSON contents will always throw if the stored JSON has extensions. I experimented with several alternatives, but this seemed the best. * The Any could remember the extension map that was in effect when it was decoded, then that would be available for later transcoding. This doesn't work for binary -> JSON transcoding (because there is currently no hook that would allow the Any to capture the extensions in effect during the outer binary decode). In addition, this would add another field to Any, increasing memory requirements for a combination of features (Any + JSON + extensions) that I expect will be very rarely used (if ever). * Transcoding JSON -> Binary could silently return empty data, but I don't like silently losing data. * Transcoding JSON -> Binary could ignore extensions, but again I wasn't silently losing data, even in this obscure case. --- Sources/SwiftProtobuf/AnyMessageStorage.swift | 27 ++++++++++--- Sources/SwiftProtobuf/JSONDecoder.swift | 6 +-- Sources/SwiftProtobuf/JSONEncoder.swift | 6 ++- .../SwiftProtobuf/JSONEncodingVisitor.swift | 29 +++++--------- .../SwiftProtobuf/Message+JSONAdditions.swift | 38 +++--------------- .../Message+JSONArrayAdditions.swift | 8 ++-- Tests/LinuxMain.swift | 3 +- Tests/SwiftProtobufTests/TestHelpers.swift | 39 +++++++++++++++---- Tests/SwiftProtobufTests/Test_Any.swift | 38 +++++++++++++++++- .../Test_JSON_Extensions.swift | 11 ++++++ 10 files changed, 128 insertions(+), 77 deletions(-) diff --git a/Sources/SwiftProtobuf/AnyMessageStorage.swift b/Sources/SwiftProtobuf/AnyMessageStorage.swift index c0541cb51..7e4e53ab9 100644 --- a/Sources/SwiftProtobuf/AnyMessageStorage.swift +++ b/Sources/SwiftProtobuf/AnyMessageStorage.swift @@ -25,8 +25,8 @@ fileprivate func serializeAnyJSON( typeURL: String, options: JSONEncodingOptions ) throws -> String { - var visitor = try JSONEncodingVisitor(message: message, options: options) - visitor.startObject() + var visitor = try JSONEncodingVisitor(type: type(of: message), options: options) + visitor.startObject(message: message) visitor.encodeField(name: "@type", stringValue: typeURL) if let m = message as? _CustomJSONCodable { let value = try m.encodedJSONString(options: options) @@ -58,11 +58,12 @@ fileprivate func asJSONObject(body: Data) -> Data { } fileprivate func unpack(contentJSON: Data, + extensions: ExtensionMap, options: JSONDecodingOptions, as messageType: Message.Type) throws -> Message { guard messageType is _CustomJSONCodable.Type else { let contentJSONAsObject = asJSONObject(body: contentJSON) - return try messageType.init(jsonUTF8Data: contentJSONAsObject, options: options) + return try messageType.init(jsonUTF8Data: contentJSONAsObject, extensions: extensions, options: options) } var value = String() @@ -85,7 +86,7 @@ fileprivate func unpack(contentJSON: Data, } } } - return try messageType.init(jsonString: value, options: options) + return try messageType.init(jsonString: value, extensions: extensions, options: options) } internal class AnyMessageStorage { @@ -109,6 +110,7 @@ internal class AnyMessageStorage { } do { let m = try unpack(contentJSON: contentJSON, + extensions: SimpleExtensionMap(), options: options, as: messageType) return try m.serializedData(partial: true) @@ -154,7 +156,7 @@ internal class AnyMessageStorage { return encodedType == M.protoMessageName } - // This is only ever called with the expactation that target will be fully + // This is only ever called with the expectation that target will be fully // replaced during the unpacking and never as a merge. func unpackTo( target: inout M, @@ -181,6 +183,7 @@ internal class AnyMessageStorage { case .contentJSON(let contentJSON, let options): target = try unpack(contentJSON: contentJSON, + extensions: extensions ?? SimpleExtensionMap(), options: options, as: M.self) as! M } @@ -202,7 +205,18 @@ internal class AnyMessageStorage { // never inserted. break - case .contentJSON: + case .contentJSON(let contentJSON, let options): + do { + guard let messageType = Google_Protobuf_Any.messageType(forTypeURL: _typeURL) else { + throw BinaryEncodingError.anyTranscodeFailure + } + _ = try unpack(contentJSON: contentJSON, + extensions: SimpleExtensionMap(), + options: options, + as: messageType) + } catch { + throw BinaryEncodingError.anyTranscodeFailure + } // contentJSON requires a good URL and our ability to look up // the message type to transcode. if Google_Protobuf_Any.messageType(forTypeURL: _typeURL) == nil { @@ -271,6 +285,7 @@ extension AnyMessageStorage { if let messageType = Google_Protobuf_Any.messageType(forTypeURL: _typeURL) { do { let m = try unpack(contentJSON: contentJSON, + extensions: SimpleExtensionMap(), options: options, as: messageType) emitVerboseTextForm(visitor: &visitor, message: m, typeURL: _typeURL) diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index 5758a771b..7480bce0c 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -17,7 +17,7 @@ import Foundation internal struct JSONDecoder: Decoder { internal var scanner: JSONScanner internal var options: JSONDecodingOptions - internal var extensions: ExtensionMap? + internal var extensions: ExtensionMap internal var messageType: Message.Type? private var fieldCount = 0 private var isMapKey = false @@ -28,7 +28,7 @@ internal struct JSONDecoder: Decoder { } internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions, - messageType: Message.Type, extensions: ExtensionMap?) { + messageType: Message.Type, extensions: ExtensionMap) { self.options = options self.scanner = JSONScanner(source: source, messageDepthLimit: self.options.messageDepthLimit, @@ -708,7 +708,7 @@ internal struct JSONDecoder: Decoder { fieldNumber: Int ) throws { // Force-unwrap: we can only get here if the extension exists. - let ext = extensions![messageType, fieldNumber]! + let ext = extensions[messageType, fieldNumber]! var fieldValue = values[fieldNumber] if fieldValue != nil { diff --git a/Sources/SwiftProtobuf/JSONEncoder.swift b/Sources/SwiftProtobuf/JSONEncoder.swift index 0a2079337..5ef793959 100644 --- a/Sources/SwiftProtobuf/JSONEncoder.swift +++ b/Sources/SwiftProtobuf/JSONEncoder.swift @@ -158,7 +158,8 @@ internal struct JSONEncoder { } /// Append an open curly brace `{` to the JSON. - internal mutating func startObject() { + /// Assumes this object is part of an array of objects. + internal mutating func startArrayObject() { if let s = separator { data.append(s) } @@ -166,7 +167,8 @@ internal struct JSONEncoder { separator = nil } - internal mutating func startNestedObject() { + /// Append an open curly brace `{` to the JSON. + internal mutating func startObject() { data.append(asciiOpenCurlyBracket) separator = nil } diff --git a/Sources/SwiftProtobuf/JSONEncodingVisitor.swift b/Sources/SwiftProtobuf/JSONEncodingVisitor.swift index 6ceeea238..955fd8bdd 100644 --- a/Sources/SwiftProtobuf/JSONEncodingVisitor.swift +++ b/Sources/SwiftProtobuf/JSONEncodingVisitor.swift @@ -43,17 +43,6 @@ internal struct JSONEncodingVisitor: Visitor { self.options = options } - /// Creates a new visitor that serializes the given message to JSON format. - init(message: Message, options: JSONEncodingOptions) throws { - if let nameProviding = message as? _ProtoNameProviding { - self.nameMap = type(of: nameProviding)._protobuf_nameMap - } else { - throw JSONEncodingError.missingFieldNames - } - self.extensions = (message as? ExtensibleMessage)?._protobuf_extensionFieldValues - self.options = options - } - mutating func startArray() { encoder.startArray() } @@ -62,10 +51,16 @@ internal struct JSONEncodingVisitor: Visitor { encoder.endArray() } - mutating func startObject() { + mutating func startObject(message: Message) { + self.extensions = (message as? ExtensibleMessage)?._protobuf_extensionFieldValues encoder.startObject() } + mutating func startArrayObject(message: Message) { + self.extensions = (message as? ExtensibleMessage)?._protobuf_extensionFieldValues + encoder.startArrayObject() + } + mutating func endObject() { encoder.endObject() } @@ -172,7 +167,6 @@ internal struct JSONEncodingVisitor: Visitor { let json = try m.encodedJSONString(options: options) encoder.append(text: json) } else if let newNameMap = (M.self as? _ProtoNameProviding.Type)?._protobuf_nameMap { - encoder.startNestedObject() // Preserve outer object's name and extension maps; restore them before returning let oldNameMap = self.nameMap let oldExtensions = self.extensions @@ -182,7 +176,7 @@ internal struct JSONEncodingVisitor: Visitor { } // Install inner object's name and extension maps self.nameMap = newNameMap - self.extensions = (value as? ExtensibleMessage)?._protobuf_extensionFieldValues + startObject(message: value) try value.traverse(visitor: &self) endObject() } else { @@ -317,12 +311,7 @@ internal struct JSONEncodingVisitor: Visitor { } self.nameMap = newNameMap for v in value { - if comma { - encoder.comma() - } - comma = true - encoder.startNestedObject() - self.extensions = (v as? ExtensibleMessage)?._protobuf_extensionFieldValues + startArrayObject(message: v) try v.traverse(visitor: &self) encoder.endObject() } diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift index b8e854e22..b4c07feeb 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions.swift @@ -53,8 +53,8 @@ extension Message { let data = string.data(using: String.Encoding.utf8)! // Cannot fail! return data } - var visitor = try JSONEncodingVisitor(message: self, options: options) - visitor.startObject() + var visitor = try JSONEncodingVisitor(type: Self.self, options: options) + visitor.startObject(message: self) try traverse(visitor: &visitor) visitor.endObject() return visitor.dataResult @@ -64,25 +64,12 @@ extension Message { /// serialized message in JSON format. /// /// - Parameter jsonString: The JSON-formatted string to decode. + /// - Parameter extensions: An ExtensionMap for looking up extensions by name /// - Parameter options: The JSONDecodingOptions to use. /// - Throws: `JSONDecodingError` if decoding fails. public init( jsonString: String, - options: JSONDecodingOptions = JSONDecodingOptions() - ) throws { - try self.init(jsonString: jsonString, extensions: nil, options: options) - } - - /// Creates a new message by decoding the given string containing a - /// serialized message in JSON format. - /// - /// - Parameter jsonString: The JSON-formatted string to decode. - /// - Parameter extensions: An ExtensionMap - /// - Parameter options: The JSONDecodingOptions to use. - /// - Throws: `JSONDecodingError` if decoding fails. - public init( - jsonString: String, - extensions: ExtensionMap?, + extensions: ExtensionMap = SimpleExtensionMap(), options: JSONDecodingOptions = JSONDecodingOptions() ) throws { if jsonString.isEmpty { @@ -95,21 +82,6 @@ extension Message { } } - /// Creates a new message by decoding the given `Data` containing a - /// serialized message in JSON format, interpreting the data as UTF-8 encoded - /// text. - /// - /// - Parameter jsonUTF8Data: The JSON-formatted data to decode, represented - /// as UTF-8 encoded text. - /// - Parameter options: The JSONDecodingOptions to use. - /// - Throws: `JSONDecodingError` if decoding fails. - public init( - jsonUTF8Data: Data, - options: JSONDecodingOptions = JSONDecodingOptions() - ) throws { - try self.init(jsonUTF8Data: jsonUTF8Data, extensions: nil, options: options) - } - /// Creates a new message by decoding the given `Data` containing a /// serialized message in JSON format, interpreting the data as UTF-8 encoded /// text. @@ -121,7 +93,7 @@ extension Message { /// - Throws: `JSONDecodingError` if decoding fails. public init( jsonUTF8Data: Data, - extensions: ExtensionMap?, + extensions: ExtensionMap = SimpleExtensionMap(), options: JSONDecodingOptions = JSONDecodingOptions() ) throws { self.init() diff --git a/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift b/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift index a3272f3b1..aa63c2a56 100644 --- a/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift @@ -51,7 +51,7 @@ extension Message { var visitor = try JSONEncodingVisitor(type: Self.self, options: options) visitor.startArray() for message in collection { - visitor.startObject() + visitor.startArrayObject(message: message) try message.traverse(visitor: &visitor) visitor.endObject() } @@ -67,13 +67,14 @@ extension Message { /// - Throws: `JSONDecodingError` if decoding fails. public static func array( fromJSONString jsonString: String, + extensions: ExtensionMap = SimpleExtensionMap(), options: JSONDecodingOptions = JSONDecodingOptions() ) throws -> [Self] { if jsonString.isEmpty { throw JSONDecodingError.truncated } if let data = jsonString.data(using: String.Encoding.utf8) { - return try array(fromJSONUTF8Data: data, options: options) + return try array(fromJSONUTF8Data: data, extensions: extensions, options: options) } else { throw JSONDecodingError.truncated } @@ -89,6 +90,7 @@ extension Message { /// - Throws: `JSONDecodingError` if decoding fails. public static func array( fromJSONUTF8Data jsonUTF8Data: Data, + extensions: ExtensionMap = SimpleExtensionMap(), options: JSONDecodingOptions = JSONDecodingOptions() ) throws -> [Self] { return try jsonUTF8Data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in @@ -96,7 +98,7 @@ extension Message { if body.count > 0 { var decoder = JSONDecoder(source: body, options: options, - messageType: Self.self, extensions: nil) + messageType: Self.self, extensions: extensions) try decoder.decodeRepeatedMessageField(value: &array) if !decoder.scanner.complete { throw JSONDecodingError.trailingGarbage diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index dedef18cb..48334dda2 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -266,8 +266,9 @@ extension Test_Any { ("test_Any_Value_string_transcode", test_Any_Value_string_transcode), ("test_Any_OddTypeURL_FromValue", test_Any_OddTypeURL_FromValue), ("test_Any_OddTypeURL_FromMessage", test_Any_OddTypeURL_FromMessage), + ("test_Any_JSON_Extensions", test_Any_JSON_Extensions), ("test_IsA", test_IsA), - ("test_Any_Registery", test_Any_Registery) + ("test_Any_Registry", test_Any_Registry) ] } diff --git a/Tests/SwiftProtobufTests/TestHelpers.swift b/Tests/SwiftProtobufTests/TestHelpers.swift index 50bc313a5..9513632b4 100644 --- a/Tests/SwiftProtobufTests/TestHelpers.swift +++ b/Tests/SwiftProtobufTests/TestHelpers.swift @@ -134,7 +134,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } - func assertJSONEncode(_ expected: String, extensions: ExtensionMap? = nil, file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout MessageTestType) -> Void) { + func assertJSONEncode(_ expected: String, extensions: ExtensionMap = SimpleExtensionMap(), file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout MessageTestType) -> Void) { let empty = MessageTestType() var configured = empty configure(&configured) @@ -173,7 +173,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable /// This uses the provided block to initialize the object, then: /// * Encodes the object and checks that the result is the expected result /// * Decodes it again and verifies that the round-trip gives an equal object - func assertTextFormatEncode(_ expected: String, extensions: SimpleExtensionMap? = nil, file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout MessageTestType) -> Void) { + func assertTextFormatEncode(_ expected: String, extensions: ExtensionMap? = nil, file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout MessageTestType) -> Void) { let empty = MessageTestType() var configured = empty configure(&configured) @@ -189,7 +189,13 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } } - func assertJSONArrayEncode(_ expected: String, file: XCTestFileArgType = #file, line: UInt = #line, configure: (inout [MessageTestType]) -> Void) { + func assertJSONArrayEncode( + _ expected: String, + extensions: ExtensionMap = SimpleExtensionMap(), + file: XCTestFileArgType = #file, + line: UInt = #line, + configure: (inout [MessageTestType]) -> Void + ) { let empty = [MessageTestType]() var configured = empty configure(&configured) @@ -198,7 +204,8 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable let encoded = try MessageTestType.jsonString(from: configured) XCTAssert(expected == encoded, "Did not encode correctly: got \(encoded)", file: file, line: line) do { - let decoded = try MessageTestType.array(fromJSONString: encoded) + let decoded = try MessageTestType.array(fromJSONString: encoded, + extensions: extensions) 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: \(encoded), but it threw \(error)", file: file, line: line) @@ -208,7 +215,13 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } } - func assertJSONDecodeSucceeds(_ json: String, extensions: ExtensionMap? = nil, file: XCTestFileArgType = #file, line: UInt = #line, check: (MessageTestType) -> Bool) { + func assertJSONDecodeSucceeds( + _ json: String, + extensions: ExtensionMap = SimpleExtensionMap(), + file: XCTestFileArgType = #file, + line: UInt = #line, + check: (MessageTestType) -> Bool + ) { do { let decoded: MessageTestType = try MessageTestType(jsonString: json, extensions: extensions) XCTAssert(check(decoded), "Condition failed for \(decoded)", file: file, line: line) @@ -282,7 +295,12 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } } - func assertJSONArrayDecodeSucceeds(_ json: String, file: XCTestFileArgType = #file, line: UInt = #line, check: ([MessageTestType]) -> Bool) { + func assertJSONArrayDecodeSucceeds( + _ json: String, + file: XCTestFileArgType = #file, + line: UInt = #line, + check: ([MessageTestType]) -> Bool + ) { do { let decoded: [MessageTestType] = try MessageTestType.array(fromJSONString: json) XCTAssert(check(decoded), "Condition failed for \(decoded)", file: file, line: line) @@ -307,7 +325,7 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable func assertJSONDecodeFails( _ json: String, - extensions: ExtensionMap? = nil, + extensions: ExtensionMap = SimpleExtensionMap(), options: JSONDecodingOptions = JSONDecodingOptions(), file: XCTestFileArgType = #file, line: UInt = #line @@ -337,7 +355,12 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable } } - func assertJSONArrayDecodeFails(_ json: String, file: XCTestFileArgType = #file, line: UInt = #line) { + func assertJSONArrayDecodeFails( + _ json: String, + extensions: ExtensionMap = SimpleExtensionMap(), + file: XCTestFileArgType = #file, + line: UInt = #line + ) { do { let _ = try MessageTestType.array(fromJSONString: json) XCTFail("Swift decode should have failed: \(json)", file: file, line: line) diff --git a/Tests/SwiftProtobufTests/Test_Any.swift b/Tests/SwiftProtobufTests/Test_Any.swift index 8435304a5..2b3828a16 100644 --- a/Tests/SwiftProtobufTests/Test_Any.swift +++ b/Tests/SwiftProtobufTests/Test_Any.swift @@ -645,6 +645,42 @@ class Test_Any: XCTestCase { XCTAssertEqual(newJSON, "{\"optionalAny\":{\"@type\":\"Odd\\nPrefix\\\"/google.protobuf.Value\",\"value\":\"abc\"}}") } + func test_Any_JSON_Extensions() throws { + var content = ProtobufUnittest_TestAllExtensions() + content.ProtobufUnittest_optionalInt32Extension = 17 + + var msg = ProtobufUnittest_TestAny() + msg.anyValue = try Google_Protobuf_Any(message: content) + + let json = try msg.jsonString() + XCTAssertEqual(json, "{\"anyValue\":{\"@type\":\"type.googleapis.com/protobuf_unittest.TestAllExtensions\",\"[protobuf_unittest.optional_int32_extension]\":17}}") + + // Decode the outer message without any extension knowledge + let decoded = try ProtobufUnittest_TestAny(jsonString: json) + // Decoding the inner content fails without extension info + XCTAssertThrowsError(try ProtobufUnittest_TestAllExtensions(unpackingAny: decoded.anyValue)) + // Succeeds if you do provide extension info + let decodedContent = try ProtobufUnittest_TestAllExtensions(unpackingAny: decoded.anyValue, + extensions: ProtobufUnittest_Unittest_Extensions) + XCTAssertEqual(content, decodedContent) + + // Transcoding should fail without extension info + XCTAssertThrowsError(try decoded.serializedData()) + + // Decode the outer message with extension information + let decodedWithExtensions = try ProtobufUnittest_TestAny(jsonString: json, + extensions: ProtobufUnittest_Unittest_Extensions) + // Still fails; the Any doesn't record extensions that were in effect when the outer Any was decoded + XCTAssertThrowsError(try ProtobufUnittest_TestAllExtensions(unpackingAny: decodedWithExtensions.anyValue)) + let decodedWithExtensionsContent = try ProtobufUnittest_TestAllExtensions(unpackingAny: decodedWithExtensions.anyValue, + extensions: ProtobufUnittest_Unittest_Extensions) + XCTAssertEqual(content, decodedWithExtensionsContent) + + XCTAssertTrue(Google_Protobuf_Any.register(messageType: ProtobufUnittest_TestAllExtensions.self)) + // Throws because the extensions can't be implicitly transcoded + XCTAssertThrowsError(try decodedWithExtensions.serializedData()) + } + func test_IsA() { var msg = Google_Protobuf_Any() @@ -666,7 +702,7 @@ class Test_Any: XCTestCase { XCTAssertFalse(msg.isA(Google_Protobuf_Empty.self)) } - func test_Any_Registery() { + func test_Any_Registry() { // Registering the same type multiple times is ok. XCTAssertTrue(Google_Protobuf_Any.register(messageType: ProtobufUnittestImport_ImportMessage.self)) XCTAssertTrue(Google_Protobuf_Any.register(messageType: ProtobufUnittestImport_ImportMessage.self)) diff --git a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift index a793f6571..2c6dbf8b5 100644 --- a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift +++ b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift @@ -40,6 +40,17 @@ class Test_JSON_Extensions: XCTestCase, PBTestHelpers { (o: inout MessageTestType) in o.ProtobufUnittest_optionalInt32Extension = 17 } + + assertJSONDecodeFails("{\"[protobuf_unittest.XXoptional_int32_extensionXX]\":17}", + extensions: extensions) + + assertJSONArrayEncode("[{\"[protobuf_unittest.optional_int32_extension]\":17}]", + extensions: extensions) { + (o: inout [MessageTestType]) in + var o1 = MessageTestType() + o1.ProtobufUnittest_optionalInt32Extension = 17 + o.append(o1) + } } } From 80634324ebac46e7e81e748fba5e333a92f7526a Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Tue, 2 Jun 2020 16:32:10 -0700 Subject: [PATCH 06/17] Test case for decoding JSON array of messages with extensions --- Tests/LinuxMain.swift | 3 ++- .../Test_JSON_Extensions.swift | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 48334dda2..b377e3a69 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -646,7 +646,8 @@ extension Test_JSON_Conformance { extension Test_JSON_Extensions { static var allTests = [ - ("test_optionalInt32Extension", test_optionalInt32Extension) + ("test_optionalInt32Extension", test_optionalInt32Extension), + ("test_ArrayWithExtensions", test_ArrayWithExtensions) ] } diff --git a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift index 2c6dbf8b5..f7224cf20 100644 --- a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift +++ b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift @@ -52,6 +52,28 @@ class Test_JSON_Extensions: XCTestCase, PBTestHelpers { o.append(o1) } } + + func test_ArrayWithExtensions() throws { + assertJSONArrayEncode( + "[" + + "{\"[protobuf_unittest.optional_int32_extension]\":17}," + + "{}," + + "{\"[protobuf_unittest.optional_double_extension]\":1.23}" + + "]", + extensions: extensions) + { + (o: inout [MessageTestType]) in + let o1 = MessageTestType.with { + $0.ProtobufUnittest_optionalInt32Extension = 17 + } + o.append(o1) + o.append(MessageTestType()) + let o3 = MessageTestType.with { + $0.ProtobufUnittest_optionalDoubleExtension = 1.23 + } + o.append(o3) + } + } } class Test_JSON_RecursiveNested_Extensions: XCTestCase, PBTestHelpers { From f73398d7810aa9fee6f4623482590df8ee49a188 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Tue, 2 Jun 2020 17:09:15 -0700 Subject: [PATCH 07/17] Test more extension + JSON scenarios --- Tests/LinuxMain.swift | 4 ++ .../Test_JSON_Extensions.swift | 47 ++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index b377e3a69..6504be05d 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -647,6 +647,10 @@ extension Test_JSON_Conformance { extension Test_JSON_Extensions { static var allTests = [ ("test_optionalInt32Extension", test_optionalInt32Extension), + ("test_optionalMessageExtension", test_optionalMessageExtension), + ("test_repeatedInt32Extension", test_repeatedInt32Extension), + ("test_repeatedMessageExtension", test_repeatedMessageExtension), + ("test_optionalStringExtensionWithDefault", test_optionalStringExtensionWithDefault), ("test_ArrayWithExtensions", test_ArrayWithExtensions) ] } diff --git a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift index f7224cf20..5e3fef317 100644 --- a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift +++ b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift @@ -53,7 +53,52 @@ class Test_JSON_Extensions: XCTestCase, PBTestHelpers { } } - func test_ArrayWithExtensions() throws { + func test_optionalMessageExtension() throws { + assertJSONEncode("{\"[protobuf_unittest.optional_nested_message_extension]\":{\"bb\":12}}", + extensions: extensions) + { + (o: inout MessageTestType) in + o.ProtobufUnittest_optionalNestedMessageExtension = + ProtobufUnittest_TestAllTypes.NestedMessage.with { + $0.bb = 12 + } + } + } + + func test_repeatedInt32Extension() throws { + assertJSONEncode("{\"[protobuf_unittest.repeated_int32_extension]\":[1,2,3,17]}", + extensions: extensions) { + (o: inout MessageTestType) in + o.ProtobufUnittest_repeatedInt32Extension = [1,2,3,17] + } + } + + func test_repeatedMessageExtension() throws { + assertJSONEncode("{\"[protobuf_unittest.repeated_nested_message_extension]\":[{\"bb\":12},{}]}", + extensions: extensions) + { + (o: inout MessageTestType) in + o.ProtobufUnittest_repeatedNestedMessageExtension = + [ + ProtobufUnittest_TestAllTypes.NestedMessage.with { $0.bb = 12 }, + ProtobufUnittest_TestAllTypes.NestedMessage() + ] + } + } + + func test_optionalStringExtensionWithDefault() throws { + assertJSONEncode("{\"[protobuf_unittest.default_string_extension]\":\"hi\"}", extensions: extensions) + { + (o: inout MessageTestType) in + o.ProtobufUnittest_defaultStringExtension = "hi" + } + + assertJSONDecodeSucceeds("{}", extensions: extensions) { + $0.ProtobufUnittest_defaultStringExtension == "hello" + } + } + + func test_ArrayWithExtensions() throws { assertJSONArrayEncode( "[" + "{\"[protobuf_unittest.optional_int32_extension]\":17}," From 483360586cade82ed121f10f2700e116258e01f5 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 13:48:07 -0700 Subject: [PATCH 08/17] Use the more general type here --- Sources/Conformance/main.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Conformance/main.swift b/Sources/Conformance/main.swift index 234001cbe..aabbc9ae7 100644 --- a/Sources/Conformance/main.swift +++ b/Sources/Conformance/main.swift @@ -85,7 +85,7 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse { } let msgType: SwiftProtobuf.Message.Type - let extensions: SwiftProtobuf.SimpleExtensionMap + let extensions: SwiftProtobuf.ExtensionMap switch request.messageType { case "": // Note: This case is here to cover using a old version of the conformance test From cdd13ff9274998df3477a222d6d9e7ef888ddfab Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 13:50:51 -0700 Subject: [PATCH 09/17] Corrected the comment describing unknown field handling --- Sources/SwiftProtobuf/JSONScanner.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftProtobuf/JSONScanner.swift b/Sources/SwiftProtobuf/JSONScanner.swift index cb154de01..ef1067ae9 100644 --- a/Sources/SwiftProtobuf/JSONScanner.swift +++ b/Sources/SwiftProtobuf/JSONScanner.swift @@ -1248,8 +1248,9 @@ internal struct JSONScanner { /// and return the corresponding field number. /// /// Throws if field name cannot be parsed. - /// If it encounters an unknown field name, it silently skips - /// the value and looks at the following field name. + /// If it encounters an unknown field name, it throws + /// unless `ignoreUnknownFields` is set, in which case + /// it silently skips it. internal mutating func nextFieldNumber( names: _NameMap, messageType: Message.Type?, From 51abfcd4b2ed90edde39f2808581757d2b41ede2 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:01:53 -0700 Subject: [PATCH 10/17] Remove duplicate check, simplify error handling, clarify comments --- Sources/SwiftProtobuf/AnyMessageStorage.swift | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftProtobuf/AnyMessageStorage.swift b/Sources/SwiftProtobuf/AnyMessageStorage.swift index 7e4e53ab9..e5d105603 100644 --- a/Sources/SwiftProtobuf/AnyMessageStorage.swift +++ b/Sources/SwiftProtobuf/AnyMessageStorage.swift @@ -206,10 +206,15 @@ internal class AnyMessageStorage { break case .contentJSON(let contentJSON, let options): + // contentJSON requires we have the type available for decoding + guard let messageType = Google_Protobuf_Any.messageType(forTypeURL: _typeURL) else { + throw BinaryEncodingError.anyTranscodeFailure + } do { - guard let messageType = Google_Protobuf_Any.messageType(forTypeURL: _typeURL) else { - throw BinaryEncodingError.anyTranscodeFailure - } + // Decodes the full JSON and then discard the result. + // The regular traversal will decode this again by querying the + // `value` field, but that has no way to fail. As a result, + // we need this to accurately handle decode errors. _ = try unpack(contentJSON: contentJSON, extensions: SimpleExtensionMap(), options: options, @@ -217,12 +222,6 @@ internal class AnyMessageStorage { } catch { throw BinaryEncodingError.anyTranscodeFailure } - // contentJSON requires a good URL and our ability to look up - // the message type to transcode. - if Google_Protobuf_Any.messageType(forTypeURL: _typeURL) == nil { - // Isn't registered, we can't transform it for binary. - throw BinaryEncodingError.anyTranscodeFailure - } } } } From 4feb5dc5eaba04a818e019d2c608ca9a5ecf0059 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:07:34 -0700 Subject: [PATCH 11/17] Make messageType non-optional since in practice it is always available --- Sources/SwiftProtobuf/JSONDecoder.swift | 2 +- Sources/SwiftProtobuf/JSONScanner.swift | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index 7480bce0c..787d0a183 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -18,7 +18,7 @@ internal struct JSONDecoder: Decoder { internal var scanner: JSONScanner internal var options: JSONDecodingOptions internal var extensions: ExtensionMap - internal var messageType: Message.Type? + internal var messageType: Message.Type private var fieldCount = 0 private var isMapKey = false private var fieldNameMap: _NameMap? diff --git a/Sources/SwiftProtobuf/JSONScanner.swift b/Sources/SwiftProtobuf/JSONScanner.swift index ef1067ae9..c7b3db788 100644 --- a/Sources/SwiftProtobuf/JSONScanner.swift +++ b/Sources/SwiftProtobuf/JSONScanner.swift @@ -1253,7 +1253,7 @@ internal struct JSONScanner { /// it silently skips it. internal mutating func nextFieldNumber( names: _NameMap, - messageType: Message.Type?, + messageType: Message.Type, extensionMap: ExtensionMap? ) throws -> Int? { while true { @@ -1274,7 +1274,6 @@ internal struct JSONScanner { } } if let extensions = extensionMap, - let messageType = messageType, let first = fieldName.first, first == "[", let last = fieldName.last, last == "]" { From c5e54c80cb1c180424a00888f7dab6aef04e1fab Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:08:26 -0700 Subject: [PATCH 12/17] Remove stray newline --- Sources/SwiftProtobuf/Message+JSONAdditions.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift index b4c07feeb..231ddc5c0 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions.swift @@ -1,4 +1,3 @@ - // Sources/SwiftProtobuf/Message+JSONAdditions.swift - JSON format primitive types // // Copyright (c) 2014 - 2016 Apple Inc. and the project authors From 671d7df1b96cf3765b6727adac276d8fe69138b4 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:09:51 -0700 Subject: [PATCH 13/17] Use the staticText form here for symmetry --- Sources/SwiftProtobuf/JSONEncoder.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/SwiftProtobuf/JSONEncoder.swift b/Sources/SwiftProtobuf/JSONEncoder.swift index 5ef793959..89f6e1037 100644 --- a/Sources/SwiftProtobuf/JSONEncoder.swift +++ b/Sources/SwiftProtobuf/JSONEncoder.swift @@ -133,8 +133,7 @@ internal struct JSONEncoder { if let s = separator { data.append(s) } - data.append(asciiDoubleQuote) - data.append(asciiOpenSquareBracket) + append(staticText: "\"[") data.append(contentsOf: name.utf8) append(staticText: "]\":") separator = asciiComma From b7d7c4c25d664865f0cb36f88327717aa45fabfe Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:31:36 -0700 Subject: [PATCH 14/17] Push the options and extensions down into the JSONScanner, since they are immutable for the duration of the decode and are more heavily used in the scanner --- Sources/SwiftProtobuf/AnyMessageStorage.swift | 4 +-- Sources/SwiftProtobuf/JSONDecoder.swift | 30 ++++++++----------- Sources/SwiftProtobuf/JSONScanner.swift | 24 +++++++-------- 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/Sources/SwiftProtobuf/AnyMessageStorage.swift b/Sources/SwiftProtobuf/AnyMessageStorage.swift index e5d105603..13bde5aec 100644 --- a/Sources/SwiftProtobuf/AnyMessageStorage.swift +++ b/Sources/SwiftProtobuf/AnyMessageStorage.swift @@ -69,9 +69,7 @@ fileprivate func unpack(contentJSON: Data, var value = String() try contentJSON.withUnsafeBytes { (body: UnsafeRawBufferPointer) in if body.count > 0 { - var scanner = JSONScanner(source: body, - messageDepthLimit: options.messageDepthLimit, - ignoreUnknownFields: options.ignoreUnknownFields) + var scanner = JSONScanner(source: body, options: options, extensions: extensions) let key = try scanner.nextQuotedString() if key != "value" { // The only thing within a WKT should be "value". diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index 787d0a183..c81f64056 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -16,32 +16,27 @@ import Foundation internal struct JSONDecoder: Decoder { internal var scanner: JSONScanner - internal var options: JSONDecodingOptions - internal var extensions: ExtensionMap internal var messageType: Message.Type private var fieldCount = 0 private var isMapKey = false private var fieldNameMap: _NameMap? + internal var options: JSONDecodingOptions { scanner.options } + mutating func handleConflictingOneOf() throws { throw JSONDecodingError.conflictingOneOf } internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions, messageType: Message.Type, extensions: ExtensionMap) { - self.options = options - self.scanner = JSONScanner(source: source, - messageDepthLimit: self.options.messageDepthLimit, - ignoreUnknownFields: self.options.ignoreUnknownFields) - self.messageType = messageType - self.extensions = extensions + let scanner = JSONScanner(source: source, + options: options, + extensions: extensions) + self.init(scanner: scanner, messageType: messageType) } - private init(decoder: JSONDecoder, messageType: Message.Type) { - // The scanner is copied over along with the options. - self.scanner = decoder.scanner - self.options = decoder.options - self.extensions = decoder.extensions + private init(scanner: JSONScanner, messageType: Message.Type) { + self.scanner = scanner self.messageType = messageType } @@ -53,8 +48,7 @@ internal struct JSONDecoder: Decoder { try scanner.skipRequiredComma() } let fieldNumber = try scanner.nextFieldNumber(names: fieldNameMap!, - messageType: messageType, - extensionMap: extensions) + messageType: messageType) if let fieldNumber = fieldNumber { fieldCount += 1 return fieldNumber @@ -539,7 +533,7 @@ internal struct JSONDecoder: Decoder { if value == nil { value = M() } - var subDecoder = JSONDecoder(decoder: self, messageType: M.self) + var subDecoder = JSONDecoder(scanner: scanner, messageType: M.self) try subDecoder.decodeFullObject(message: &value!) assert(scanner.recursionBudget == subDecoder.scanner.recursionBudget) scanner = subDecoder.scanner @@ -570,7 +564,7 @@ internal struct JSONDecoder: Decoder { } } else { var message = M() - var subDecoder = JSONDecoder(decoder: self, messageType: M.self) + var subDecoder = JSONDecoder(scanner: scanner, messageType: M.self) try subDecoder.decodeFullObject(message: &message) value.append(message) assert(scanner.recursionBudget == subDecoder.scanner.recursionBudget) @@ -708,7 +702,7 @@ internal struct JSONDecoder: Decoder { fieldNumber: Int ) throws { // Force-unwrap: we can only get here if the extension exists. - let ext = extensions[messageType, fieldNumber]! + let ext = scanner.extensions[messageType, fieldNumber]! var fieldValue = values[fieldNumber] if fieldValue != nil { diff --git a/Sources/SwiftProtobuf/JSONScanner.swift b/Sources/SwiftProtobuf/JSONScanner.swift index c7b3db788..5ded2ad0f 100644 --- a/Sources/SwiftProtobuf/JSONScanner.swift +++ b/Sources/SwiftProtobuf/JSONScanner.swift @@ -374,9 +374,10 @@ internal struct JSONScanner { private let source: UnsafeRawBufferPointer private var index: UnsafeRawBufferPointer.Index private var numberParser = DoubleParser() + internal var options: JSONDecodingOptions + internal var extensions: ExtensionMap internal var recursionLimit: Int internal var recursionBudget: Int - private var ignoreUnknownFields: Bool /// True if the scanner has read all of the data from the source, with the /// exception of any trailing whitespace (which is consumed by reading this @@ -400,14 +401,15 @@ internal struct JSONScanner { internal init( source: UnsafeRawBufferPointer, - messageDepthLimit: Int, - ignoreUnknownFields: Bool + options: JSONDecodingOptions, + extensions: ExtensionMap ) { self.source = source self.index = source.startIndex - self.recursionLimit = messageDepthLimit - self.recursionBudget = messageDepthLimit - self.ignoreUnknownFields = ignoreUnknownFields + self.recursionLimit = options.messageDepthLimit + self.recursionBudget = options.messageDepthLimit + self.options = options + self.extensions = extensions } private mutating func incrementRecursionDepth() throws { @@ -1249,12 +1251,11 @@ internal struct JSONScanner { /// /// Throws if field name cannot be parsed. /// If it encounters an unknown field name, it throws - /// unless `ignoreUnknownFields` is set, in which case + /// unless `options.ignoreUnknownFields` is set, in which case /// it silently skips it. internal mutating func nextFieldNumber( names: _NameMap, - messageType: Message.Type, - extensionMap: ExtensionMap? + messageType: Message.Type ) throws -> Int? { while true { var fieldName: String @@ -1273,8 +1274,7 @@ internal struct JSONScanner { return fieldNumber } } - if let extensions = extensionMap, - let first = fieldName.first, first == "[", + if let first = fieldName.first, first == "[", let last = fieldName.last, last == "]" { fieldName.removeFirst() @@ -1283,7 +1283,7 @@ internal struct JSONScanner { return fieldNumber } } - if !ignoreUnknownFields { + if !options.ignoreUnknownFields { throw JSONDecodingError.unknownField(fieldName) } // Unknown field, skip it and try to parse the next field name From 5ec35787cf9df86a732d57d0bd9b0ece00d24301 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:35:37 -0700 Subject: [PATCH 15/17] More checks that malformed JSON extensions are correctly rejected --- Tests/SwiftProtobufTests/Test_JSON_Extensions.swift | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift index 5e3fef317..b72672827 100644 --- a/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift +++ b/Tests/SwiftProtobufTests/Test_JSON_Extensions.swift @@ -41,7 +41,17 @@ class Test_JSON_Extensions: XCTestCase, PBTestHelpers { o.ProtobufUnittest_optionalInt32Extension = 17 } - assertJSONDecodeFails("{\"[protobuf_unittest.XXoptional_int32_extensionXX]\":17}", + assertJSONDecodeFails("{\"[protobuf_unittest.UNKNOWN_EXTENSION]\":17}", + extensions: extensions) + assertJSONDecodeFails("{\"[UNKNOWN_PACKAGE.optional_int32_extension]\":17}", + extensions: extensions) + assertJSONDecodeFails("{\"[protobuf_unittest.optional_int32_extension\":17}", + extensions: extensions) + assertJSONDecodeFails("{\"protobuf_unittest.optional_int32_extension]\":17}", + extensions: extensions) + assertJSONDecodeFails("{\"[optional_int32_extension\":17}", + extensions: extensions) + assertJSONDecodeFails("{\"protobuf_unittest.optional_int32_extension\":17}", extensions: extensions) assertJSONArrayEncode("[{\"[protobuf_unittest.optional_int32_extension]\":17}]", From 212b23a9578bfd889ec7c8c3a8d658c2d5941593 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 14:57:36 -0700 Subject: [PATCH 16/17] Make the JSON API consistent with binary and TextFormat by having the extensions argument be optional and default to nil --- Sources/SwiftProtobuf/JSONDecoder.swift | 2 +- Sources/SwiftProtobuf/JSONScanner.swift | 4 ++-- Sources/SwiftProtobuf/Message+JSONAdditions.swift | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index c81f64056..62a4d6807 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -28,7 +28,7 @@ internal struct JSONDecoder: Decoder { } internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions, - messageType: Message.Type, extensions: ExtensionMap) { + messageType: Message.Type, extensions: ExtensionMap?) { let scanner = JSONScanner(source: source, options: options, extensions: extensions) diff --git a/Sources/SwiftProtobuf/JSONScanner.swift b/Sources/SwiftProtobuf/JSONScanner.swift index 5ded2ad0f..ebd03849b 100644 --- a/Sources/SwiftProtobuf/JSONScanner.swift +++ b/Sources/SwiftProtobuf/JSONScanner.swift @@ -402,14 +402,14 @@ internal struct JSONScanner { internal init( source: UnsafeRawBufferPointer, options: JSONDecodingOptions, - extensions: ExtensionMap + extensions: ExtensionMap? ) { self.source = source self.index = source.startIndex self.recursionLimit = options.messageDepthLimit self.recursionBudget = options.messageDepthLimit self.options = options - self.extensions = extensions + self.extensions = extensions ?? SimpleExtensionMap() } private mutating func incrementRecursionDepth() throws { diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift index 231ddc5c0..c8d934315 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions.swift @@ -68,7 +68,7 @@ extension Message { /// - Throws: `JSONDecodingError` if decoding fails. public init( jsonString: String, - extensions: ExtensionMap = SimpleExtensionMap(), + extensions: ExtensionMap? = nil, options: JSONDecodingOptions = JSONDecodingOptions() ) throws { if jsonString.isEmpty { @@ -92,7 +92,7 @@ extension Message { /// - Throws: `JSONDecodingError` if decoding fails. public init( jsonUTF8Data: Data, - extensions: ExtensionMap = SimpleExtensionMap(), + extensions: ExtensionMap? = nil, options: JSONDecodingOptions = JSONDecodingOptions() ) throws { self.init() From c0f09b736fb02deef20f42efec9ec9e8c8264455 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 3 Jun 2020 15:20:39 -0700 Subject: [PATCH 17/17] Swift 4.2 does not support implicit returns --- Sources/SwiftProtobuf/JSONDecoder.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftProtobuf/JSONDecoder.swift b/Sources/SwiftProtobuf/JSONDecoder.swift index 62a4d6807..386d67238 100644 --- a/Sources/SwiftProtobuf/JSONDecoder.swift +++ b/Sources/SwiftProtobuf/JSONDecoder.swift @@ -21,7 +21,9 @@ internal struct JSONDecoder: Decoder { private var isMapKey = false private var fieldNameMap: _NameMap? - internal var options: JSONDecodingOptions { scanner.options } + internal var options: JSONDecodingOptions { + return scanner.options + } mutating func handleConflictingOneOf() throws { throw JSONDecodingError.conflictingOneOf