Skip to content

Commit e876103

Browse files
thomasvlthomasvl
authored
During JSON parsing, validate @type to be minimally valid. (#1742)
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.
1 parent 8208b8d commit e876103

8 files changed

+79
-17
lines changed

‎Sources/Conformance/failure_list_swift.txt

+1-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1 @@
1-
Required.Editions_Proto3.JsonInput.AnyWktRepresentationWithBadType # Should have failed to parse, but didn't.
2-
Required.Editions_Proto3.JsonInput.AnyWktRepresentationWithEmptyTypeAndValue # Should have failed to parse, but didn't.
3-
Required.Proto3.JsonInput.AnyWktRepresentationWithBadType # Should have failed to parse, but didn't.
4-
Required.Proto3.JsonInput.AnyWktRepresentationWithEmptyTypeAndValue # Should have failed to parse, but didn't.
1+
# No known failures.

‎Sources/SwiftProtobuf/AnyMessageStorage.swift

+7
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,13 @@ extension AnyMessageStorage {
488488
try decoder.scanner.skipRequiredColon()
489489
if key == "@type" {
490490
_typeURL = try decoder.scanner.nextQuotedString()
491+
// Spec for Any says this should contain atleast one slash. Looking at
492+
// upstream languages, most actually look up the value in their runtime
493+
// registries, but since we do deferred parsing, just do this minimal
494+
// validation check.
495+
guard _typeURL.contains("/") else {
496+
throw SwiftProtobufError.JSONDecoding.invalidAnyTypeURL(type_url: _typeURL)
497+
}
491498
} else {
492499
jsonEncoder.startField(name: key)
493500
let keyValueJSON = try decoder.scanner.skip()

‎Sources/SwiftProtobuf/Message+JSONAdditions.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ extension Message {
6363
///
6464
/// - Parameter jsonString: The JSON-formatted string to decode.
6565
/// - Parameter options: The JSONDecodingOptions to use.
66-
/// - Throws: ``JSONDecodingError`` if decoding fails.
66+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
6767
public init(
6868
jsonString: String,
6969
options: JSONDecodingOptions = JSONDecodingOptions()
@@ -77,7 +77,7 @@ extension Message {
7777
/// - Parameter jsonString: The JSON-formatted string to decode.
7878
/// - Parameter extensions: An ExtensionMap for looking up extensions by name
7979
/// - Parameter options: The JSONDecodingOptions to use.
80-
/// - Throws: ``JSONDecodingError`` if decoding fails.
80+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
8181
public init(
8282
jsonString: String,
8383
extensions: (any ExtensionMap)? = nil,
@@ -100,7 +100,7 @@ extension Message {
100100
/// - Parameter jsonUTF8Bytes: The JSON-formatted data to decode, represented
101101
/// as UTF-8 encoded text.
102102
/// - Parameter options: The JSONDecodingOptions to use.
103-
/// - Throws: ``JSONDecodingError`` if decoding fails.
103+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
104104
public init<Bytes: SwiftProtobufContiguousBytes>(
105105
jsonUTF8Bytes: Bytes,
106106
options: JSONDecodingOptions = JSONDecodingOptions()
@@ -116,7 +116,7 @@ extension Message {
116116
/// as UTF-8 encoded text.
117117
/// - Parameter extensions: The extension map to use with this decode
118118
/// - Parameter options: The JSONDecodingOptions to use.
119-
/// - Throws: ``JSONDecodingError`` if decoding fails.
119+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
120120
public init<Bytes: SwiftProtobufContiguousBytes>(
121121
jsonUTF8Bytes: Bytes,
122122
extensions: (any ExtensionMap)? = nil,

‎Sources/SwiftProtobuf/Message+JSONAdditions_Data.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ extension Message {
2222
/// - Parameter jsonUTF8Data: The JSON-formatted data to decode, represented
2323
/// as UTF-8 encoded text.
2424
/// - Parameter options: The JSONDecodingOptions to use.
25-
/// - Throws: ``JSONDecodingError`` if decoding fails.
25+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
2626
public init(
2727
jsonUTF8Data: Data,
2828
options: JSONDecodingOptions = JSONDecodingOptions()
@@ -37,7 +37,7 @@ extension Message {
3737
/// as UTF-8 encoded text.
3838
/// - Parameter extensions: The extension map to use with this decode
3939
/// - Parameter options: The JSONDecodingOptions to use.
40-
/// - Throws: ``JSONDecodingError`` if decoding fails.
40+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
4141
public init(
4242
jsonUTF8Data: Data,
4343
extensions: (any ExtensionMap)? = nil,
@@ -54,7 +54,7 @@ extension Message {
5454
/// - Returns: A Data containing the JSON serialization of the message.
5555
/// - Parameters:
5656
/// - options: The JSONEncodingOptions to use.
57-
/// - Throws: ``JSONDecodingError`` if encoding fails.
57+
/// - Throws: ``JSONEncodingError`` if encoding fails.
5858
public func jsonUTF8Data(
5959
options: JSONEncodingOptions = JSONEncodingOptions()
6060
) throws -> Data {

‎Sources/SwiftProtobuf/Message+JSONArrayAdditions.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ extension Message {
6464
///
6565
/// - Parameter jsonString: The JSON-formatted string to decode.
6666
/// - Parameter options: The JSONDecodingOptions to use.
67-
/// - Throws: ``JSONDecodingError`` if decoding fails.
67+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
6868
public static func array(
6969
fromJSONString jsonString: String,
7070
options: JSONDecodingOptions = JSONDecodingOptions()
@@ -82,7 +82,7 @@ extension Message {
8282
/// - Parameter jsonString: The JSON-formatted string to decode.
8383
/// - Parameter extensions: The extension map to use with this decode
8484
/// - Parameter options: The JSONDecodingOptions to use.
85-
/// - Throws: ``JSONDecodingError`` if decoding fails.
85+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
8686
public static func array(
8787
fromJSONString jsonString: String,
8888
extensions: any ExtensionMap = SimpleExtensionMap(),
@@ -105,7 +105,7 @@ extension Message {
105105
/// - Parameter jsonUTF8Bytes: The JSON-formatted data to decode, represented
106106
/// as UTF-8 encoded text.
107107
/// - Parameter options: The JSONDecodingOptions to use.
108-
/// - Throws: ``JSONDecodingError`` if decoding fails.
108+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
109109
public static func array<Bytes: SwiftProtobufContiguousBytes>(
110110
fromJSONUTF8Bytes jsonUTF8Bytes: Bytes,
111111
options: JSONDecodingOptions = JSONDecodingOptions()
@@ -125,7 +125,7 @@ extension Message {
125125
/// as UTF-8 encoded text.
126126
/// - Parameter extensions: The extension map to use with this decode
127127
/// - Parameter options: The JSONDecodingOptions to use.
128-
/// - Throws: ``JSONDecodingError`` if decoding fails.
128+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
129129
public static func array<Bytes: SwiftProtobufContiguousBytes>(
130130
fromJSONUTF8Bytes jsonUTF8Bytes: Bytes,
131131
extensions: any ExtensionMap = SimpleExtensionMap(),

‎Sources/SwiftProtobuf/Message+JSONArrayAdditions_Data.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension Message {
2323
/// - Parameter jsonUTF8Data: The JSON-formatted data to decode, represented
2424
/// as UTF-8 encoded text.
2525
/// - Parameter options: The JSONDecodingOptions to use.
26-
/// - Throws: ``JSONDecodingError`` if decoding fails.
26+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
2727
public static func array(
2828
fromJSONUTF8Data jsonUTF8Data: Data,
2929
options: JSONDecodingOptions = JSONDecodingOptions()
@@ -43,7 +43,7 @@ extension Message {
4343
/// as UTF-8 encoded text.
4444
/// - Parameter extensions: The extension map to use with this decode
4545
/// - Parameter options: The JSONDecodingOptions to use.
46-
/// - Throws: ``JSONDecodingError`` if decoding fails.
46+
/// - Throws: ``SwiftProtobufError`` or ``JSONDecodingError`` if decoding fails.
4747
public static func array(
4848
fromJSONUTF8Data jsonUTF8Data: Data,
4949
extensions: any ExtensionMap = SimpleExtensionMap(),

‎Sources/SwiftProtobuf/SwiftProtobufError.swift

+28
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,16 @@ extension SwiftProtobufError {
9191
private enum Wrapped: Hashable, Sendable, CustomStringConvertible {
9292
case binaryDecodingError
9393
case binaryStreamDecodingError
94+
case jsonDecodingError
9495

9596
var description: String {
9697
switch self {
9798
case .binaryDecodingError:
9899
return "Binary decoding error"
99100
case .binaryStreamDecodingError:
100101
return "Stream decoding error"
102+
case .jsonDecodingError:
103+
return "JSON decoding error"
101104
}
102105
}
103106
}
@@ -122,6 +125,12 @@ extension SwiftProtobufError {
122125
public static var binaryStreamDecodingError: Self {
123126
Self(.binaryStreamDecodingError)
124127
}
128+
129+
/// Errors arising from JSON decoding of data into protobufs.
130+
public static var jsonDecodingError: Self {
131+
Self(.jsonDecodingError)
132+
}
133+
125134
}
126135

127136
/// A location within source code.
@@ -238,4 +247,23 @@ extension SwiftProtobufError {
238247
)
239248
}
240249
}
250+
251+
/// Errors arising from JSON decoding of data into protobufs.
252+
public enum JSONDecoding {
253+
/// While decoding a `google.protobuf.Any` encountered a malformed `@type` key for
254+
/// the `type_url` field.
255+
public static func invalidAnyTypeURL(
256+
type_url: String,
257+
function: String = #function,
258+
file: String = #fileID,
259+
line: Int = #line
260+
) -> SwiftProtobufError {
261+
SwiftProtobufError(
262+
code: .jsonDecodingError,
263+
message: "google.protobuf.Any '@type' was invalid: \(type_url).",
264+
location: SourceLocation(function: function, file: file, line: line)
265+
)
266+
}
267+
}
268+
241269
}

‎Tests/SwiftProtobufTests/Test_Any.swift

+30
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,36 @@ final class Test_Any: XCTestCase {
873873
XCTAssertEqual(rejson, start)
874874
}
875875

876+
func test_Any_invalid() throws {
877+
// These come from the upstream conformace tests.
878+
879+
// AnyWktRepresentationWithEmptyTypeAndValue
880+
let emptyType = "{\"optional_any\":{\"@type\":\"\",\"value\":\"\"}}"
881+
XCTAssertThrowsError(
882+
try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: emptyType)
883+
) { error in
884+
XCTAssertTrue(
885+
self.isSwiftProtobufErrorEqual(
886+
error as! SwiftProtobufError,
887+
.JSONDecoding.invalidAnyTypeURL(type_url: "")
888+
)
889+
)
890+
}
891+
892+
// AnyWktRepresentationWithBadType
893+
let notAType = "{\"optional_any\":{\"@type\":\"not_a_url\",\"value\":\"\"}}"
894+
XCTAssertThrowsError(
895+
try SwiftProtoTesting_Test3_TestAllTypesProto3(jsonString: notAType)
896+
) { error in
897+
XCTAssertTrue(
898+
self.isSwiftProtobufErrorEqual(
899+
error as! SwiftProtobufError,
900+
.JSONDecoding.invalidAnyTypeURL(type_url: "not_a_url")
901+
)
902+
)
903+
}
904+
}
905+
876906
func test_Any_nestedList() throws {
877907
var start = "{\"optionalAny\":{\"x\":"
878908
for _ in 0...10000 {

0 commit comments

Comments
 (0)