Skip to content

Commit

Permalink
Build out the validation of google.protobuf.Any JSON support. (#1744)
Browse files Browse the repository at this point in the history 
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).

---------

Co-authored-by: Gus Cairo <[email protected]>
  • Loading branch information
@thomasvl @gjcairo
thomasvl and gjcairo authored Jan 13, 2025
1 parent 881a549 commit e4c3a0c
Show file tree
Hide file tree
Showing 7 changed files with 357 additions and 37 deletions.
40 changes: 31 additions & 9 deletions Sources/SwiftProtobuf/AnyMessageStorage.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftProtobuf/Message+JSONAdditions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
49 changes: 49 additions & 0 deletions Sources/SwiftProtobuf/SwiftProtobufError.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ extension SwiftProtobufError {
case binaryDecodingError
case binaryStreamDecodingError
case jsonDecodingError
case jsonEncodingError

var description: String {
switch self {
Expand All @@ -101,6 +102,8 @@ extension SwiftProtobufError {
return "Stream decoding error"
case .jsonDecodingError:
return "JSON decoding error"
case .jsonEncodingError:
return "JSON encoding error"
}
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
)
}
}
}
Loading

0 comments on commit e4c3a0c

Please sign in to comment.