PM-14646: Fix JSON decoding errors (#1122)

matt-livefront · web-flow · commit 2a079717b7a6 · 2024-11-11T13:57:52.000-06:00
diff --git a/BitwardenShared/Core/Platform/Utilities/AnyCodable.swift b/BitwardenShared/Core/Platform/Utilities/AnyCodable.swift
@@ -6,6 +6,9 @@ public enum AnyCodable: Codable, Equatable, Sendable {
     /// The wrapped value is a bool value.
     case bool(Bool)
 
+    /// The wrapped value is a double value.
+    case double(Double)
+
     /// The wrapped value is an int value.
     case int(Int)
 
@@ -24,6 +27,9 @@ public enum AnyCodable: Codable, Equatable, Sendable {
             self = .bool(boolValue)
         } else if let intValue = try? container.decode(Int.self) {
             self = .int(intValue)
+        } else if let doubleValue = try? container.decode(Double.self) {
+            // Double needs to attempt decoding after `Int` otherwise it will capture any integer values.
+            self = .double(doubleValue)
         } else if container.decodeNil() {
             self = .null
         } else if let stringValue = try? container.decode(String.self) {
@@ -44,6 +50,8 @@ public enum AnyCodable: Codable, Equatable, Sendable {
         switch self {
         case let .bool(boolValue):
             try container.encode(boolValue)
+        case let .double(doubleValue):
+            try container.encode(doubleValue)
         case let .int(intValue):
             try container.encode(intValue)
         case .null:
@@ -61,10 +69,36 @@ extension AnyCodable {
         return boolValue
     }
 
-    /// Returns the associated int value if the type is `int`.
+    /// Returns the associated double value if the type is `double` or could be converted to `Double`.
+    var doubleValue: Double? {
+        switch self {
+        case let .bool(boolValue):
+            boolValue ? 1 : 0
+        case let .double(doubleValue):
+            doubleValue
+        case let .int(intValue):
+            Double(intValue)
+        case .null:
+            nil
+        case let .string(stringValue):
+            Double(stringValue)
+        }
+    }
+
+    /// Returns the associated int value if the type is `int` or could be converted to `Int`.
     var intValue: Int? {
-        guard case let .int(intValue) = self else { return nil }
-        return intValue
+        switch self {
+        case let .bool(boolValue):
+            boolValue ? 1 : 0
+        case let .double(doubleValue):
+            Int(doubleValue)
+        case let .int(intValue):
+            intValue
+        case .null:
+            nil
+        case let .string(stringValue):
+            Int(stringValue)
+        }
     }
 
     /// Returns the associated string value if the type is `string`.
diff --git a/BitwardenShared/Core/Platform/Utilities/AnyCodableTests.swift b/BitwardenShared/Core/Platform/Utilities/AnyCodableTests.swift
@@ -27,7 +27,8 @@ class AnyCodableTests: BitwardenTestCase {
           "requireNumbers": false,
           "requireSpecial": false,
           "enforceOnLogin": false,
-          "type": "password"
+          "type": "password",
+          "minutes": 1.5
         }
         """
 
@@ -45,10 +46,33 @@ class AnyCodableTests: BitwardenTestCase {
                 "requireSpecial": AnyCodable.bool(false),
                 "enforceOnLogin": AnyCodable.bool(false),
                 "type": AnyCodable.string("password"),
+                "minutes": AnyCodable.double(1.5),
             ]
         )
     }
 
+    /// `doubleValue` returns the double associated value if the type is a `double` or could be
+    /// converted to `Double`.
+    func test_doubleValue() {
+        XCTAssertEqual(AnyCodable.bool(true).doubleValue, 1)
+        XCTAssertEqual(AnyCodable.bool(false).doubleValue, 0)
+
+        XCTAssertEqual(AnyCodable.double(2).doubleValue, 2)
+        XCTAssertEqual(AnyCodable.double(3.1).doubleValue, 3.1)
+        XCTAssertEqual(AnyCodable.double(3.8).doubleValue, 3.8)
+        XCTAssertEqual(AnyCodable.double(-5.5).doubleValue, -5.5)
+
+        XCTAssertEqual(AnyCodable.int(1).doubleValue, 1)
+        XCTAssertEqual(AnyCodable.int(5).doubleValue, 5)
+        XCTAssertEqual(AnyCodable.int(15).doubleValue, 15)
+
+        XCTAssertNil(AnyCodable.null.doubleValue)
+
+        XCTAssertEqual(AnyCodable.string("1").doubleValue, 1)
+        XCTAssertEqual(AnyCodable.string("1.5").doubleValue, 1.5)
+        XCTAssertNil(AnyCodable.string("abc").doubleValue)
+    }
+
     /// `AnyCodable` can be used to encode JSON.
     func test_encode() throws {
         let dictionary: [String: AnyCodable] = [
@@ -60,6 +84,7 @@ class AnyCodableTests: BitwardenTestCase {
             "requireSpecial": AnyCodable.bool(false),
             "enforceOnLogin": AnyCodable.bool(false),
             "type": AnyCodable.string("password"),
+            "minutes": AnyCodable.double(1.5),
         ]
 
         let encoder = JSONEncoder()
@@ -74,6 +99,7 @@ class AnyCodableTests: BitwardenTestCase {
               "enforceOnLogin" : false,
               "minComplexity" : null,
               "minLength" : 12,
+              "minutes" : 1.5,
               "requireLower" : true,
               "requireNumbers" : false,
               "requireSpecial" : false,
@@ -84,13 +110,25 @@ class AnyCodableTests: BitwardenTestCase {
         )
     }
 
-    /// `intValue` returns the int associated value if the type is an `int`.
+    /// `intValue` returns the int associated value if the type is an `int` or could be converted
+    /// to `Int`.
     func test_intValue() {
+        XCTAssertEqual(AnyCodable.bool(true).intValue, 1)
+        XCTAssertEqual(AnyCodable.bool(false).intValue, 0)
+
+        XCTAssertEqual(AnyCodable.double(2).intValue, 2)
+        XCTAssertEqual(AnyCodable.double(3.1).intValue, 3)
+        XCTAssertEqual(AnyCodable.double(3.8).intValue, 3)
+        XCTAssertEqual(AnyCodable.double(-5.5).intValue, -5)
+
         XCTAssertEqual(AnyCodable.int(1).intValue, 1)
         XCTAssertEqual(AnyCodable.int(5).intValue, 5)
+        XCTAssertEqual(AnyCodable.int(15).intValue, 15)
 
-        XCTAssertNil(AnyCodable.bool(false).intValue)
         XCTAssertNil(AnyCodable.null.intValue)
+
+        XCTAssertEqual(AnyCodable.string("1").intValue, 1)
+        XCTAssertNil(AnyCodable.string("1.5").intValue)
         XCTAssertNil(AnyCodable.string("abc").intValue)
     }
 
diff --git a/BitwardenShared/Core/Platform/Utilities/DefaultValue.swift b/BitwardenShared/Core/Platform/Utilities/DefaultValue.swift
@@ -0,0 +1,99 @@
+import OSLog
+
+// MARK: - DefaultValueProvider
+
+/// A protocol for defining a default value for a `Decodable` type if an invalid or missing value
+/// is received.
+///
+protocol DefaultValueProvider: Decodable {
+    /// The default value to use if the value to decode is invalid or missing.
+    static var defaultValue: Self { get }
+}
+
+// MARK: - DefaultValue
+
+/// A property wrapper that will default the wrapped value to a default value if decoding fails.
+/// This is useful for decoding types which may not be present in the response or to prevent a
+/// decoding failure if an invalid value is received.
+///
+@propertyWrapper
+struct DefaultValue<T: DefaultValueProvider> {
+    // MARK: Properties
+
+    /// The wrapped value.
+    let wrappedValue: T
+}
+
+// MARK: - Decodable
+
+extension DefaultValue: Decodable {
+    init(from decoder: any Decoder) throws {
+        let container = try decoder.singleValueContainer()
+        do {
+            wrappedValue = try container.decode(T.self)
+        } catch {
+            if let intValue = try? container.decode(Int.self) {
+                Logger.application.warning(
+                    """
+                    Cannot initialize \(T.self) from invalid Int value \(intValue, privacy: .private), \
+                    defaulting to \(String(describing: T.defaultValue)).
+                    """
+                )
+            } else if let stringValue = try? container.decode(String.self) {
+                Logger.application.warning(
+                    """
+                    Cannot initialize \(T.self) from invalid String value \(stringValue, privacy: .private), \
+                    defaulting to \(String(describing: T.defaultValue))
+                    """
+                )
+            } else {
+                Logger.application.warning(
+                    """
+                    Cannot initialize \(T.self) from invalid unknown valid, \
+                    defaulting to \(String(describing: T.defaultValue))
+                    """
+                )
+            }
+            wrappedValue = T.defaultValue
+        }
+    }
+}
+
+// MARK: - Encodable
+
+extension DefaultValue: Encodable where T: Encodable {
+    func encode(to encoder: any Encoder) throws {
+        var container = encoder.singleValueContainer()
+        try container.encode(wrappedValue)
+    }
+}
+
+// MARK: - Equatable
+
+extension DefaultValue: Equatable where T: Equatable {}
+
+// MARK: - Hashable
+
+extension DefaultValue: Hashable where T: Hashable {}
+
+// MARK: - KeyedDecodingContainer
+
+extension KeyedDecodingContainer {
+    /// When decoding a `DefaultValue` wrapped value, if the property doesn't exist, default to the
+    /// type's default value.
+    ///
+    /// - Parameters:
+    ///   - type: The type of value to attempt to decode.
+    ///   - key: The key used to decode the value.
+    ///
+    func decode<T>(_ type: DefaultValue<T>.Type, forKey key: Key) throws -> DefaultValue<T> {
+        if let value = try decodeIfPresent(DefaultValue<T>.self, forKey: key) {
+            return value
+        } else {
+            Logger.application.warning(
+                "Missing value for \(T.self), defaulting to \(String(describing: T.defaultValue))"
+            )
+            return DefaultValue(wrappedValue: T.defaultValue)
+        }
+    }
+}
diff --git a/BitwardenShared/Core/Platform/Utilities/DefaultValueTests.swift b/BitwardenShared/Core/Platform/Utilities/DefaultValueTests.swift
@@ -0,0 +1,79 @@
+import XCTest
+
+@testable import BitwardenShared
+
+class DefaultValueTests: BitwardenTestCase {
+    // MARK: Types
+
+    enum ValueType: String, Codable, DefaultValueProvider {
+        case one, two, three
+
+        static var defaultValue: ValueType { .one }
+    }
+
+    struct Model: Codable, Equatable {
+        @DefaultValue var value: ValueType
+    }
+
+    // MARK: Tests
+
+    /// `DefaultValue` encodes the wrapped value.
+    func test_encode() throws {
+        let subject = Model(value: .two)
+        let data = try JSONEncoder().encode(subject)
+        XCTAssertEqual(String(data: data, encoding: .utf8), #"{"value":"two"}"#)
+    }
+
+    /// Decoding a `DefaultValue` wrapped value will use the default value if an array cannot be
+    /// initialized to the type.
+    func test_decode_invalidArray() throws {
+        let json = #"{"value": ["three"]}"#
+        let data = try XCTUnwrap(json.data(using: .utf8))
+        let subject = try JSONDecoder().decode(Model.self, from: data)
+        XCTAssertEqual(subject, Model(value: .one))
+    }
+
+    /// Decoding a `DefaultValue` wrapped value will use the default value if an int value cannot
+    /// be initialized to the type.
+    func test_decode_invalidInt() throws {
+        let json = #"{"value": 5}"#
+        let data = try XCTUnwrap(json.data(using: .utf8))
+        let subject = try JSONDecoder().decode(Model.self, from: data)
+        XCTAssertEqual(subject, Model(value: .one))
+    }
+
+    /// Decoding a `DefaultValue` wrapped value will use the default value if a string value cannot
+    /// be initialized to the type.
+    func test_decode_invalidString() throws {
+        let json = #"{"value": "unknown"}"#
+        let data = try XCTUnwrap(json.data(using: .utf8))
+        let subject = try JSONDecoder().decode(Model.self, from: data)
+        XCTAssertEqual(subject, Model(value: .one))
+    }
+
+    /// Decoding a `DefaultValue` wrapped value will use the default value if the value is
+    /// unknown in the JSON.
+    func test_decode_missing() throws {
+        let json = #"{}"#
+        let data = try XCTUnwrap(json.data(using: .utf8))
+        let subject = try JSONDecoder().decode(Model.self, from: data)
+        XCTAssertEqual(subject, Model(value: .one))
+    }
+
+    /// Decoding a `DefaultValue` wrapped value will use the default value if the value is `null`
+    /// in the JSON.
+    func test_decode_null() throws {
+        let json = #"{"value": null}"#
+        let data = try XCTUnwrap(json.data(using: .utf8))
+        let subject = try JSONDecoder().decode(Model.self, from: data)
+        XCTAssertEqual(subject, Model(value: .one))
+    }
+
+    /// Decoding a `DefaultValue` wrapped value will decode the enum value from the JSON.
+    func test_decode_value() throws {
+        let json = #"{"value": "three"}"#
+        let data = try XCTUnwrap(json.data(using: .utf8))
+        let subject = try JSONDecoder().decode(Model.self, from: data)
+        XCTAssertEqual(subject, Model(value: .three))
+    }
+}
diff --git a/BitwardenShared/Core/Vault/Models/API/CipherSecureNoteModel.swift b/BitwardenShared/Core/Vault/Models/API/CipherSecureNoteModel.swift
@@ -4,5 +4,5 @@ struct CipherSecureNoteModel: Codable, Equatable {
     // MARK: Properties
 
     /// The type of secure note.
-    let type: SecureNoteType
+    @DefaultValue var type: SecureNoteType
 }
diff --git a/BitwardenShared/Core/Vault/Models/Enum/CipherRepromptType.swift b/BitwardenShared/Core/Vault/Models/Enum/CipherRepromptType.swift
@@ -7,3 +7,9 @@ enum CipherRepromptType: Int, Codable {
     /// The user should be prompted for their master password prior to using the cipher password.
     case password = 1
 }
+
+// MARK: - DefaultValueProvider
+
+extension CipherRepromptType: DefaultValueProvider {
+    static var defaultValue: CipherRepromptType { .none }
+}
diff --git a/BitwardenShared/Core/Vault/Models/Enum/CipherRepromptTypeTests.swift b/BitwardenShared/Core/Vault/Models/Enum/CipherRepromptTypeTests.swift
@@ -0,0 +1,11 @@
+import XCTest
+
+@testable import BitwardenShared
+
+class CipherRepromptTypeTests: BitwardenTestCase {
+    /// `defaultValue` returns the default value for the type if an invalid or missing value is
+    /// received when decoding the type.
+    func test_defaultValue() {
+        XCTAssertEqual(CipherRepromptType.defaultValue, .none)
+    }
+}
diff --git a/BitwardenShared/Core/Vault/Models/Enum/SecureNoteType.swift b/BitwardenShared/Core/Vault/Models/Enum/SecureNoteType.swift
@@ -4,3 +4,9 @@ enum SecureNoteType: Int, Codable {
     /// A generic note.
     case generic = 0
 }
+
+// MARK: - DefaultValueProvider
+
+extension SecureNoteType: DefaultValueProvider {
+    static var defaultValue: SecureNoteType { .generic }
+}
diff --git a/BitwardenShared/Core/Vault/Models/Response/CipherDetailsResponseModel.swift b/BitwardenShared/Core/Vault/Models/Response/CipherDetailsResponseModel.swift
@@ -62,7 +62,7 @@ struct CipherDetailsResponseModel: JSONResponse, Equatable {
 
     /// Whether the user needs to be re-prompted for their master password prior to autofilling the
     /// cipher's password.
-    let reprompt: CipherRepromptType
+    @DefaultValue var reprompt: CipherRepromptType
 
     /// The date the cipher was last updated.
     let revisionDate: Date
diff --git a/BitwardenShared/Core/Vault/Models/SecureNoteTypeTests.swift b/BitwardenShared/Core/Vault/Models/SecureNoteTypeTests.swift
@@ -0,0 +1,11 @@
+import XCTest
+
+@testable import BitwardenShared
+
+class SecureNoteTypeTests: BitwardenTestCase {
+    /// `defaultValue` returns the default value for the type if an invalid or missing value is
+    /// received when decoding the type.
+    func test_defaultValue() {
+        XCTAssertEqual(SecureNoteType.defaultValue, .generic)
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -4,5 +4,5 @@ struct CipherSecureNoteModel: Codable, Equatable {`
`4`	`4`	`// MARK: Properties`
`5`	`5`
`6`	`6`	`/// The type of secure note.`
`7`		`- let type: SecureNoteType`
	`7`	`+ @DefaultValue var type: SecureNoteType`
`8`	`8`	`}`