Build out the validation of google.protobuf.Any JSON support.

thomasvl · thomasvl · commit e295e4e64573 · 2025-01-08T15:47:58.000-05:00
The upstream major languages directly use their type registries to eagerly
validate `type_url`s. This means they will fail to decode or encode JSON for a
lot of cases.

The conformance tests only cover some of this so I hacked in some C++ unittests
to see what errors are thrown and then made the matching cases here and expanded
our error handling to duplicate the results.

The bulk of the changes are really just update comments to cover what errors
are now throw (not actual code changes).
diff --git 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
@@ -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<Bytes: SwiftProtobufContiguousBytes>(
         options: JSONEncodingOptions = JSONEncodingOptions()
     ) throws -> Bytes {
diff --git 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
@@ -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<C: Collection>(
         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<C: Collection, Bytes: SwiftProtobufContiguousBytes>(
         from collection: C,
         options: JSONEncodingOptions = JSONEncodingOptions()
diff --git 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<C: Collection>(
         from collection: C,
         options: JSONEncodingOptions = JSONEncodingOptions()
diff --git 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
