diff --git a/Package.swift b/Package.swift index 5d933b521..79c68b6de 100644 --- a/Package.swift +++ b/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", diff --git a/Sources/SmithyJSON/Reader/Reader+JSONDeserialization.swift b/Sources/SmithyJSON/Reader/Reader+JSONDeserialization.swift index 8f44ce0be..022a79254 100644 --- a/Sources/SmithyJSON/Reader/Reader+JSONDeserialization.swift +++ b/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) } } } diff --git a/Sources/SmithySerialization/ResponseDecodingError.swift b/Sources/SmithySerialization/ResponseDecodingError.swift new file mode 100644 index 000000000..9b3bafb17 --- /dev/null +++ b/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)" + } +} diff --git a/Sources/SmithyXML/Reader/Reader+libxml2.swift b/Sources/SmithyXML/Reader/Reader+libxml2.swift index 3ae5c35f2..bd85eac9f 100644 --- a/Sources/SmithyXML/Reader/Reader+libxml2.swift +++ b/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") } diff --git a/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift b/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift index df4fe7b8c..fe28f2d36 100644 --- a/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift +++ b/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift @@ -224,10 +224,11 @@ 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") @@ -235,8 +236,8 @@ class OrchestratorTests: XCTestCase { 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 { diff --git a/Tests/SmithyJSONTests/ReaderTests.swift b/Tests/SmithyJSONTests/ReaderTests.swift index 76a10b30f..fd256e84b 100644 --- a/Tests/SmithyJSONTests/ReaderTests.swift +++ b/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) + } + } } diff --git a/Tests/SmithyXMLTests/ReaderTests.swift b/Tests/SmithyXMLTests/ReaderTests.swift index 34af35b8d..b2767cba5 100644 --- a/Tests/SmithyXMLTests/ReaderTests.swift +++ b/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(""" + + x + y + z +/a> +""".utf8) + + XCTAssertThrowsError(try Reader.from(data: invalidXML)) { error in + XCTAssert(error is ResponseDecodingError) + } + } }