🔥 [🐛] Firestore integer and floating point types all mapped to floating point

[benceg]

Issue

Any document field rules that assert is int contained within firestore.rules cause the following error when a write is attempted using JavaScript integer values for those fields (see example.ts below):

Error: [firestore/permission-denied] The caller does not have permission to execute the specified operation.

This error is resolved if is int is changed to is number. I assume this is due to numeric values of any kind serialising and being sent as float values. However, allowing floats for certain use cases (document order fields, for example) poses a security risk.

This does not happen when the web SDK is used.

---

Project Files

package.json:

"@react-native-firebase/firestore": "^10.4.1"

firestore.rules:

match /MyCollection/{doc} {
   allow create: if request.resource.data.order is int;
}

example.ts:

firestore().collection('MyCollection').doc().set({ order: 1 });

Environment

Click To Expand

react-native info output:

System:
    OS: macOS 11.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 28.47 GB / 64.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 12.14.0 - ~/.nvm/versions/node/v12.14.0/bin/node
    Yarn: 1.22.5 - ~/.yarn/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v12.14.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.0 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 14.3, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
    Android SDK:
      API Levels: 29, 30
      Build Tools: 28.0.3, 29.0.2, 30.0.0, 30.0.2
      System Images: android-30 | Google Play Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 4.0 AI-193.6911.18.40.6514223
    Xcode: 12.3/12C33 - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_272 - /usr/bin/javac
    Python: 2.7.16 - /usr/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: ^17.0.1 => 17.0.1
    react-native: ^0.63.4 => 0.63.4
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

• Platform that you're experiencing the issue on:
  • iOS
  • Android
  • [✔️] iOS but have not tested behavior on Android
  • Android but have not tested behavior on iOS
  • Both
• react-native-firebase version you're using that has this issue:
  • "@react-native-firebase/app": "^10.4.0"
• Firebase module(s) you're using that has the issue:
  • "@react-native-firebase/firestore": "^10.4.1"
• Are you using TypeScript?
  • Y & ^4.1.3

---

• 👉 Check out React Native Firebase and Invertase on Twitter for updates on the library.

[mikehardy]

Indeed. This sounds like something in the serializer related to all numbers in javascript being floating point.

Our test rules are here: https://github.com/invertase/react-native-firebase/blob/master/.github/workflows/scripts/firestore.rules

We collect issue-specific tests here: https://github.com/invertase/react-native-firebase/blob/master/packages/firestore/e2e/issues.e2e.js

