Skip to content

Commit

Permalink
PM-14646: Fix JSON decoding errors (#1122)
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-livefront authored Nov 11, 2024
1 parent 91a1853 commit 2a07971
Show file tree
Hide file tree
Showing 10 changed files with 292 additions and 8 deletions.
40 changes: 37 additions & 3 deletions BitwardenShared/Core/Platform/Utilities/AnyCodable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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) {
Expand All @@ -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:
Expand All @@ -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`.
Expand Down
44 changes: 41 additions & 3 deletions BitwardenShared/Core/Platform/Utilities/AnyCodableTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class AnyCodableTests: BitwardenTestCase {
"requireNumbers": false,
"requireSpecial": false,
"enforceOnLogin": false,
"type": "password"
"type": "password",
"minutes": 1.5
}
"""

Expand All @@ -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] = [
Expand All @@ -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()
Expand All @@ -74,6 +99,7 @@ class AnyCodableTests: BitwardenTestCase {
"enforceOnLogin" : false,
"minComplexity" : null,
"minLength" : 12,
"minutes" : 1.5,
"requireLower" : true,
"requireNumbers" : false,
"requireSpecial" : false,
Expand All @@ -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)
}

Expand Down
99 changes: 99 additions & 0 deletions BitwardenShared/Core/Platform/Utilities/DefaultValue.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
79 changes: 79 additions & 0 deletions BitwardenShared/Core/Platform/Utilities/DefaultValueTests.swift
Original file line number Diff line number Diff line change
@@ -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))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ struct CipherSecureNoteModel: Codable, Equatable {
// MARK: Properties

/// The type of secure note.
let type: SecureNoteType
@DefaultValue var type: SecureNoteType
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Original file line number Diff line number Diff line change
@@ -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)
}
}
6 changes: 6 additions & 0 deletions BitwardenShared/Core/Vault/Models/Enum/SecureNoteType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,9 @@ enum SecureNoteType: Int, Codable {
/// A generic note.
case generic = 0
}

// MARK: - DefaultValueProvider

extension SecureNoteType: DefaultValueProvider {
static var defaultValue: SecureNoteType { .generic }
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions BitwardenShared/Core/Vault/Models/SecureNoteTypeTests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit 2a07971

Please sign in to comment.