Skip to content

Commit 9b8d1ba

Browse files
author
Itai Ferber
committed
Restore decoder state after throwing on decode
Resolve SR-6408 by restoring the JSON/PlistDecoder stack if an error is thrown after a container was pushed during encode.
1 parent 91c45b4 commit 9b8d1ba

File tree

4 files changed

+94
-37
lines changed

4 files changed

+94
-37
lines changed

‎stdlib/public/SDK/Foundation/JSONEncoder.swift

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2277,9 +2277,8 @@ extension _JSONDecoder {
22772277
switch self.options.dateDecodingStrategy {
22782278
case .deferredToDate:
22792279
self.storage.push(container: value)
2280-
let date = try Date(from: self)
2281-
self.storage.popContainer()
2282-
return date
2280+
defer { self.storage.popContainer() }
2281+
return try Date(from: self)
22832282

22842283
case .secondsSince1970:
22852284
let double = try self.unbox(value, as: Double.self)!
@@ -2311,9 +2310,8 @@ extension _JSONDecoder {
23112310

23122311
case .custom(let closure):
23132312
self.storage.push(container: value)
2314-
let date = try closure(self)
2315-
self.storage.popContainer()
2316-
return date
2313+
defer { self.storage.popContainer() }
2314+
return try closure(self)
23172315
}
23182316
}
23192317

@@ -2323,9 +2321,8 @@ extension _JSONDecoder {
23232321
switch self.options.dataDecodingStrategy {
23242322
case .deferredToData:
23252323
self.storage.push(container: value)
2326-
let data = try Data(from: self)
2327-
self.storage.popContainer()
2328-
return data
2324+
defer { self.storage.popContainer() }
2325+
return try Data(from: self)
23292326

23302327
case .base64:
23312328
guard let string = value as? String else {
@@ -2340,9 +2337,8 @@ extension _JSONDecoder {
23402337

23412338
case .custom(let closure):
23422339
self.storage.push(container: value)
2343-
let data = try closure(self)
2344-
self.storage.popContainer()
2345-
return data
2340+
defer { self.storage.popContainer() }
2341+
return try closure(self)
23462342
}
23472343
}
23482344

@@ -2359,13 +2355,10 @@ extension _JSONDecoder {
23592355
}
23602356

23612357
fileprivate func unbox<T : Decodable>(_ value: Any, as type: T.Type) throws -> T? {
2362-
let decoded: T
23632358
if type == Date.self || type == NSDate.self {
2364-
guard let date = try self.unbox(value, as: Date.self) else { return nil }
2365-
decoded = date as! T
2359+
return try self.unbox(value, as: Date.self) as? T
23662360
} else if type == Data.self || type == NSData.self {
2367-
guard let data = try self.unbox(value, as: Data.self) else { return nil }
2368-
decoded = data as! T
2361+
return try self.unbox(value, as: Data.self) as? T
23692362
} else if type == URL.self || type == NSURL.self {
23702363
guard let urlString = try self.unbox(value, as: String.self) else {
23712364
return nil
@@ -2376,17 +2369,14 @@ extension _JSONDecoder {
23762369
debugDescription: "Invalid URL string."))
23772370
}
23782371

2379-
decoded = (url as! T)
2372+
return url as! T
23802373
} else if type == Decimal.self || type == NSDecimalNumber.self {
2381-
guard let decimal = try self.unbox(value, as: Decimal.self) else { return nil }
2382-
decoded = decimal as! T
2374+
return try self.unbox(value, as: Decimal.self) as? T
23832375
} else {
23842376
self.storage.push(container: value)
2385-
decoded = try type.init(from: self)
2386-
self.storage.popContainer()
2377+
defer { self.storage.popContainer() }
2378+
return try type.init(from: self)
23872379
}
2388-
2389-
return decoded
23902380
}
23912381
}
23922382

‎stdlib/public/SDK/Foundation/PlistEncoder.swift

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1764,20 +1764,15 @@ extension _PlistDecoder {
17641764
}
17651765

17661766
fileprivate func unbox<T : Decodable>(_ value: Any, as type: T.Type) throws -> T? {
1767-
let decoded: T
17681767
if type == Date.self || type == NSDate.self {
1769-
guard let date = try self.unbox(value, as: Date.self) else { return nil }
1770-
decoded = date as! T
1768+
return try self.unbox(value, as: Date.self) as? T
17711769
} else if type == Data.self || type == NSData.self {
1772-
guard let data = try self.unbox(value, as: Data.self) else { return nil }
1773-
decoded = data as! T
1770+
return try self.unbox(value, as: Data.self) as? T
17741771
} else {
17751772
self.storage.push(container: value)
1776-
decoded = try type.init(from: self)
1777-
self.storage.popContainer()
1773+
defer { self.storage.popContainer() }
1774+
return try type.init(from: self)
17781775
}
1779-
1780-
return decoded
17811776
}
17821777
}
17831778

