React-native-firebase: 🔥 Crash on Android with the realtime database module when using the set or push methods with a value containing invalid keys.

Created on 13 Jul 2020 · 12Comments · Source: invertase/react-native-firebase

Issue

When an invalid key is passed to one of these methods, the application immediately crashes, and the exception cannot be caught from the React side.
This is the error message we see in the adb logcat:

07-13 21:57:28.827 29915 30137 E AndroidRuntime: FATAL EXCEPTION: pool-26-thread-1
07-13 21:57:28.827 29915 30137 E AndroidRuntime: Process: com.accopilot, PID: 29915
07-13 21:57:28.827 29915 30137 E AndroidRuntime: java.lang.ClassCastException: com.google.firebase.database.DatabaseException cannot be cast to io.invertase.firebase.database.UniversalDatabaseException
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at io.invertase.firebase.database.ReactNativeFirebaseDatabaseCommon.rejectPromiseDatabaseException(ReactNativeFirebaseDatabaseCommon.java:39)
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at io.invertase.firebase.database.ReactNativeFirebaseDatabaseReferenceModule.lambda$set$2(ReactNativeFirebaseDatabaseReferenceModule.java:50)
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at io.invertase.firebase.database.-$$Lambda$ReactNativeFirebaseDatabaseReferenceModule$lLWLBGxgO9baVIkrTuyqZw2iHic.onComplete(Unknown Source:2)
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at com.google.android.gms.tasks.zzj.run(Unknown Source:4)
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
07-13 21:57:28.827 29915 30137 E AndroidRuntime:        at java.lang.Thread.run(Thread.java:764)

I found the source of the bug from within the realtime database module. I was setting an object with an invalid key, which in turn resulted in the following DatabaseException:

com.google.firebase.database.DatabaseException: Invalid key: my/custom/path. Keys must not contain '/', '.', '#', '$', '[', or ']'
        at com.google.firebase.database.core.utilities.Validation.validateWritableKey(Validation.java:120)
        at com.google.firebase.database.core.utilities.Validation.validateWritableObject(Validation.java:103)
        at com.google.firebase.database.core.utilities.Validation.validateWritableObject(Validation.java:104)
        at com.google.firebase.database.core.utilities.Validation.validateWritableObject(Validation.java:104)
        at com.google.firebase.database.DatabaseReference.setValueInternal(DatabaseReference.java:283)
        at com.google.firebase.database.DatabaseReference.setValue(DatabaseReference.java:237)
        at io.invertase.firebase.database.UniversalFirebaseDatabaseReferenceModule.set(UniversalFirebaseDatabaseReferenceModule.java:41)
        at io.invertase.firebase.database.ReactNativeFirebaseDatabaseReferenceModule.lambda$set$1$ReactNativeFirebaseDatabaseReferenceModule(ReactNativeFirebaseDatabaseReferenceModule.java:45)
        at io.invertase.firebase.database.-$$Lambda$ReactNativeFirebaseDatabaseReferenceModule$mi4WNYuhnr6UpE26LL1it2iLAjg.then(Unknown Source:8)
        at com.google.android.gms.tasks.zzp.run(Unknown Source:2)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7179)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:975)

This DatabaseException was then passed to ReactNativeFirebaseDatabaseCommon.rejectPromiseDatabaseException which tried to cast it into UniversalDatabaseException:
https://github.com/invertase/react-native-firebase/blob/554c4b403520f537c10426ce377e32dd87d41469/packages/database/android/src/reactnative/java/io/invertase/firebase/database/ReactNativeFirebaseDatabaseCommon.java#L39
Which causes the crash: java.lang.ClassCastException: com.google.firebase.database.DatabaseException cannot be cast to io.invertase.firebase.database.UniversalDatabaseException

Project Files

iOS

Click To Expand

#### `ios/Podfile`: - [x] I'm not using Pods - [ ] I'm using Pods and my Podfile looks like:

# N/A

#### `AppDelegate.m`:

// N/A

Android

Click To Expand

#### `android/build.gradle`:

// N/A

#### `android/app/build.gradle`:

// N/A

#### `android/settings.gradle`:

// N/A

#### `MainApplication.java`:

// N/A

#### `AndroidManifest.xml`:

<!-- N/A -->

Environment

Click To Expand

**`react-native info` output:**

System:
    OS: Linux 5.3 Ubuntu 18.04.4 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
    Memory: 722.18 MB / 15.49 GB
    Shell: 4.4.20 - /bin/bash
  Binaries:
    Node: 10.21.0 - /usr/bin/node
    npm: 6.14.6 - /usr/bin/npm
  npmPackages:
    react: 16.9.0 => 16.9.0 
    react-native: 0.61.5 => 0.61.5

- **Platform that you're experiencing the issue on**: - [ ] iOS - [ ] Android - [ ] **iOS** but have not tested behavior on Android - [x] **Android** but have not tested behavior on iOS - [ ] Both - **`Firebase` module(s) you're using that has the issue:** - realtime-database 7.3.2. Issue was also happening is 6.7.1 - **Are you using `TypeScript`?** - N

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

Crash Database >= 6

Source

mickael-h

Most helpful comment

@mikehardy Sorry I missed these, I can add to my list for investigation!

dackers86 on 4 Aug 2020

❤2

