Skip to content

Commit b0f5815

Browse files
jrose-applejrose-apple
authored
Merge pull request #20951 from pitiphong-p/encoders-nested-key-edge-case-bug
Reuse the container when requesting for a new nested container with the old key in JSONEncoder and PlistEncoder
2 parents 0919b1b + 61238ed commit b0f5815

File tree

4 files changed

+244
-10
lines changed

4 files changed

+244
-10
lines changed

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,18 @@ fileprivate struct _JSONKeyedEncodingContainer<K : CodingKey> : KeyedEncodingCon
507507
}
508508

509509
public mutating func nestedContainer<NestedKey>(keyedBy keyType: NestedKey.Type, forKey key: Key) -> KeyedEncodingContainer<NestedKey> {
510-
let dictionary = NSMutableDictionary()
511-
self.container[_converted(key).stringValue] = dictionary
510+
let containerKey = _converted(key).stringValue
511+
let dictionary: NSMutableDictionary
512+
if let existingContainer = self.container[containerKey] {
513+
precondition(
514+
existingContainer is NSMutableDictionary,
515+
"Attempt to re-encode into nested KeyedEncodingContainer<\(Key.self)> for key \"\(containerKey)\" is invalid: non-keyed container already encoded for this key"
516+
)
517+
dictionary = existingContainer as! NSMutableDictionary
518+
} else {
519+
dictionary = NSMutableDictionary()
520+
self.container[containerKey] = dictionary
521+
}
512522

513523
self.codingPath.append(key)
514524
defer { self.codingPath.removeLast() }
@@ -518,8 +528,18 @@ fileprivate struct _JSONKeyedEncodingContainer<K : CodingKey> : KeyedEncodingCon
518528
}
519529

520530
public mutating func nestedUnkeyedContainer(forKey key: Key) -> UnkeyedEncodingContainer {
521-
let array = NSMutableArray()
522-
self.container[_converted(key).stringValue] = array
531+
let containerKey = _converted(key).stringValue
532+
let array: NSMutableArray
533+
if let existingContainer = self.container[containerKey] {
534+
precondition(
535+
existingContainer is NSMutableArray,
536+
"Attempt to re-encode into nested UnkeyedEncodingContainer for key \"\(containerKey)\" is invalid: keyed container/single value already encoded for this key"
537+
)
538+
array = existingContainer as! NSMutableArray
539+
} else {
540+
array = NSMutableArray()
541+
self.container[containerKey] = array
542+
}
523543

524544
self.codingPath.append(key)
525545
defer { self.codingPath.removeLast() }

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

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,19 @@ fileprivate struct _PlistKeyedEncodingContainer<K : CodingKey> : KeyedEncodingCo
273273
}
274274

275275
public mutating func nestedContainer<NestedKey>(keyedBy keyType: NestedKey.Type, forKey key: Key) -> KeyedEncodingContainer<NestedKey> {
276-
let dictionary = NSMutableDictionary()
277-
self.container[key.stringValue] = dictionary
278-
276+
let containerKey = key.stringValue
277+
let dictionary: NSMutableDictionary
278+
if let existingContainer = self.container[containerKey] {
279+
precondition(
280+
existingContainer is NSMutableDictionary,
281+
"Attempt to re-encode into nested KeyedEncodingContainer<\(Key.self)> for key \"\(containerKey)\" is invalid: non-keyed container already encoded for this key"
282+
)
283+
dictionary = existingContainer as! NSMutableDictionary
284+
} else {
285+
dictionary = NSMutableDictionary()
286+
self.container[containerKey] = dictionary
287+
}
288+
279289
self.codingPath.append(key)
280290
defer { self.codingPath.removeLast() }
281291

@@ -284,8 +294,18 @@ fileprivate struct _PlistKeyedEncodingContainer<K : CodingKey> : KeyedEncodingCo
284294
}
285295