‎test/stdlib/TestJSONEncoder.swift

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,40 @@ class TestJSONEncoder : TestJSONEncoderSuper {
878878
_ = try? encoder.encode(ReferencingEncoderWrapper(Data()))
879879
}
880880

881+
// MARK: - Decoder State
882+
// SR-6048
883+
func testDecoderStateThrowOnDecode() {
884+
// The container stack here starts as [[1,2,3]]. Attempting to decode as [String] matches the outer layer (Array), and begins decoding the array.
885+
// Once Array decoding begins, 1 is pushed onto the container stack ([[1,2,3], 1]), and 1 is attempted to be decoded as String. This throws a .typeMismatch, but the container is not popped off the stack.
886+
// When attempting to decode [Int], the container stack is still ([[1,2,3], 1]), and 1 fails to decode as [Int].
887+
let json = "[1,2,3]".data(using: .utf8)!
888+
let _ = try! JSONDecoder().decode(EitherDecodable<[String], [Int]>.self, from: json)
889+
}
890+
891+
func testDecoderStateThrowOnDecodeCustomDate() {
892+
// This test is identical to testDecoderStateThrowOnDecode, except we're going to fail because our closure throws an error, not because we hit a type mismatch.
893+
let decoder = JSONDecoder()
894+
decoder.dateDecodingStrategy = .custom({ decoder in
895+
enum CustomError : Error { case foo }
896+
throw CustomError.foo
897+
})
898+
899+
let json = "{\"value\": 1}".data(using: .utf8)!
900+
let _ = try! decoder.decode(EitherDecodable<TopLevelWrapper<Date>, TopLevelWrapper<Int>>.self, from: json)
901+
}
902+
903+
func testDecoderStateThrowOnDecodeCustomData() {
904+
// This test is identical to testDecoderStateThrowOnDecode, except we're going to fail because our closure throws an error, not because we hit a type mismatch.
905+
let decoder = JSONDecoder()
906+
decoder.dataDecodingStrategy = .custom({ decoder in
907+
enum CustomError : Error { case foo }
908+
throw CustomError.foo
909+
})
910+
911+
let json = "{\"value\": 1}".data(using: .utf8)!
912+
let _ = try! decoder.decode(EitherDecodable<TopLevelWrapper<Data>, TopLevelWrapper<Int>>.self, from: json)
913+
}
914+
881915
// MARK: - Helper Functions
882916
private var _jsonEmptyDictionary: Data {
883917
return "{}".data(using: .utf8)!
@@ -1470,6 +1504,20 @@ fileprivate struct DoubleNaNPlaceholder : Codable, Equatable {
14701504
}
14711505
}
14721506

1507+
fileprivate enum EitherDecodable<T : Decodable, U : Decodable> : Decodable {
1508+
case t(T)
1509+
case u(U)
1510+
1511+
init(from decoder: Decoder) throws {
1512+
let container = try decoder.singleValueContainer()
1513+
do {
1514+
self = .t(try container.decode(T.self))
1515+
} catch {
1516+
self = .u(try container.decode(U.self))
1517+
}
1518+
}
1519+
}
1520+
14731521
// MARK: - Run Tests
14741522