All 12 comments

That is pretty strange, I'm not sure how a DatabaseException is getting in there, when the code you reference is definitely converting to the correct type here:

https://github.com/invertase/react-native-firebase/blob/554c4b403520f537c10426ce377e32dd87d41469/packages/database/android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseReferenceModule.java#L44

That's a very general error / exception pathway that all code goes through, and the pathway is exercised in testing AFAICS

https://github.com/invertase/react-native-firebase/blob/master/packages/database/e2e/reference/set.e2e.js

:thinking:

mikehardy on 13 Jul 2020

All I can recommend as an unsatisfying but stops-the-crashing workaround is to go into node_modules, hack the line that has the ClassCastException to be inside a try/catch(Exception) java safety block, and in the catch log the important information like the offending exception message and stack, then call the promise reject method with a code and message that you create in order to flag this situation - I think they can be whatever you like

With the extra telemetry from that you / we may be able to figure out the real root cause

(patch-package is the tool I'd use to persist this change by the way - if you've never used it, it is an amazing tool for cases exactly like this where you need changes to packages but you probably won't want them forever)

mikehardy on 13 Jul 2020

I found the source of the bug. I was setting an object with an invalid key, which in turn resulted in the following DatabaseException:

com.google.firebase.database.DatabaseException: Invalid key: my/custom/path. Keys must not contain '/', '.', '#', '$', '[', or ']'
        at com.google.firebase.database.core.utilities.Validation.validateWritableKey(Validation.java:120)
        at com.google.firebase.database.core.utilities.Validation.validateWritableObject(Validation.java:103)
        at com.google.firebase.database.core.utilities.Validation.validateWritableObject(Validation.java:104)
        at com.google.firebase.database.core.utilities.Validation.validateWritableObject(Validation.java:104)
        at com.google.firebase.database.DatabaseReference.setValueInternal(DatabaseReference.java:283)
        at com.google.firebase.database.DatabaseReference.setValue(DatabaseReference.java:237)
        at io.invertase.firebase.database.UniversalFirebaseDatabaseReferenceModule.set(UniversalFirebaseDatabaseReferenceModule.java:41)
        at io.invertase.firebase.database.ReactNativeFirebaseDatabaseReferenceModule.lambda$set$1$ReactNativeFirebaseDatabaseReferenceModule(ReactNativeFirebaseDatabaseReferenceModule.java:45)
        at io.invertase.firebase.database.-$$Lambda$ReactNativeFirebaseDatabaseReferenceModule$mi4WNYuhnr6UpE26LL1it2iLAjg.then(Unknown Source:8)
        at com.google.android.gms.tasks.zzp.run(Unknown Source:2)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7179)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:975)

mickael-h on 15 Jul 2020

@mickael-h please allow me to active listen to make sure I understand:

you are no longer blocked as you have likely stopped sending in invalid keys (if so, excellent!)
the module is still doing something incorrect when handling incorrect keys though, as crashes should not be possible, and you could still crash it at will if sent in incorrect keys

mikehardy on 15 Jul 2020

Hey @dackers86 I saw you just made an e2e test for database, I wonder if this crash would be easy to trigger, knowing it happens with an invalid key?

mikehardy on 15 Jul 2020

@mickael-h please allow me to active listen to make sure I understand:

you are no longer blocked as you have likely stopped sending in invalid keys (if so, excellent!)

the module is still doing something incorrect when handling incorrect keys though, as crashes should not be possible, and you could still crash it at will if sent in incorrect keys

Both of these statements are correct, yes.
Even though the problem originated from our code, there is still an error handling related issue.
That's why I haven't closed the issue, but I'll edit it as soon as I'm finished with today's work.

mickael-h on 15 Jul 2020

👍1

I just ran into this same issue. The reason this leads to a crash instead of propagating the exception to the JS side is: the exception is thrown by Firebase's DatabaseReference.updateChildren(), not passed to its DatabaseReference.CompletionListener callback.

The code in UniversalFirebaseDatabaseReferenceModule.update() that wraps the exception in a UniversalDatabaseException is only checking for errors passed to the callback—it doesn't try to catch thrown DatabaseExceptions. So the thrown exception bubbles up and fails the outer Task in ReactNativeFirebaseDatabaseReferenceModule.update(), which expects it to be an instance of UniversalDatabaseException.

It looks like the Firebase Android codebase throws client-side validation errors, and server-side errors get passed to the callback. I took a look at the Firebase JS codebase and it _looks_ like that's the case there too. (I haven't actually tried running it yet.)

I think if RNFirebase needs to throw the validation Errors synchronously, that means the validation will need to happen in the JS code, not native?

justinwh on 3 Aug 2020

That's valuable extra context - I think this is a deeper question that would need @dackers86 (in the area) or @Salakar thought if they have time to judge whether to actually throw or just propagate the errors

mikehardy on 3 Aug 2020

@mikehardy Sorry I missed these, I can add to my list for investigation!

dackers86 on 4 Aug 2020

❤2

@mickael-h @mikehardy

Is there an existing example of what code is being called on the JS side?

I'll try and replicate this through our e2e tests...

dackers86 on 12 Aug 2020

@dackers86 from the looks of this you could just set an invalid character into the ref you pass and it'll surface the error handling snafu per description and comment this was passed in: