Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only unbox NSNumbers 0, 1 to Bool for objCType 'c' #181

Merged
merged 1 commit into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions Sources/Turf/JSON.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,42 @@ extension JSONValue: RawRepresentable {

public init?(rawValue: Any) {
// Like `JSONSerialization.jsonObject(with:options:)` with `JSONSerialization.ReadingOptions.fragmentsAllowed` specified.
if let bool = rawValue as? Bool {
self = .boolean(bool)
} else if let string = rawValue as? String {
if let string = rawValue as? String {
self = .string(string)
} else if let number = rawValue as? NSNumber {
self = .number(number.doubleValue)
/// When a Swift Bool or Objective-C BOOL is boxed with NSNumber, the value of the
/// resulting NSNumber's objCType property is 'c' (Int8 (aka CChar) in Swift, char in
/// Objective-C) and the value is 0 for false/NO and 1 for true/YES.
///
/// Strictly speaking, an NSNumber with those characteristics can be created by boxing
/// other non-boolean values (e.g. boxing 0 or 1 using the `init(value: CChar)`
/// initializer). Moreover, NSNumber doesn't guarantee to preserve the type suggested
/// by the initializer that's used to create it.
///
/// This means that when these values are encountered, it is ambiguous whether to
/// decode to JSONValue.number or JSONValue.boolean.
///
/// In practice, choosing .boolean yields the desired result more often since it is more
/// common to work with Bool than it is Int8.
switch String(cString: number.objCType) {
case "c": // char
Copy link
Contributor

@1ec5 1ec5 Feb 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with mapbox/mapbox-gl-native#6049 (comment). so I’m confident in this fix.

if number.int8Value == 0 {
self = .boolean(false)
} else if number.int8Value == 1 {
self = .boolean(true)
} else {
self = .number(number.doubleValue)
}
default:
self = .number(number.doubleValue)
}
} else if let boolean = rawValue as? Bool {
/// This branch must happen after the `NSNumber` branch
/// to avoid converting `NSNumber` instances with values
/// 0 and 1 but of objCType != 'c' to `Bool` since `as? Bool`
/// can succeed when the NSNumber's value is 0 or 1 even
/// when its objCType is not 'c'.
self = .boolean(boolean)
} else if let rawArray = rawValue as? JSONArray.RawValue,
let array = JSONArray(rawValue: rawArray) {
self = .array(array)
Expand Down
12 changes: 12 additions & 0 deletions Tests/TurfTests/JSONTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ class JSONTests: XCTestCase {
XCTAssertEqual(JSONValue(rawValue: 3.1415 as NSNumber), .number(3.1415))
XCTAssertEqual(JSONValue(rawValue: false as NSNumber), .boolean(false))
XCTAssertEqual(JSONValue(rawValue: true as NSNumber), .boolean(true))
XCTAssertEqual(JSONValue(rawValue: false), .boolean(false))
XCTAssertEqual(JSONValue(rawValue: true), .boolean(true))
Comment on lines +11 to +12
Copy link
Contributor Author

@macdrevx macdrevx Feb 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether supporting passing unboxed Bool was intentional @1ec5, but the old implementation did allow it, so I kept the behavior and added tests to verify it.

XCTAssertEqual(JSONValue(rawValue: 0 as NSNumber), .number(0))
XCTAssertEqual(JSONValue(rawValue: 1 as NSNumber), .number(1))
XCTAssertEqual(JSONValue(rawValue: ["Jason", 42, 3.1415, false, true, nil, [], [:]] as NSArray),
.array(["Jason", 42, 3.1415, false, true, nil, [], [:]]))
XCTAssertEqual(JSONValue(rawValue: [
Expand Down Expand Up @@ -245,5 +249,13 @@ class JSONTests: XCTestCase {
XCTAssertEqual(decodedValue?.rawValue as? NSDictionary, rawObject as NSDictionary)

XCTAssertNoThrow(try JSONEncoder().encode(decodedValue))

// check decoding of 0/1 true/false to ensure unwanted conversions are avoided
let rawString = "[0, 1, true, false]"
// force-unwrap is safe since we control the input
let serializedArrayFromString = rawString.data(using: .utf8)!
XCTAssertNoThrow(decodedValue = try JSONDecoder().decode(JSONValue.self, from: serializedArrayFromString))
XCTAssertNotNil(decodedValue)
XCTAssertEqual(.array([.number(0), .number(1), .boolean(true), .boolean(false)]), decodedValue)
}
}