14751523
#if !FOUNDATION_XCTEST
@@ -1521,5 +1569,8 @@ JSONEncoderTests.test("testDecodingConcreteTypeParameter") { TestJSONEncoder().t
15211569
JSONEncoderTests.test("testEncoderStateThrowOnEncode") { TestJSONEncoder().testEncoderStateThrowOnEncode() }
15221570
JSONEncoderTests.test("testEncoderStateThrowOnEncodeCustomDate") { TestJSONEncoder().testEncoderStateThrowOnEncodeCustomDate() }
15231571
JSONEncoderTests.test("testEncoderStateThrowOnEncodeCustomData") { TestJSONEncoder().testEncoderStateThrowOnEncodeCustomData() }
1572+
JSONEncoderTests.test("testDecoderStateThrowOnDecode") { TestJSONEncoder().testDecoderStateThrowOnDecode() }
1573+
JSONEncoderTests.test("testDecoderStateThrowOnDecodeCustomDate") { TestJSONEncoder().testDecoderStateThrowOnDecodeCustomDate() }
1574+
JSONEncoderTests.test("testDecoderStateThrowOnDecodeCustomData") { TestJSONEncoder().testDecoderStateThrowOnDecodeCustomData() }
15241575
runAllTests()
15251576
#endif

‎test/stdlib/TestPlistEncoder.swift

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,8 @@ class TestPropertyListEncoder : TestPropertyListEncoderSuper {
234234
}
235235

236236
struct Throwing : Encodable {
237-
private enum EncodingError : Error {
238-
case foo
239-
}
240-
241237
func encode(to encoder: Encoder) throws {
238+
enum EncodingError : Error { case foo }
242239
throw EncodingError.foo
243240
}
244241
}
@@ -261,6 +258,13 @@ class TestPropertyListEncoder : TestPropertyListEncoderSuper {
261258
_ = try? PropertyListEncoder().encode(Wrapper([Throwing()]))
262259
}
263260

261+
// MARK: - Encoder State
262+
// SR-6048
263+
func testDecoderStateThrowOnDecode() {
264+
let plist = try! PropertyListEncoder().encode([1,2,3])
265+
let _ = try! PropertyListDecoder().decode(EitherDecodable<[String], [Int]>.self, from: plist)
266+
}
267+
264268
// MARK: - Helper Functions
265269
private var _plistEmptyDictionaryBinary: Data {
266270
return Data(base64Encoded: "YnBsaXN0MDDQCAAAAAAAAAEBAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAJ")!
@@ -772,6 +776,22 @@ fileprivate struct TopLevelWrapper<T> : Codable, Equatable where T : Codable, T
772776
}
773777
}
774778

779+
fileprivate enum EitherDecodable<T : Decodable, U : Decodable> : Decodable {
780+
case t(T)
781+
case u(U)
782+
783+
init(from decoder: Decoder) throws {
784+
let container = try decoder.singleValueContainer()
785+
if let t = try? container.decode(T.self) {
786+
self = .t(t)
787+
} else if let u = try? container.decode(U.self) {
788+
self = .u(u)
789+
} else {
790+
throw DecodingError.dataCorruptedError(in: container, debugDescription: "Data was neither \(T.self) nor \(U.self).")
791+
}
792+
}
793+
}
794+
775795
// MARK: - Run Tests
776796

777797
#if !FOUNDATION_XCTEST
@@ -796,5 +816,6 @@ PropertyListEncoderTests.test("testInterceptDate") { TestPropertyListEncoder().t
796816
PropertyListEncoderTests.test("testTypeCoercion") { TestPropertyListEncoder().testTypeCoercion() }
797817
PropertyListEncoderTests.test("testDecodingConcreteTypeParameter") { TestPropertyListEncoder().testDecodingConcreteTypeParameter() }
798818
PropertyListEncoderTests.test("testEncoderStateThrowOnEncode") { TestPropertyListEncoder().testEncoderStateThrowOnEncode() }
819+
PropertyListEncoderTests.test("testDecoderStateThrowOnDecode") { TestPropertyListEncoder().testDecoderStateThrowOnDecode() }
799820
runAllTests()
800821
#endif

0 commit comments

Comments
 (0)