diff --git a/Sources/SwiftProtobuf/AnyMessageStorage.swift b/Sources/SwiftProtobuf/AnyMessageStorage.swift index e24880e5d..7c3a38991 100644 --- a/Sources/SwiftProtobuf/AnyMessageStorage.swift +++ b/Sources/SwiftProtobuf/AnyMessageStorage.swift @@ -411,6 +411,13 @@ extension AnyMessageStorage { // _CustomJSONCodable support for Google_Protobuf_Any extension AnyMessageStorage { + // Spec for Any says this should contain atleast one slash. Looking at upstream languages, most + // actually look up the value in their runtime registries, but since we do deferred parsing + // we can't assume the registry is complete, thus just do this minimal validation check. + fileprivate func isTypeURLValid() -> Bool { + _typeURL.contains("/") + } + // Override the traversal-based JSON encoding // This builds an Any JSON representation from one of: // * The message we were initialized with, @@ -423,11 +430,18 @@ extension AnyMessageStorage { case .binary(let valueData): // Follow the C++ protostream_objectsource.cc's // ProtoStreamObjectSource::RenderAny() special casing of an empty value. - guard !valueData.isEmpty else { + if valueData.isEmpty && _typeURL.isEmpty { + return "{}" + } + guard isTypeURLValid() else { if _typeURL.isEmpty { - return "{}" + throw SwiftProtobufError.JSONEncoding.emptyAnyTypeURL() } + throw SwiftProtobufError.JSONEncoding.invalidAnyTypeURL(type_url: _typeURL) + } + if valueData.isEmpty { var jsonEncoder = JSONEncoder() + jsonEncoder.startObject() jsonEncoder.startField(name: "@type") jsonEncoder.putStringValue(value: _typeURL) jsonEncoder.endObject() @@ -446,12 +460,21 @@ extension AnyMessageStorage { return try serializeAnyJSON(for: m, typeURL: _typeURL, options: options) case .message(let msg): - // We should have been initialized with a typeURL, but - // ensure it wasn't cleared. + // We should have been initialized with a typeURL, make sure it is valid. + if !_typeURL.isEmpty && !isTypeURLValid() { + throw SwiftProtobufError.JSONEncoding.invalidAnyTypeURL(type_url: _typeURL) + } + // If it was cleared, default it. let url = !_typeURL.isEmpty ? _typeURL : buildTypeURL(forMessage: msg, typePrefix: defaultAnyTypeURLPrefix) return try serializeAnyJSON(for: msg, typeURL: url, options: options) case .contentJSON(let contentJSON, _): + guard isTypeURLValid() else { + if _typeURL.isEmpty { + throw SwiftProtobufError.JSONEncoding.emptyAnyTypeURL() + } + throw SwiftProtobufError.JSONEncoding.invalidAnyTypeURL(type_url: _typeURL) + } var jsonEncoder = JSONEncoder() jsonEncoder.startObject() jsonEncoder.startField(name: "@type") @@ -488,11 +511,7 @@ extension AnyMessageStorage { try decoder.scanner.skipRequiredColon() if key == "@type" { _typeURL = try decoder.scanner.nextQuotedString() - // Spec for Any says this should contain atleast one slash. Looking at - // upstream languages, most actually look up the value in their runtime - // registries, but since we do deferred parsing, just do this minimal - // validation check. - guard _typeURL.contains("/") else { + guard isTypeURLValid() else { throw SwiftProtobufError.JSONDecoding.invalidAnyTypeURL(type_url: _typeURL) } } else { @@ -501,6 +520,9 @@ extension AnyMessageStorage { jsonEncoder.append(text: keyValueJSON) } if decoder.scanner.skipOptionalObjectEnd() { + if _typeURL.isEmpty { + throw SwiftProtobufError.JSONDecoding.emptyAnyTypeURL() + } // Capture the options, but set the messageDepthLimit to be what // was left right now, as that is the limit when the JSON is finally // parsed. diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift index 359f14766..a8c14cd37 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions.swift @@ -24,7 +24,7 @@ extension Message { /// - Returns: A string containing the JSON serialization of the message. /// - Parameters: /// - options: The JSONEncodingOptions to use. - /// - Throws: ``JSONEncodingError`` if encoding fails. + /// - Throws: ``SwiftProtobufError`` or ``JSONEncodingError`` if encoding fails. public func jsonString( options: JSONEncodingOptions = JSONEncodingOptions() ) throws -> String { @@ -43,7 +43,7 @@ extension Message { /// - Returns: A `SwiftProtobufContiguousBytes` containing the JSON serialization of the message. /// - Parameters: /// - options: The JSONEncodingOptions to use. - /// - Throws: ``JSONEncodingError`` if encoding fails. + /// - Throws: ``SwiftProtobufError`` or ``JSONEncodingError`` if encoding fails. public func jsonUTF8Bytes( options: JSONEncodingOptions = JSONEncodingOptions() ) throws -> Bytes { diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift b/Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift index c31077f6b..3c1c01a3a 100644 --- a/Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift +++ b/Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift @@ -54,7 +54,7 @@ extension Message { /// - Returns: A Data containing the JSON serialization of the message. /// - Parameters: /// - options: The JSONEncodingOptions to use. - /// - Throws: ``JSONEncodingError`` if encoding fails. + /// - Throws: ``SwiftProtobufError`` or ``JSONEncodingError`` if encoding fails. public func jsonUTF8Data( options: JSONEncodingOptions = JSONEncodingOptions() ) throws -> Data { diff --git a/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift b/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift index ad4e85b56..05a52437c 100644 --- a/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift +++ b/Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift @@ -25,7 +25,7 @@ extension Message { /// - Parameters: /// - collection: The list of messages to encode. /// - options: The JSONEncodingOptions to use. - /// - Throws: ``JSONEncodingError`` if encoding fails. + /// - Throws: ``SwiftProtobufError`` or ``JSONEncodingError`` if encoding fails. public static func jsonString( from collection: C, options: JSONEncodingOptions = JSONEncodingOptions() @@ -43,7 +43,7 @@ extension Message { /// - Parameters: /// - collection: The list of messages to encode. /// - options: The JSONEncodingOptions to use. - /// - Throws: ``JSONEncodingError`` if encoding fails. + /// - Throws: ``SwiftProtobufError`` or ``JSONEncodingError`` if encoding fails. public static func jsonUTF8Bytes( from collection: C, options: JSONEncodingOptions = JSONEncodingOptions() diff --git a/Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift b/Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift index 7a0116541..4003f60df 100644 --- a/Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift +++ b/Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift @@ -65,7 +65,7 @@ extension Message { /// - Parameters: /// - collection: The list of messages to encode. /// - options: The JSONEncodingOptions to use. - /// - Throws: ``JSONEncodingError`` if encoding fails. + /// - Throws: ``SwiftProtobufError`` or ``JSONEncodingError`` if encoding fails. public static func jsonUTF8Data( from collection: C, options: JSONEncodingOptions = JSONEncodingOptions() diff --git a/Sources/SwiftProtobuf/SwiftProtobufError.swift b/Sources/SwiftProtobuf/SwiftProtobufError.swift index c6849412b..361e20968 100644 --- a/Sources/SwiftProtobuf/SwiftProtobufError.swift +++ b/Sources/SwiftProtobuf/SwiftProtobufError.swift @@ -92,6 +92,7 @@ extension SwiftProtobufError { case binaryDecodingError case binaryStreamDecodingError case jsonDecodingError + case jsonEncodingError var description: String { switch self { @@ -101,6 +102,8 @@ extension SwiftProtobufError { return "Stream decoding error" case .jsonDecodingError: return "JSON decoding error" + case .jsonEncodingError: + return "JSON encoding error" } } } @@ -131,6 +134,10 @@ extension SwiftProtobufError { Self(.jsonDecodingError) } + /// Errors arising from JSON encoding of messages. + public static var jsonEncodingError: Self { + Self(.jsonEncodingError) + } } /// A location within source code. @@ -264,6 +271,48 @@ extension SwiftProtobufError { location: SourceLocation(function: function, file: file, line: line) ) } + + /// While decoding a `google.protobuf.Any` no `@type` field but the message had other fields. + public static func emptyAnyTypeURL( + function: String = #function, + file: String = #fileID, + line: Int = #line + ) -> SwiftProtobufError { + SwiftProtobufError( + code: .jsonDecodingError, + message: "google.protobuf.Any '@type' was must be present if if the object is not empty.", + location: SourceLocation(function: function, file: file, line: line) + ) + } } + /// Errors arising from JSON encoding of messages. + public enum JSONEncoding { + /// While encoding a `google.protobuf.Any` encountered a malformed `type_url` field. + public static func invalidAnyTypeURL( + type_url: String, + function: String = #function, + file: String = #fileID, + line: Int = #line + ) -> SwiftProtobufError { + SwiftProtobufError( + code: .jsonEncodingError, + message: "google.protobuf.Any 'type_url' was invalid: \(type_url).", + location: SourceLocation(function: function, file: file, line: line) + ) + } + + /// While encoding a `google.protobuf.Any` encountered an empty `type_url` field. + public static func emptyAnyTypeURL( + function: String = #function, + file: String = #fileID, + line: Int = #line + ) -> SwiftProtobufError { + SwiftProtobufError( + code: .jsonEncodingError, + message: "google.protobuf.Any 'type_url' was empty, only allowed for empty objects.", + location: SourceLocation(function: function, file: file, line: line) + ) + } + } } diff --git a/Tests/SwiftProtobufTests/Test_Any.swift b/Tests/SwiftProtobufTests/Test_Any.swift index 0561e2044..145a92337 100644 --- a/Tests/SwiftProtobufTests/Test_Any.swift +++ b/Tests/SwiftProtobufTests/Test_Any.swift @@ -873,38 +873,287 @@ final class Test_Any: XCTestCase { XCTAssertEqual(rejson, start) } - func test_Any_invalid() throws { - // These come from the upstream conformace tests. + func test_Any_typeURLValidations() throws { + // Upstream most langauges end up validating the type_url during encoding as well + // as during decoding. Basically to do things, they end up looking up the type + // in their global registries to use reflection to do things. SwiftProtobuf doesn't + // have a registry we can reply on being complete. So instead during encoding we + // do the most basic of checks. These tests help ensure those checks are working as + // expected. + // + // The inspiration for the tests and the errors comes from editing upstream's + // json_test.cc and observing a few things: + // + // TEST_P(JsonTest, HackingTypeURLs) { + // google::protobuf::Any any; + // any.set_type_url("not_valid"); + // // INVALID_ARGUMENT: @type must contain at least one / and a nonempty host; got: not_valid + // EXPECT_THAT(ToJson(any), StatusIs(absl::StatusCode::kInvalidArgument)); + // + // // This works because the message counts as empty + // any.set_type_url("type.googleapis.com/proto3.TestMessage"); + // EXPECT_THAT(ToJson(any), + // IsOkAndHolds(R"({"@type":"type.googleapis.com/proto3.TestMessage"})")); + // INVALID_ARGUMENT: @type must contain at least one / and a nonempty host; got: + // + // EXPECT_THAT(ToProto(R"json( + // { + // "@type": "" + // } + // )json"), + // StatusIs(absl::StatusCode::kInvalidArgument)); + // } + // + // TEST_P(JsonTest, HackingValues) { + // google::protobuf::Any any; + // any.set_value("abc"); + // std::string blob = any.SerializeAsString(); + // EXPECT_FALSE(blob.empty()); // It didn't fail to serialize + // // INVALID_ARGUMENT: broken Any: missing type URL + // EXPECT_THAT(ToJson(any), StatusIs(absl::StatusCode::kInvalidArgument)); + // + // // INVALID_ARGUMENT: invalid JSON in google.protobuf.Any, near 2:5 (offset 5): in legacy mode, missing @type in Any is only allowed for an empty object + // EXPECT_THAT(ToProto(R"json( + // { + // "value": "abc" + // } + // )json"), + // StatusIs(absl::StatusCode::kInvalidArgument)); + // } + + // ---- With a binary protobuf data in the backing store: - // AnyWktRepresentationWithEmptyTypeAndValue - let emptyType = "{\"optional_any\":{\"@type\":\"\",\"value\":\"\"}}" - XCTAssertThrowsError( - try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: emptyType) - ) { error in - XCTAssertTrue( - self.isSwiftProtobufErrorEqual( - error as! SwiftProtobufError, - .JSONDecoding.invalidAnyTypeURL(type_url: "") + do { + // No payload, no type_url + var any = Google_Protobuf_Any() + any.value = Data() + XCTAssertEqual(try any.jsonString(), "{}") + // No payload, valid type_url + any.typeURL = "type.googleapis.com/SomeMessage" + XCTAssertEqual( + try any.jsonString(), + "{\"@type\":\"type.googleapis.com/SomeMessage\"}" + ) + // No payload, invalid type url. + any.typeURL = "not_valid" + XCTAssertThrowsError( + try any.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } + + // Has payload, no type_url + any = Google_Protobuf_Any() + any.value = Data([8, 1]) // SwiftProtoTesting_TestAllTypes.optionalInt32 = 1 + XCTAssertThrowsError( + try any.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.emptyAnyTypeURL() + ) ) + } + // Has payload, valid type_url (gets lazy decoded) + Google_Protobuf_Any.register(messageType: SwiftProtoTesting_TestAllTypes.self) + any.typeURL = "type.googleapis.com/swift_proto_testing.TestAllTypes" + XCTAssertEqual( + try any.jsonString(), + "{\"@type\":\"type.googleapis.com/swift_proto_testing.TestAllTypes\",\"optionalInt32\":1}" ) + // Has payload, invalid type url. + any.typeURL = "not_valid" + XCTAssertThrowsError( + try any.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } } - // AnyWktRepresentationWithBadType - let notAType = "{\"optional_any\":{\"@type\":\"not_a_url\",\"value\":\"\"}}" - XCTAssertThrowsError( - try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: notAType) - ) { error in - XCTAssertTrue( - self.isSwiftProtobufErrorEqual( - error as! SwiftProtobufError, - .JSONDecoding.invalidAnyTypeURL(type_url: "not_a_url") - ) + // ---- With a message in the backing store: + + do { + var content = SwiftProtoTesting_TestAllTypes() + var anyEmpty = try Google_Protobuf_Any(message: content) + content.optionalInt32 = 17 + var anyNonEmpty = try Google_Protobuf_Any(message: content) + + // Valid + XCTAssertEqual( + try anyEmpty.jsonString(), + "{\"@type\":\"type.googleapis.com/swift_proto_testing.TestAllTypes\"}" + ) + XCTAssertEqual( + try anyNonEmpty.jsonString(), + "{\"@type\":\"type.googleapis.com/swift_proto_testing.TestAllTypes\",\"optionalInt32\":17}" ) + // Blank type url, will get defaulted again. + anyEmpty.typeURL = "" + anyNonEmpty.typeURL = "" + XCTAssertEqual( + try anyEmpty.jsonString(), + "{\"@type\":\"type.googleapis.com/swift_proto_testing.TestAllTypes\"}" + ) + XCTAssertEqual( + try anyNonEmpty.jsonString(), + "{\"@type\":\"type.googleapis.com/swift_proto_testing.TestAllTypes\",\"optionalInt32\":17}" + ) + // Invalid type url, will error + anyEmpty.typeURL = "not_valid" + anyNonEmpty.typeURL = "not_valid" + XCTAssertThrowsError( + try anyEmpty.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } + XCTAssertThrowsError( + try anyNonEmpty.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } + } + + // ---- With json data in the backing store: + + do { + // Empty, round trips. + var json = "{}" + var any = try Google_Protobuf_Any(jsonString: json) + XCTAssertEqual(json, try any.jsonString()) + // Empty with valid type_url, round trips + json = "{\"@type\":\"type.googleapis.com/SomeMessage\"}" + any = try Google_Protobuf_Any(jsonString: json) + XCTAssertEqual(json, try any.jsonString()) + // Empty with invalid type_url + XCTAssertThrowsError( + try Google_Protobuf_Any(jsonString: "{\"@type\":\"not_valid\"}") + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONDecoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } + // Empty, override with valid type_url, round trips + json = "{\"@type\":\"type.googleapis.com/SomeMessage\"}" + any = try Google_Protobuf_Any(jsonString: json) + any.typeURL = "type.googleapis.com/AnotherMessage" + XCTAssertEqual("{\"@type\":\"type.googleapis.com/AnotherMessage\"}", try any.jsonString()) + // Empty, override with invalid type_url + any = try Google_Protobuf_Any(jsonString: json) + any.typeURL = "not_valid" + XCTAssertThrowsError( + try any.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } + // Field but no type_url won't even decode. + XCTAssertThrowsError( + try Google_Protobuf_Any(jsonString: "{\"value\":1}") + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONDecoding.emptyAnyTypeURL() + ) + ) + } + // Non empty and a type_url round trips. + json = "{\"@type\":\"type.googleapis.com/SomeMessage\",\"value\":1}" + any = try Google_Protobuf_Any(jsonString: json) + XCTAssertEqual(json, try any.jsonString()) + // Non empty and override of the type_url still works + any = try Google_Protobuf_Any(jsonString: json) + any.typeURL = "type.googleapis.com/AnotherMessage" + XCTAssertEqual("{\"@type\":\"type.googleapis.com/AnotherMessage\",\"value\":1}", try any.jsonString()) + // Non empty, override with invalid type_url + any = try Google_Protobuf_Any(jsonString: json) + any.typeURL = "not_valid" + XCTAssertThrowsError( + try any.jsonString() + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONEncoding.invalidAnyTypeURL(type_url: "not_valid") + ) + ) + } + } + + // ---- These come from the upstream conformace tests: + + do { + // AnyWktRepresentationWithEmptyTypeAndValue + let emptyTypeAndValue = "{\"optional_any\":{\"@type\":\"\",\"value\":\"\"}}" + XCTAssertThrowsError( + try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: emptyTypeAndValue) + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONDecoding.invalidAnyTypeURL(type_url: "") + ) + ) + } + + // AnyWktRepresentationWithBadType + let notAType = "{\"optional_any\":{\"@type\":\"not_a_url\",\"value\":\"\"}}" + XCTAssertThrowsError( + try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: notAType) + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONDecoding.invalidAnyTypeURL(type_url: "not_a_url") + ) + ) + } + + // ---- Variant of AnyWktRepresentationWithEmptyTypeAndValue above with no value. + let emptyType = "{\"optional_any\":{\"@type\":\"\"}}" + XCTAssertThrowsError( + try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: emptyType) + ) { error in + XCTAssertTrue( + self.isSwiftProtobufErrorEqual( + error as! SwiftProtobufError, + .JSONDecoding.invalidAnyTypeURL(type_url: "") + ) + ) + } } } func test_Any_nestedList() throws { - var start = "{\"optionalAny\":{\"x\":" + var start = "{\"optionalAny\":{\"@type\":\"type.googleapis.com/Someting\",\"x\":" for _ in 0...10000 { start.append("[") }