286296
public mutating func nestedUnkeyedContainer(forKey key: Key) -> UnkeyedEncodingContainer {
287-
let array = NSMutableArray()
288-
self.container[key.stringValue] = array
297+
let containerKey = key.stringValue
298+
let array: NSMutableArray
299+
if let existingContainer = self.container[containerKey] {
300+
precondition(
301+
existingContainer is NSMutableArray,
302+
"Attempt to re-encode into nested UnkeyedEncodingContainer for key \"\(containerKey)\" is invalid: keyed container/single value already encoded for this key"
303+
)
304+
array = existingContainer as! NSMutableArray
305+
} else {
306+
array = NSMutableArray()
307+
self.container[containerKey] = array
308+
}
289309

290310
self.codingPath.append(key)
291311
defer { self.codingPath.removeLast() }

‎test/stdlib/TestJSONEncoder.swift

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,103 @@ class TestJSONEncoder : TestJSONEncoderSuper {
102102
_testRoundTrip(of: TopLevelWrapper(EnhancedBool.false), expectedJSON: "{\"value\":false}".data(using: .utf8)!)
103103
_testRoundTrip(of: TopLevelWrapper(EnhancedBool.fileNotFound), expectedJSON: "{\"value\":null}".data(using: .utf8)!)
104104
}
105+
106+
func testEncodingMultipleNestedContainersWithTheSameTopLevelKey() {
107+
struct Model : Codable, Equatable {
108+
let first: String
109+
let second: String
110+
111+
init(from coder: Decoder) throws {
112+
let container = try coder.container(keyedBy: TopLevelCodingKeys.self)
113+
114+
let firstNestedContainer = try container.nestedContainer(keyedBy: FirstNestedCodingKeys.self, forKey: .top)
115+
self.first = try firstNestedContainer.decode(String.self, forKey: .first)
116+
117+
let secondNestedContainer = try container.nestedContainer(keyedBy: SecondNestedCodingKeys.self, forKey: .top)
118+
self.second = try secondNestedContainer.decode(String.self, forKey: .second)
119+
}
120+
121+
func encode(to encoder: Encoder) throws {
122+
var container = encoder.container(keyedBy: TopLevelCodingKeys.self)
123+
124+
var firstNestedContainer = container.nestedContainer(keyedBy: FirstNestedCodingKeys.self, forKey: .top)
125+
try firstNestedContainer.encode(self.first, forKey: .first)
126+
127+
var secondNestedContainer = container.nestedContainer(keyedBy: SecondNestedCodingKeys.self, forKey: .top)
128+
try secondNestedContainer.encode(self.second, forKey: .second)
129+
}
130+
131+
init(first: String, second: String) {
132+
self.first = first
133+
self.second = second
134+
}
135+
136+
static var testValue: Model {
137+
return Model(first: "Johnny Appleseed",
138+
second: "[email protected]")
139+
}
140+
141+
enum TopLevelCodingKeys : String, CodingKey {
142+
case top
143+
}
144+
145+
enum FirstNestedCodingKeys : String, CodingKey {
146+
case first
147+
}
148+
enum SecondNestedCodingKeys : String, CodingKey {
149+
case second
150+
}
151+
}
152+
153+
let model = Model.testValue
154+
if #available(OSX 10.13, iOS 11.0, watchOS 4.0, tvOS 11.0, *) {
155+
let expectedJSON = "{\"top\":{\"first\":\"Johnny Appleseed\",\"second\":\"[email protected]\"}}".data(using: .utf8)!
156+
_testRoundTrip(of: model, expectedJSON: expectedJSON, outputFormatting: [.sortedKeys])
157+
} else {
158+
_testRoundTrip(of: model)
159+
}
160+
}
161+
162+
func testEncodingConflictedTypeNestedContainersWithTheSameTopLevelKey() {
163+
struct Model : Encodable, Equatable {
164+
let first: String
105165

166+
func encode(to encoder: Encoder) throws {
167+
var container = encoder.container(keyedBy: TopLevelCodingKeys.self)
168+
169+
var firstNestedContainer = container.nestedContainer(keyedBy: FirstNestedCodingKeys.self, forKey: .top)
170+
try firstNestedContainer.encode(self.first, forKey: .first)
171+
172+
// The following line would fail as it attempts to re-encode into already encoded container is invalid. This will always fail
173+
var secondNestedContainer = container.nestedUnkeyedContainer(forKey: .top)
174+
try secondNestedContainer.encode("second")
175+
}
176+
177+
init(first: String) {
178+
self.first = first
179+
}
180+
181+
static var testValue: Model {
182+
return Model(first: "Johnny Appleseed")
183+
}
184+
185+
enum TopLevelCodingKeys : String, CodingKey {
186+
case top
187+
}
188+
enum FirstNestedCodingKeys : String, CodingKey {
189+
case first
190+
}
191+
}
192+
193+
let model = Model.testValue
194+
// This following test would fail as it attempts to re-encode into already encoded container is invalid. This will always fail
195+
if #available(OSX 10.13, iOS 11.0, watchOS 4.0, tvOS 11.0, *) {
196+
_testEncodeFailure(of: model)
197+
} else {
198+
_testEncodeFailure(of: model)
199+
}
200+
}
201+
106202
// MARK: - Output Formatting Tests
107203
func testEncodingOutputFormattingDefault() {
108204
let expectedJSON = "{\"name\":\"Johnny Appleseed\",\"email\":\"[email protected]\"}".data(using: .utf8)!
@@ -1615,6 +1711,12 @@ JSONEncoderTests.test("testEncodingTopLevelStructuredSingleClass") { TestJSONEnc
16151711
JSONEncoderTests.test("testEncodingTopLevelDeepStructuredType") { TestJSONEncoder().testEncodingTopLevelDeepStructuredType()}
16161712
JSONEncoderTests.test("testEncodingClassWhichSharesEncoderWithSuper") { TestJSONEncoder().testEncodingClassWhichSharesEncoderWithSuper() }
16171713
JSONEncoderTests.test("testEncodingTopLevelNullableType") { TestJSONEncoder().testEncodingTopLevelNullableType() }
1714+
JSONEncoderTests.test("testEncodingMultipleNestedContainersWithTheSameTopLevelKey") { TestJSONEncoder().testEncodingMultipleNestedContainersWithTheSameTopLevelKey() }
1715+
JSONEncoderTests.test("testEncodingConflictedTypeNestedContainersWithTheSameTopLevelKey") {
1716+
expectCrash() {
1717+
TestJSONEncoder().testEncodingConflictedTypeNestedContainersWithTheSameTopLevelKey()
1718+
}
1719+
}
16181720
JSONEncoderTests.test("testEncodingOutputFormattingDefault") { TestJSONEncoder().testEncodingOutputFormattingDefault() }
16191721
JSONEncoderTests.test("testEncodingOutputFormattingPrettyPrinted") { TestJSONEncoder().testEncodingOutputFormattingPrettyPrinted() }
16201722
JSONEncoderTests.test("testEncodingOutputFormattingSortedKeys") { TestJSONEncoder().testEncodingOutputFormattingSortedKeys() }

‎test/stdlib/TestPlistEncoder.swift

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,93 @@ class TestPropertyListEncoder : TestPropertyListEncoderSuper {
128128
_testRoundTrip(of: TopLevelWrapper(EnhancedBool.fileNotFound), in: .xml)
129129
}
130130

131-
131+
func testEncodingMultipleNestedContainersWithTheSameTopLevelKey() {
132+
struct Model : Codable, Equatable {
133+
let first: String
134+
let second: String
135+
136+
init(from coder: Decoder) throws {
137+
let container = try coder.container(keyedBy: TopLevelCodingKeys.self)
138+
139+
let firstNestedContainer = try container.nestedContainer(keyedBy: FirstNestedCodingKeys.self, forKey: .top)
140+
self.first = try firstNestedContainer.decode(String.self, forKey: .first)
141+
142+
let secondNestedContainer = try container.nestedContainer(keyedBy: SecondNestedCodingKeys.self, forKey: .top)
143+
self.second = try secondNestedContainer.decode(String.self, forKey: .second)
144+
}
145+
146+
func encode(to encoder: Encoder) throws {
147+
var container = encoder.container(keyedBy: TopLevelCodingKeys.self)
148+
149+
var firstNestedContainer = container.nestedContainer(keyedBy: FirstNestedCodingKeys.self, forKey: .top)
150+
try firstNestedContainer.encode(self.first, forKey: .first)
151+
152+
var secondNestedContainer = container.nestedContainer(keyedBy: SecondNestedCodingKeys.self, forKey: .top)
153+
try secondNestedContainer.encode(self.second, forKey: .second)
154+
}
155+
156+
init(first: String, second: String) {
157+
self.first = first
158+
self.second = second
159+
}
160+
161+
static var testValue: Model {
162+
return Model(first: "Johnny Appleseed",
163+
second: "[email protected]")
164+
}
165+
enum TopLevelCodingKeys : String, CodingKey {
166+
case top
167+
}
168+
169+
enum FirstNestedCodingKeys : String, CodingKey {
170+
case first
171+
}
172+
enum SecondNestedCodingKeys : String, CodingKey {
173+
case second
174+
}
175+
}
176+
177+
let model = Model.testValue
178+
let expectedXML = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n<plist version=\"1.0\">\n<dict>\n\t<key>top</key>\n\t<dict>\n\t\t<key>first</key>\n\t\t<string>Johnny Appleseed</string>\n\t\t<key>second</key>\n\t\t<string>[email protected]</string>\n\t</dict>\n</dict>\n</plist>\n".data(using: .utf8)!
179+
_testRoundTrip(of: model, in: .xml, expectedPlist: expectedXML)
180+
}
181+
182+
func testEncodingConflictedTypeNestedContainersWithTheSameTopLevelKey() {
183+
struct Model : Encodable, Equatable {
184+
let first: String
185+
186+
func encode(to encoder: Encoder) throws {
187+
var container = encoder.container(keyedBy: TopLevelCodingKeys.self)
188+
189+
var firstNestedContainer = container.nestedContainer(keyedBy: FirstNestedCodingKeys.self, forKey: .top)
190+
try firstNestedContainer.encode(self.first, forKey: .first)
191+
192+
// The following line would fail as it attempts to re-encode into already encoded container is invalid. This will always fail
193+
var secondNestedContainer = container.nestedUnkeyedContainer(forKey: .top)
194+
try secondNestedContainer.encode("second")
195+
}
196+
197+
init(first: String) {
198+
self.first = first
199+
}
200+
201+
static var testValue: Model {
202+
return Model(first: "Johnny Appleseed")
203+
}
204+
enum TopLevelCodingKeys : String, CodingKey {
205+
case top
206+
}
207+
208+
enum FirstNestedCodingKeys : String, CodingKey {
209+
case first
210+
}
211+
}
212+
213+
let model = Model.testValue
214+
// This following test would fail as it attempts to re-encode into already encoded container is invalid. This will always fail
215+
_testEncodeFailure(of: model, in: .xml)
216+
}
217+
132218
// MARK: - Encoder Features
133219
func testNestedContainerCodingPaths() {
134220
let encoder = JSONEncoder()
@@ -787,6 +873,12 @@ PropertyListEncoderTests.test("testEncodingTopLevelStructuredSingleClass") { Tes
787873
PropertyListEncoderTests.test("testEncodingTopLevelDeepStructuredType") { TestPropertyListEncoder().testEncodingTopLevelDeepStructuredType() }
788874
PropertyListEncoderTests.test("testEncodingClassWhichSharesEncoderWithSuper") { TestPropertyListEncoder().testEncodingClassWhichSharesEncoderWithSuper() }
789875
PropertyListEncoderTests.test("testEncodingTopLevelNullableType") { TestPropertyListEncoder().testEncodingTopLevelNullableType() }
876+
PropertyListEncoderTests.test("testEncodingMultipleNestedContainersWithTheSameTopLevelKey") { TestPropertyListEncoder().testEncodingMultipleNestedContainersWithTheSameTopLevelKey() }
877+
PropertyListEncoderTests.test("testEncodingConflictedTypeNestedContainersWithTheSameTopLevelKey") {
878+
expectCrash() {
879+
TestPropertyListEncoder().testEncodingConflictedTypeNestedContainersWithTheSameTopLevelKey()
880+
}
881+
}
790882
PropertyListEncoderTests.test("testNestedContainerCodingPaths") { TestPropertyListEncoder().testNestedContainerCodingPaths() }
791883
PropertyListEncoderTests.test("testSuperEncoderCodingPaths") { TestPropertyListEncoder().testSuperEncoderCodingPaths() }
792884
PropertyListEncoderTests.test("testEncodingTopLevelData") { TestPropertyListEncoder().testEncodingTopLevelData() }

0 commit comments

Comments
 (0)