Skip to content

Commit 3fd0ab9

Browse files
authored
Decode meta_data incorrectly sent as an object (#16141)
2 parents c4cad38 + fa134fa commit 3fd0ab9

14 files changed

+633
-29
lines changed

Modules/Sources/Networking/Model/Product/Product.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ public struct Product: Codable, GeneratedCopiable, Equatable, GeneratedFakeable
457457
// `true`
458458
value.lowercased() == Values.manageStockParent ? true : false
459459
})
460-
]) ?? false
460+
]) ?? false
461461

462462
// Even though WooCommerce Core returns Int or null values,
463463
// some plugins alter the field value from Int to Decimal or String.
@@ -535,15 +535,17 @@ public struct Product: Codable, GeneratedCopiable, Equatable, GeneratedFakeable
535535
let menuOrder = try container.decode(Int.self, forKey: .menuOrder)
536536

537537
// Filter out metadata if the key is prefixed with an underscore (internal meta keys)
538-
let customFields = (try? container.decode([MetaData].self, forKey: .metadata).filter({ !$0.key.hasPrefix("_")})) ?? []
538+
// Support both array format and object keyed by index strings
539+
let allMetaData = [MetaData].decodeFlexibly(from: container, forKey: .metadata)
540+
let customFields = allMetaData.filter { !$0.key.hasPrefix("_") }
539541

540542
// In some isolated cases, it appears to be some malformed meta-data that causes this line to throw hence the whole product decoding to throw.
541543
// Since add-ons are optional, `try?` will be used to prevent the whole decoding to stop.
542544
// https://github.com/woocommerce/woocommerce-ios/issues/4205
543545
let addOns = (try? container.decodeIfPresent(ProductAddOnEnvelope.self, forKey: .metadata)?.revolve()) ?? []
544546

545-
let metaDataExtractor = try? container.decodeIfPresent(ProductMetadataExtractor.self, forKey: .metadata)
546-
let isSampleItem = (metaDataExtractor?.extractStringValue(forKey: MetadataKeys.headStartPost) == Values.headStartValue)
547+
let metaDataExtractor = ProductMetadataExtractor(metadata: allMetaData)
548+
let isSampleItem = (metaDataExtractor.extractStringValue(forKey: MetadataKeys.headStartPost) == Values.headStartValue)
547549

548550
// Product Bundle properties
549551
// Uses failsafe decoding because non-bundle product types can return unexpected value types.
@@ -561,7 +563,7 @@ public struct Product: Codable, GeneratedCopiable, Equatable, GeneratedFakeable
561563
let compositeComponents = try container.decodeIfPresent([ProductCompositeComponent].self, forKey: .compositeComponents) ?? []
562564

563565
// Subscription properties
564-
let subscription = try? metaDataExtractor?.extractProductSubscription()
566+
let subscription = try? metaDataExtractor.extractProductSubscription()
565567

566568
// Min/Max Quantities properties
567569
let minAllowedQuantity = container.failsafeDecodeIfPresent(stringForKey: .minAllowedQuantity)

Modules/Sources/Networking/Model/Product/ProductMetadataExtractor.swift

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Foundation
22
import WordPressShared
3+
import NetworkingCore
34

