fix!: Throw error on invalid encoded data in response

Package.swift

@@ -30,6 +30,7 @@ let package = Package(
     ],
     products: [
         .library(name: "Smithy", targets: ["Smithy"]),
+        .library(name: "SmithySerialization", targets: ["SmithySerialization"]),
         .library(name: "ClientRuntime", targets: ["ClientRuntime"]),
         .library(name: "SmithyRetriesAPI", targets: ["SmithyRetriesAPI"]),
         .library(name: "SmithyRetries", targets: ["SmithyRetries"]),
@@ -78,11 +79,13 @@ let package = Package(
                 .product(name: "Logging", package: "swift-log"),
             ]
         ),
+        .target(
+            name: "SmithySerialization",
+            dependencies: ["Smithy"]
+        ),
         .target(
             name: "SmithyTelemetryAPI",
-            dependencies: [
-                "Smithy",
-            ]
+            dependencies: ["Smithy"]
         ),
         .target(
             name: "SmithyHTTPClientAPI",
@@ -168,6 +171,7 @@ let package = Package(
         .target(
             name: "SmithyXML",
             dependencies: [
+                "SmithySerialization",
                 "SmithyReadWrite",
                 "SmithyTimestamps",
                 libXML2DependencyOrNil
@@ -176,6 +180,7 @@ let package = Package(
         .target(
             name: "SmithyJSON",
             dependencies: [
+                "SmithySerialization",
                 "SmithyReadWrite",
                 "SmithyTimestamps"
             ]
@@ -319,7 +324,7 @@ let package = Package(
         ),
         .testTarget(
             name: "SmithyXMLTests",
-            dependencies: ["SmithyXML", "ClientRuntime"]
+            dependencies: ["SmithySerialization", "SmithyXML", "ClientRuntime"]
         ),
         .testTarget(
             name: "SmithyHTTPAuthTests",
@@ -331,7 +336,7 @@ let package = Package(
         ),
         .testTarget(
             name: "SmithyJSONTests",
-            dependencies: ["SmithyJSON", "ClientRuntime", "SmithyTestUtil"]
+            dependencies: ["SmithySerialization", "SmithyJSON", "ClientRuntime", "SmithyTestUtil"]
         ),
         .testTarget(
             name: "SmithyFormURLTests",

Sources/SmithyJSON/Reader/Reader+JSONDeserialization.swift

@@ -9,16 +9,22 @@ import struct Foundation.Data
 import class Foundation.JSONSerialization
 import class Foundation.NSError
 import class Foundation.NSNull
+import struct SmithySerialization.ResponseDecodingError
 
 extension Reader {
 
     public static func from(data: Data) throws -> Reader {
+        // Empty bodies are allowed.  When the body is empty,
+        // return a reader with no JSON content.
         guard !data.isEmpty else { return Reader(nodeInfo: "", parent: nil) }
+
+        // Attempt to parse JSON from the non-empty body.
+        // Throw an error if JSON is invalid.
         do {
-            let jsonObject = try JSONSerialization.jsonObject(with: data, options: [.fragmentsAllowed])
+            let jsonObject = try JSONSerialization.jsonObject(with: data)
             return try Reader(nodeInfo: "", jsonObject: jsonObject)
-        } catch let error as NSError where error.domain == "NSCocoaErrorDomain" && error.code == 3840 {
-            return try Reader(nodeInfo: "", jsonObject: [:])
+        } catch {
+            throw ResponseDecodingError(wrapped: error)
         }
     }
 }

Sources/SmithySerialization/ResponseDecodingError.swift

@@ -0,0 +1,28 @@
+//
+// Copyright Amazon.com Inc. or its affiliates.
+// All Rights Reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+/// The data being deserialized was invalid, and a response could not be parsed.
+///
+/// If an error of this type is received, it indicates a bug either on the server or client.  Please file a [bug ticket](https://github.com/smithy-lang/smithy-swift/issues).
+public struct ResponseDecodingError {
+
+    /// The error thrown by the deserializing implementation.
+    ///
+    /// The exact error returned may vary depending on the encoding in use.
+    public let wrapped: any Swift.Error // this will be the underlying error thrown by the parser implementation
+
+    public init(wrapped: any Error) {
+        self.wrapped = wrapped
+    }
+}
+
+extension ResponseDecodingError: Error {
+
+    public var localizedDescription: String {
+        "The data in the response could not be parsed.  More info: \(wrapped)"
+    }
+}

Sources/SmithyXML/Reader/Reader+libxml2.swift

@@ -7,6 +7,7 @@
 
 import struct Foundation.Data
 @preconcurrency import libxml2
+import struct SmithySerialization.ResponseDecodingError
 
 extension Reader {
 
@@ -21,7 +22,9 @@ extension Reader {
         guard let buffer else { return Reader() }
 
         // Read the buffer into a XML document tree
-        guard let doc = xmlReadMemory(buffer.pointee.content, Int32(count), "", "UTF-8", 0) else { return Reader() }
+        guard let doc = xmlReadMemory(buffer.pointee.content, Int32(count), "", "UTF-8", 0) else {
+            throw ResponseDecodingError(wrapped: XMLError.parsingError)
+        }
 
         // Get rootNode ptr. Just a ptr to inside the doc struct, so no memory allocated
         guard let rootNode = xmlDocGetRootElement(doc) else { return Reader() }
@@ -154,4 +157,5 @@ private struct XMLError: Error {
     static let memoryError = XMLError("XML buffer could not be allocated")
     static let invalidNode = XMLError("XML node was invalid")
     static let invalidNodeName = XMLError("XML node name was invalid")
+    static let parsingError = XMLError("The XML could not be parsed")
 }

Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift

@@ -224,19 +224,20 @@ class OrchestratorTests: XCTestCase {
             .attributes(attributes)
             .serialize({ input, builder, _ in
                 trace.append("serialize")
+                let data = try JSONEncoder().encode(["foo": input.foo])
                 builder.withMethod(.get)
                     .withPath("/")
                     .withHost("localhost")
-                    .withBody(.data(try! JSONEncoder().encode(input.foo)))
+                    .withBody(.data(data))
             })
             .deserialize({ response, _ in
                 trace.append("deserialize")
                 if (200..<300).contains(response.statusCode.rawValue) {
                     guard case let .data(data) = response.body else {
                         return TestOutput(bar: "")
                     }
-                    let bar = try! JSONDecoder().decode(String.self, from: data!)
-                    return TestOutput(bar: bar)
+                    let object = try! JSONDecoder().decode([String: String].self, from: data!)
+                    return TestOutput(bar: object["foo"]!)
                 } else {
                     let responseReader = try SmithyJSON.Reader.from(data: try await response.data())
                     let baseError = try TestBaseError(httpResponse: response, responseReader: responseReader, noErrorWrapping: true)
@@ -1374,7 +1375,8 @@ class OrchestratorTests: XCTestCase {
         let orchestrator = traceOrchestrator(trace: trace)
             .retryStrategy(DefaultRetryStrategy(options: RetryStrategyOptions(backoffStrategy: ImmediateBackoffStrategy())))
             .serialize({ (input: TestInput, builder: HTTPRequestBuilder, context) in
-                builder.withBody(.data(Data("\"\(input.foo)\"".utf8)))
+                let data = try JSONEncoder().encode(["foo": input.foo])
+                builder.withBody(.data(data))
             })
             .executeRequest(executeRequest)
         let result = await asyncResult {

Tests/SmithyJSONTests/ReaderTests.swift

@@ -6,14 +6,15 @@
 //
 
 import XCTest
+import struct SmithySerialization.ResponseDecodingError
 @testable @_spi(SmithyReadWrite) import SmithyJSON
 
-class ReaderTests: XCTestCase {
+final class ReaderTests: XCTestCase {
 
-    func test_readsNil() async throws {
+    func test_readsEmptyDataAsNil() async throws {
         let jsonData = Data()
         let reader = try SmithyJSON.Reader.from(data: jsonData)
-        XCTAssertEqual(reader.jsonNode, nil)
+        XCTAssertNil(reader.jsonNode)
     }
 
     func test_readsAJSONObject() async throws {
@@ -24,4 +25,13 @@ class ReaderTests: XCTestCase {
         XCTAssertEqual(reader.children.count, 1)
         XCTAssertEqual(try reader["property"].readIfPresent(), "potato")
     }
+
+    func test_throwsOnInvalidJSON() async throws {
+        let jsonData = Data("""
+        { "json": "incomplet    
+        """.utf8)
+        XCTAssertThrowsError(try SmithyJSON.Reader.from(data: jsonData)) { error in
+            XCTAssert(error is ResponseDecodingError)
+        }
+    }
 }

Tests/SmithyXMLTests/ReaderTests.swift

@@ -7,6 +7,7 @@
 
 import XCTest
 @testable @_spi(SmithyReadWrite) import SmithyXML
+import struct SmithySerialization.ResponseDecodingError
 
 class ReaderTests: XCTestCase {
     let xmlData = Data("""
@@ -59,4 +60,19 @@ class ReaderTests: XCTestCase {
         XCTAssertEqual(reader.children[1].nodeInfo.namespaceDef, .init(prefix: "a2abc", uri: "https://def.ghi.com"))
         XCTAssertEqual(reader.children.count, 3)
     }
+
+    func test_invalidXML_throws() throws {
+        // Same data as above, but the last closing tag is missing its '<'
+        let invalidXML = Data("""
+<a xmlns=\"https://abc.def.com\">
+    <a1 d=\"def\">x</a1>
+    <a2 xmlns:a2abc=\"https://def.ghi.com\" e=\"efg\">y</a2>
+    <a3>z</a3>
+/a>
+""".utf8)
+
+        XCTAssertThrowsError(try Reader.from(data: invalidXML)) { error in
+            XCTAssert(error is ResponseDecodingError)
+        }
+    }
 }