I bet with a combination of a new rule and a test that exercised it we could pinpoint the failure (and have a test harness for future that makes sure it's fixed forever)

Judging by the code content of the firestore seralizer it only pays attention to "number" (which does conform to javascript the language, though perhaps not firebase-js-sdk)

react-native-firebase/packages/firestore/lib/utils/serialize.js

Line 24 in b4fb976

isNumber,

This has a corresponding entry in the iOS code

react-native-firebase/packages/firestore/ios/RNFBFirestore/RNFBFirestoreSerialize.m

Line 47 in b4fb976

INT_NUMBER,

And we do chuck a floating point (well, double) value in for them:

react-native-firebase/packages/firestore/ios/RNFBFirestore/RNFBFirestoreSerialize.m

Line 381 in b4fb976

return @([typeMap[1] doubleValue]);

(same on Android though you only mention checking iOS at the moment

react-native-firebase/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreSerialize.java

Line 287 in b4fb976

typeArray.pushDouble(((Integer) value).doubleValue());

)

Searching for 'integerValue' in here https://github.com/firebase/firebase-js-sdk/blob/master/packages/firestore/src/model/values.ts shows how work needs to be done to correctly separate the idea of integers from doubles for comparison etc on read

This is how it is handled when checking whether it is int or float:
https://github.com/firebase/firebase-js-sdk/blob/50abe6c4d455693ef6a3a3c1bc8ef6ab5b8bd9ea/packages/firestore/src/remote/number_serializer.ts#L56 / https://github.com/firebase/firebase-js-sdk/blob/50abe6c4d455693ef6a3a3c1bc8ef6ab5b8bd9ea/packages/firestore/src/util/types.ts#L44-L52

A quick way to try it might be to have (for android anyway) something like the solutions here https://stackoverflow.com/questions/5502548/checking-if-a-number-is-an-integer-in-java and if it's reasonable to assume the number that came through the react-native bridge to native code is a whole number, pushInteger instead of pushDouble here

react-native-firebase/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreSerialize.java

Line 287 in b4fb976

typeArray.pushDouble(((Integer) value).doubleValue());

and similar https://stackoverflow.com/questions/9612839/is-there-a-way-to-check-if-a-variable-is-a-whole-number-c for objective-c

You can try those native edits directly by reaching right in to node_modules. In general if firestore backend is supporting integers and doubles as separate data types and we are conflating them here as all IEEE floating point numbers, a more complete solution will require more thought

[mikehardy]

indeed firestore considers them separate first-class types. 🤔 https://firebase.google.com/docs/firestore/manage-data/data-types

[benceg]

Thanks for getting on this so quickly @mikehardy. Yep, Firestore's number is a superset of both int and float and reasonable assumptions about integers can be made in JavaScript using Number.isInteger.

The problem then becomes the inverse, as floats without decimals (e.g. 1.0) are treated as integers by JavaScript, so might not be parsed as a floating point for a float field when serialized out to Objective-C or Java, from what I understand. The web SDK just duck types its way around this.

[mikehardy]

Well, react-native-firebase has the stated goal of emulating the firebase-js-sdk. It may be acceptable to follow the exact same duck-typing style firebase-js-sdk does, perhaps with a backwards-compatibility toggle of "all double all the time like before"

[benceg]

It struck me that modulo provides an excellent workaround to this problem. Should anyone else have this issue, an interim solution would be to change your Firestore rule from:

if field is int

to the functionally identical but less type specific:

if field is number && field % 1 == 0

[mikehardy]

Since that is effectively what the javascript is doing in it's duck typing, that may in fact be the "most correct" solution, certainly the "smallest change" one! I think that's clever

[benceg]

Much obliged. Shall we continue to leave this open for the JS SDK parity goal?

[mikehardy]

We discussed this internally and I'm not sure what we can do but it was eye-opening. I do want to leave it open as at minimum we should document the difference, the implications and the way to still get a useful result e.g. in your firestore rules modulus trick

[stale]

Hello 👋, to help manage issues we automatically close stale issues.
This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs.
Thank you for your contributions.

[stale]

Closing this issue after a prolonged period of inactivity. If this is still present in the latest release, please feel free to create a new issue with up-to-date information.

[mikehardy]

This should not have closed, its still an issue - e.g. #5804

[mikehardy]

There still be some type inconsistencies but the most glaring one - that react-native-firebase cannot query docs via numbers using where with in operator is fixed we think

This is the patch that we think fixes it but we are not 100% sure how forwards-/backwards-compatible it is.
We would love testing feedback if you integrate this patch in your project:
#5840 (comment)

I have integrated it successfully in a project, and I've successfully made an e2e test here which probes the failure and demonstrates the patch fixes it, so I'm pretty confident it works, but it's a very subtle bug so I would love feedback

[mikehardy]

#5840 has been released and should fix this conclusively for iOS in v14+ here

[ninjz]

I seem to be encountering this issue as of v12.9.3.

I'm getting the error exactly as stated above:

Error: [firestore/permission-denied] The caller does not have permission to execute the specified operation.

Which is only resolved when changing my security rules from is int to is number.

cc: @mikehardy

[mikehardy]

I seem to be encountering this issue as of v12.9.3.

It is not reasonable to expect a version that old to even compile anymore. I can't think of any justification for still using it and I'd be scared to in my work phone projects, personally

If this still reproduces on current release it's an issue otherwise upgrade is best recommendation

[ninjz]

Thanks for the reply. You’re right I should update to the latest version. Sorry I didn’t realize I was on such an old version.