45
/// Helper to extract specific data from inside `Product` metadata.
56
/// Sample Json:
@@ -21,25 +22,23 @@ import WordPressShared
2122
/// }
2223
/// ]
2324
///
24-
internal struct ProductMetadataExtractor: Decodable {
25+
struct ProductMetadataExtractor {
2526

26-
private typealias DecodableDictionary = [String: AnyDecodable]
2727
private typealias AnyDictionary = [String: Any?]
2828

2929
/// Internal metadata representation
3030
///
31-
private let metadata: [DecodableDictionary]
31+
private let metadata: [MetaData]
3232

33-
/// Decode main metadata array as an untyped dictionary.
33+
/// Initialize with already-decoded metadata array
3434
///
35-
init(from decoder: Decoder) throws {
36-
let container = try decoder.singleValueContainer()
37-
self.metadata = try container.decode([DecodableDictionary].self)
35+
init(metadata: [MetaData]) {
36+
self.metadata = metadata
3837
}
3938

4039
/// Searches product metadata for subscription data and converts it to a `ProductSubscription` if possible.
4140
///
42-
internal func extractProductSubscription() throws -> ProductSubscription? {
41+
func extractProductSubscription() throws -> ProductSubscription? {
4342
let subscriptionMetadata = filterMetadata(with: Constants.subscriptionPrefix)
4443

4544
guard !subscriptionMetadata.isEmpty else {
@@ -54,30 +53,33 @@ internal struct ProductMetadataExtractor: Decodable {
5453

5554
/// Extracts a `String` metadata value for the provided key.
5655
///
57-
internal func extractStringValue(forKey key: String) -> String? {
56+
func extractStringValue(forKey key: String) -> String? {
5857
let metaData = filterMetadata(with: key)
5958
let keyValueMetadata = getKeyValueDictionary(from: metaData)
6059
return keyValueMetadata.valueAsString(forKey: key)
6160
}
6261

6362
/// Filters product metadata using the provided prefix.
6463
///
65-
private func filterMetadata(with prefix: String) -> [DecodableDictionary] {
66-
metadata.filter { object in
67-
let objectKey = object["key"]?.value as? String ?? ""
68-
return objectKey.hasPrefix(prefix)
69-
}
64+
private func filterMetadata(with prefix: String) -> [MetaData] {
65+
metadata.filter { $0.key.hasPrefix(prefix) }
7066
}
7167

7268
/// Parses provided metadata to return a dictionary with each metadata object's key and value.
7369
///
74-
private func getKeyValueDictionary(from metadata: [DecodableDictionary]) -> AnyDictionary {
75-
metadata.reduce(AnyDictionary()) { (dict, object) in
76-
var newDict = dict
77-
let objectKey = object["key"]?.value as? String ?? ""
78-
let objectValue = object["value"]?.value
79-
newDict.updateValue(objectValue, forKey: objectKey)
80-
return newDict
70+
private func getKeyValueDictionary(from metadata: [MetaData]) -> AnyDictionary {
71+
metadata.reduce(into: AnyDictionary()) { dict, object in
72+
// For JSON values, decode them to get the actual object
73+
if object.value.isJson {
74+
if let data = object.value.stringValue.data(using: .utf8),
75+
let jsonObject = try? JSONSerialization.jsonObject(with: data, options: []) {
76+
dict[object.key] = jsonObject
77+
} else {
78+
dict[object.key] = object.value.stringValue
79+
}
80+
} else {
81+
dict[object.key] = object.value.stringValue
82+
}
8183
}
8284
}
8385

Modules/Sources/Networking/Model/Product/ProductVariation.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ public struct ProductVariation: Codable, GeneratedCopiable, Equatable, Generated
279279
}
280280
return false
281281
})
282-
]) ?? false
282+
]) ?? false
283283

284284
// Even though WooCommerce Core returns Int or null values,
285285
// some plugins alter the field value from Int to Decimal or String.
@@ -309,7 +309,9 @@ public struct ProductVariation: Codable, GeneratedCopiable, Equatable, Generated
309309
let menuOrder = try container.decode(Int64.self, forKey: .menuOrder)
310310

311311
// Subscription settings for subscription variations
312-
let subscription = try? container.decodeIfPresent(ProductMetadataExtractor.self, forKey: .metadata)?.extractProductSubscription()
312+
let allMetaData = [MetaData].decodeFlexibly(from: container, forKey: .metadata)
313+
let metaDataExtractor = ProductMetadataExtractor(metadata: allMetaData)
314+
let subscription = try? metaDataExtractor.extractProductSubscription()
313315

314316
// Min/Max Quantities properties
315317
let minAllowedQuantity = container.failsafeDecodeIfPresent(stringForKey: .minAllowedQuantity)
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import Foundation
2+
3+
/// Extension to support flexible decoding of MetaData arrays
4+
/// Handles both standard array format and object format keyed by index strings
5+
extension Array where Element == MetaData {
6+
7+
/// Custom decoding from container that supports both array and dictionary formats
8+
public static func decodeFlexibly<K>(from container: KeyedDecodingContainer<K>,
9+
forKey key: KeyedDecodingContainer<K>.Key) -> [MetaData] {
10+
// Try to decode as array first (standard format)
11+
if let metaDataArray = try? container.decode([MetaData].self, forKey: key) {
12+
return metaDataArray
13+
}
14+
15+
// Try to decode as object keyed by index strings
16+
if let metaDataDict = try? container.decode([String: MetaData].self, forKey: key) {
17+
return Array(metaDataDict.values)
18+
}
19+
20+
// Fallback to empty array
21+
DDLogWarn("⚠️ Could not decode metadata as either an array or object keyed by index strings. Falling back to empty array.")
22+
return []
23+
}
24+
}

Modules/Sources/NetworkingCore/Model/Order.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,10 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
201201
// "payment_url" is only available on stores with version >= 6.4
202202
let paymentURL = try container.decodeIfPresent(URL.self, forKey: .paymentURL)
203203

204-
let allOrderMetaData = try? container.decode([MetaData].self, forKey: .metadata)
204+
let allOrderMetaData: [MetaData]? = {
205+
let metadata = [MetaData].decodeFlexibly(from: container, forKey: .metadata)
206+
return metadata.isEmpty ? nil : metadata
207+
}()
205208
var chargeID: String? = nil
206209
chargeID = allOrderMetaData?.first(where: { $0.key == "_charge_id" })?.value.stringValue
207210

Modules/Tests/NetworkingTests/Mapper/MetaDataMapperTests.swift

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,136 @@ final class MetaDataMapperTests: XCTestCase {
4949
XCTAssertEqual(metadata[5], MetaData(metadataID: 6, key: "number_field", value: "42"))
5050
XCTAssertEqual(metadata[6], MetaData(metadataID: 7, key: "empty_field", value: ""))
5151
}
52+
53+
/// Tests that flexible metadata decoding works with array format in a realistic context
54+
///
55+
func test_flexible_metadata_decoding_array_format() throws {
56+
// Given - JSON with metadata as array in a product-like structure
57+
let jsonString = """
58+
{
59+
"id": 123,
60+
"name": "Test Product",
61+
"meta_data": [
62+
{
63+
"id": 1001,
64+
"key": "custom_field_1",
65+
"value": "value1"
66+
},
67+
{
68+
"id": 1002,
69+
"key": "_internal_field",
70+
"value": "internal_value"
71+
},
72+
{
73+
"id": 1003,
74+
"key": "custom_field_2",
75+
"value": "value2"
76+
}
77+
]
78+
}
79+
"""
80+
81+
struct TestProduct: Decodable {
82+
let id: Int
83+
let name: String
84+
let metadata: [MetaData]
85+
86+
private enum CodingKeys: String, CodingKey {
87+
case id, name
88+
case metadata = "meta_data"
89+
}
90+
91+
init(from decoder: Decoder) throws {
92+
let container = try decoder.container(keyedBy: CodingKeys.self)
93+
self.id = try container.decode(Int.self, forKey: .id)
94+
self.name = try container.decode(String.self, forKey: .name)
95+
96+
// Flexible decoding logic using helper
97+
self.metadata = [MetaData].decodeFlexibly(from: container, forKey: .metadata)
98+
}
99+
}
100+
101+
let data = jsonString.data(using: .utf8)!
102+
let decoder = JSONDecoder()
103+
104+
// When
105+
let product = try decoder.decode(TestProduct.self, from: data)
106+
107+
// Then
108+
XCTAssertEqual(product.metadata.count, 3) // All fields should be present
109+
XCTAssertEqual(product.metadata[0].metadataID, 1001)
110+
XCTAssertEqual(product.metadata[0].key, "custom_field_1")
111+
XCTAssertEqual(product.metadata[0].value.stringValue, "value1")
112+
XCTAssertEqual(product.metadata[1].metadataID, 1002)
113+
XCTAssertEqual(product.metadata[1].key, "_internal_field")
114+
XCTAssertEqual(product.metadata[1].value.stringValue, "internal_value")
115+
XCTAssertEqual(product.metadata[2].metadataID, 1003)
116+
XCTAssertEqual(product.metadata[2].key, "custom_field_2")
117+
XCTAssertEqual(product.metadata[2].value.stringValue, "value2")
118+
}
119+
120+
/// Tests that flexible metadata decoding works with dictionary format in a realistic context
121+
///
122+
func test_flexible_metadata_decoding_dictionary_format() throws {
123+
// Given - JSON with metadata as object keyed by index strings in a product-like structure
124+
let jsonString = """
125+
{
126+
"id": 456,
127+
"name": "Test Product 2",
128+
"meta_data": {
129+
"0": {
130+
"id": 2001,
131+
"key": "dict_field_1",
132+
"value": "dict_value1"
133+
},
134+
"1": {
135+
"id": 2002,
136+
"key": "_internal_dict_field",
137+
"value": "internal_dict_value"
138+
},
139+
"2": {
140+
"id": 2003,
141+
"key": "dict_field_2",
142+
"value": "dict_value2"
143+
}
144+
}
145+
}
146+
"""
147+
148+
struct TestProduct: Decodable {
149+
let id: Int
150+
let name: String
151+
let metadata: [MetaData]
152+
153+
private enum CodingKeys: String, CodingKey {
154+
case id, name
155+
case metadata = "meta_data"
156+
}
157+
158+
init(from decoder: Decoder) throws {
159+
let container = try decoder.container(keyedBy: CodingKeys.self)
160+
self.id = try container.decode(Int.self, forKey: .id)
161+
self.name = try container.decode(String.self, forKey: .name)
162+
163+
// Flexible decoding logic using helper
164+
self.metadata = [MetaData].decodeFlexibly(from: container, forKey: .metadata)
165+
}
166+
}
167+
168+
let data = jsonString.data(using: .utf8)!
169+
let decoder = JSONDecoder()
170+
171+
// When
172+
let product = try decoder.decode(TestProduct.self, from: data)
173+
174+
// Then
175+
XCTAssertEqual(product.metadata.count, 3) // All fields should be present
176+
let fieldNames = Set(product.metadata.map { $0.key })
177+
XCTAssertTrue(fieldNames.contains("dict_field_1"))
178+
XCTAssertTrue(fieldNames.contains("dict_field_2"))
179+
XCTAssertTrue(fieldNames.contains("_internal_dict_field"))
180+
}
181+
52182
}
53183

54184
// MARK: - Test Helpers

0 commit comments

Comments
 (0)