During JSON parsing, validate @type to be minimally valid.

Upstream added a recent conformance test to ensure things fail for an empty `@type` or one that doesn't have atleast a single slash. This updates the library to do that validations during parsing from JSON *only*. The change for this enforcement is pretty small (just the code in Sources/SwiftProtobuf/AnyMessageStorage.swift). The rest is to add a new error in the new SwiftProtobufError approach and then to update all of the apis to document that there are an additional type of error that could be throw.
apple · Jan 7, 2025 · e7f2717 · e7f2717
1 parent 8208b8d
commit e7f2717
Show file tree

Hide file tree

Showing 8 changed files with 75 additions and 17 deletions.
diff --git a/Sources/Conformance/failure_list_swift.txt b/Sources/Conformance/failure_list_swift.txt
@@ -1,4 +1 @@
-Required.Editions_Proto3.JsonInput.AnyWktRepresentationWithBadType # Should have failed to parse, but didn't.
-Required.Editions_Proto3.JsonInput.AnyWktRepresentationWithEmptyTypeAndValue # Should have failed to parse, but didn't.
-Required.Proto3.JsonInput.AnyWktRepresentationWithBadType # Should have failed to parse, but didn't.
-Required.Proto3.JsonInput.AnyWktRepresentationWithEmptyTypeAndValue # Should have failed to parse, but didn't.
+# No known failures.
diff --git a/Sources/SwiftProtobuf/AnyMessageStorage.swift b/Sources/SwiftProtobuf/AnyMessageStorage.swift
@@ -488,6 +488,13 @@ 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 {
+                    throw SwiftProtobufError.JSONDecoding.invalidAnyTypeURL(type_url: _typeURL)
+                }
             } else {
                 jsonEncoder.startField(name: key)
                 let keyValueJSON = try decoder.scanner.skip()

diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions.swift b/Sources/SwiftProtobuf/Message+JSONAdditions.swift
@@ -63,7 +63,7 @@ extension Message {
     ///
     /// - Parameter jsonString: The JSON-formatted string to decode.
     /// - Parameter options: The JSONDecodingOptions to use.
-    /// - Throws: ``JSONDecodingError`` if decoding fails.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public init(
         jsonString: String,
         options: JSONDecodingOptions = JSONDecodingOptions()
@@ -77,7 +77,7 @@ extension Message {
     /// - 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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public init(
         jsonString: String,
         extensions: (any ExtensionMap)? = nil,
@@ -100,7 +100,7 @@ extension Message {
     /// - Parameter jsonUTF8Bytes: The JSON-formatted data to decode, represented
     ///   as UTF-8 encoded text.
     /// - Parameter options: The JSONDecodingOptions to use.
-    /// - Throws: ``JSONDecodingError`` if decoding fails.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public init<Bytes: SwiftProtobufContiguousBytes>(
         jsonUTF8Bytes: Bytes,
         options: JSONDecodingOptions = JSONDecodingOptions()
@@ -116,7 +116,7 @@ extension Message {
     ///   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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public init<Bytes: SwiftProtobufContiguousBytes>(
         jsonUTF8Bytes: Bytes,
         extensions: (any ExtensionMap)? = nil,

diff --git a/Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift b/Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift
@@ -22,7 +22,7 @@ extension Message {
     /// - 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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public init(
         jsonUTF8Data: Data,
         options: JSONDecodingOptions = JSONDecodingOptions()
@@ -37,7 +37,7 @@ extension Message {
     ///   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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public init(
         jsonUTF8Data: Data,
         extensions: (any ExtensionMap)? = nil,
@@ -54,7 +54,7 @@ extension Message {
     /// - Returns: A Data containing the JSON serialization of the message.
     /// - Parameters:
     ///   - options: The JSONEncodingOptions to use.
-    /// - Throws: ``JSONDecodingError`` if encoding fails.
+    /// - Throws: ``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
@@ -64,7 +64,7 @@ extension Message {
     ///
     /// - Parameter jsonString: The JSON-formatted string to decode.
     /// - Parameter options: The JSONDecodingOptions to use.
-    /// - Throws: ``JSONDecodingError`` if decoding fails.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public static func array(
         fromJSONString jsonString: String,
         options: JSONDecodingOptions = JSONDecodingOptions()
@@ -82,7 +82,7 @@ extension Message {
     /// - Parameter jsonString: The JSON-formatted string to decode.
     /// - Parameter extensions: The extension map to use with this decode
     /// - Parameter options: The JSONDecodingOptions to use.
-    /// - Throws: ``JSONDecodingError`` if decoding fails.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public static func array(
         fromJSONString jsonString: String,
         extensions: any ExtensionMap = SimpleExtensionMap(),
@@ -105,7 +105,7 @@ extension Message {
     /// - Parameter jsonUTF8Bytes: The JSON-formatted data to decode, represented
     ///   as UTF-8 encoded text.
     /// - Parameter options: The JSONDecodingOptions to use.
-    /// - Throws: ``JSONDecodingError`` if decoding fails.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public static func array<Bytes: SwiftProtobufContiguousBytes>(
         fromJSONUTF8Bytes jsonUTF8Bytes: Bytes,
         options: JSONDecodingOptions = JSONDecodingOptions()
@@ -125,7 +125,7 @@ extension Message {
     ///   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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public static func array<Bytes: SwiftProtobufContiguousBytes>(
         fromJSONUTF8Bytes jsonUTF8Bytes: Bytes,
         extensions: any ExtensionMap = SimpleExtensionMap(),

diff --git a/Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift b/Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift
@@ -23,7 +23,7 @@ extension Message {
     /// - 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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public static func array(
         fromJSONUTF8Data jsonUTF8Data: Data,
         options: JSONDecodingOptions = JSONDecodingOptions()
@@ -43,7 +43,7 @@ extension Message {
     ///   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.
+    /// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
     public static func array(
         fromJSONUTF8Data jsonUTF8Data: Data,
         extensions: any ExtensionMap = SimpleExtensionMap(),

diff --git a/Sources/SwiftProtobuf/SwiftProtobufError.swift b/Sources/SwiftProtobuf/SwiftProtobufError.swift
@@ -91,13 +91,16 @@ extension SwiftProtobufError {
         private enum Wrapped: Hashable, Sendable, CustomStringConvertible {
             case binaryDecodingError
             case binaryStreamDecodingError
+            case jsonDecodingError
 
             var description: String {
                 switch self {
                 case .binaryDecodingError:
                     return "Binary decoding error"
                 case .binaryStreamDecodingError:
                     return "Stream decoding error"
+                case .jsonDecodingError:
+                    return "JSON decoding error"
                 }
             }
         }
@@ -122,6 +125,12 @@ extension SwiftProtobufError {
         public static var binaryStreamDecodingError: Self {
             Self(.binaryStreamDecodingError)
         }
+
+        /// Errors arising from JSON decoding of data into protobufs.
+        public static var jsonDecodingError: Self {
+            Self(.jsonDecodingError)
+        }
+
     }
 
     /// A location within source code.
@@ -238,4 +247,23 @@ extension SwiftProtobufError {
             )
         }
     }
+
+    /// Errors arising from JSON decoding of data into protobufs.
+    public enum JSONDecoding {
+        /// While decoding a `google.protobuf.Any` encountered a malformed `@type` key for
+        /// the `type_url` field.
+        public static func invalidAnyTypeURL(
+            type_url: String,
+            function: String = #function,
+            file: String = #fileID,
+            line: Int = #line
+        ) -> SwiftProtobufError {
+            SwiftProtobufError(
+                code: .jsonDecodingError,
+                message: "google.protobuf.Any '@type' was invalid: \(type_url).",
+                location: SourceLocation(function: function, file: file, line: line)
+            )
+        }
+    }
+
 }
diff --git a/Tests/SwiftProtobufTests/Test_Any.swift b/Tests/SwiftProtobufTests/Test_Any.swift
@@ -873,6 +873,32 @@ final class Test_Any: XCTestCase {
         XCTAssertEqual(rejson, start)
     }
 
+    func test_Any_invalid() throws {
+        // These come from the upstream conformace tests.
+
+        // 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: ""))
+            )
+        }
+
+        // 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"))
+            )
+        }
+    }
+
     func test_Any_nestedList() throws {
         var start = "{\"optionalAny\":{\"x\":"
         for _ in 0...10000 {