RTDB persistence layer triggers a bug with number parsing

[mortenbekditlevsen]

UPDATED After digging around a bit more, I think that I have gotten closer to the actual issue. My findings are added in the bottom.

[READ] Step 1: Are you in the right place?

Yup, most definitely.

[REQUIRED] Step 2: Describe your environment

• Xcode version: 10.3
• Firebase SDK version: 6.2.0 (also present in 6.10.0)
• Firebase Component: Database
• Component version: 6.2.0

[REQUIRED] Step 3: Describe the problem

From the user perspective we wrote the value 1000 to the RTDB and when we read it back in we got -844.

What actually happened is that we wrote the value 999.9999999999999487.
Reading back this value through the persistence layer triggers an issue with NSDecimalNumber conversion.

When disabling persistence and reading/writing directly to the RTDB, this problem does not occur.

I started a discussion about the issue on the Swift forums:
https://forums.swift.org/t/nsdecimalnumber-conversion-error/29862/3

And I was directed to previous descriptions of the issue. Apparently it has been around for a long time.
http://www.openradar.me/radar?id=5007005597040640

Steps to reproduce:

Use Firebase RTDB with persistence enabled.
Write an NSDecimalNumber with many decimals of precision.
Read it back and check that the returned value is completely bonkers.
Turn off persistence and repeat to verify that it only happens with persistence enabled.

Add the following code to a Swift project to test.
Run twice to trigger the error. Then disable persistence and run again to see that the error is gone:

import UIKit
import FirebaseDatabase
import Firebase

class ViewController: UIViewController {

    override func viewDidLoad() {
        super.viewDidLoad()

        let db = Database.database()
        db.isPersistenceEnabled = true
        let ref = db.reference(withPath: "a")

        let decimal = Decimal(_exponent: -16, _length: 4, _isNegative: 0, _isCompact: 0, _reserved: 0, _mantissa: (65023, 35303, 8964, 35527, 0, 0, 0, 0))

        _ = ref.observeSingleEvent(of: .value) { snap in
            print("SNAP", snap)
        }

        ref.setValue(decimal as NSDecimalNumber)
    }
}

Relevant Code:

The code that triggers the issue is in FLevelDBStorageEngine.m:

- (id)fixDoubleParsing:(id)value
    __attribute__((no_sanitize("float-cast-overflow"))) {
    // The parser for double values in JSONSerialization at the root takes some
    // short-cuts and delivers wrong results (wrong rounding) for some double
    // values, including 2.47. Because we use the exact bytes for hashing on the
    // server this will lead to hash mismatches. The parser of NSNumber seems to
    // be more in line with what the server expects, so we use that here
    if ([value isKindOfClass:[NSNumber class]]) {
        CFNumberType type = CFNumberGetType((CFNumberRef)value);
        if (type == kCFNumberDoubleType || type == kCFNumberFloatType) {
            // The NSJSON parser returns all numbers as double values, even
            // those that contain no exponent. To make sure that the String
            // conversion below doesn't unexpectedly reduce precision, we make
            // sure that our number is indeed not an integer.
            if ((double)(int64_t)[value doubleValue] != [value doubleValue]) {
                NSString *doubleString = [value stringValue];
                return [NSNumber numberWithDouble:[doubleString doubleValue]];
            } else {
                return [NSNumber numberWithLongLong:[value longLongValue]];
            }
        }
    }
    return value;
}

The value is checked to see if it can be represented as an integer. Since the doubleValue is 1000, it can be represented as an integer.
But then a new number is created using the longLongValue of the same number, and this triggers the error and gives -844.

Update

I think that the core of the issue is, that if I try to save a double value like 2.47 (which cannot be represented directly in a double), then the RTDB stores it precisely as 2.47. At least that is what I see when I retrieve the value again.
BUT: If I wrap the double in an NSDecimalNumber, then the double value is represented 'as is' with all decimals: 2.470000000000000512

So if the persistence layer tries to fix up any values when reading them, basically it has no knowledge about whether 2.47 or 2.470000000000000512 is how the RTDB actually stores the value. And as such I guess that it cannot be certain to calculate the same hashes or produce the same results when fetching data.

If those suspicions are correct, then any 'fixing' of the data needs to be performed when writing the data into the persisence layer - and not when reading out the values.

---

Another solution is to disallow serializing values of the NSDecimalNumber subclass, but the RTDB documentation states that you can write NSNumbers, and that of course includes NSDecimalNumbers...

[schmidt-sebastian]

I will close this for now as #4109 was merged. If we need to improve our parsing further to match the hash codes the backend uses, then we can open a follow-up issue.

[schmidt-sebastian]

@mortenbekditlevsen Thank you for your contribution.