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

Crash when trying signInWithEmailAndPassword with empty string #127

Closed
ohtangza opened this issue May 26, 2017 · 11 comments
Closed

Crash when trying signInWithEmailAndPassword with empty string #127

ohtangza opened this issue May 26, 2017 · 11 comments
Assignees
Labels
Type: Firebase Issue or PR that addresses external Firebase behaviour issues or changes, e.g. a Firebase SDK issue

Comments

@ohtangza
Copy link

In Firebase Web SDK, it provides error response instead of an exception.
(We encountered this error after migrating from Web SDK to this react-native-firebase.)

I think it is better it works same as the official SDK. Any thoughts?

@Salakar
Copy link
Member

Salakar commented May 26, 2017

Thanks for the report,

  • Can you explain what you mean by empty string, empty email, empty password?
  • What's the behaviour of the web sdk specifically that is not happening with RNFirebase?
  • Can you provide screenshots/code examples please of expected behaviour vs what's not expected?

@ohtangza
Copy link
Author

ohtangza commented May 26, 2017

Oh, further investigate this issue, and this issue is specific to Android. iOS works as the web SDK by outputting a message ".The email address is badly formatted."

  • empty string/email/password is just empty string ''.
  • The following is the stack trace from calling firebase.auth().signInWithEmailAndPassword('', 'test') on Android.
java.lang.IllegalArgumentException Given String is empty or null 
    Unknown:-1 com.google.android.gms.common.internal.zzac.zzdr
    Unknown:-1 com.google.firebase.auth.FirebaseAuth.signInWithEmailAndPassword
    RNFirebaseAuth.java:177 io.invertase.firebase.auth.RNFirebaseAuth.signInWithEmailAndPassword
    Method.java:-2 java.lang.reflect.Method.invoke
    BaseJavaModule.java:345 com.facebook.react.bridge.BaseJavaModule$JavaMethod.invoke
    JavaModuleWrapper.java:141 com.facebook.react.cxxbridge.JavaModuleWrapper.invoke
    NativeRunnable.java:-2 com.facebook.react.bridge.queue.NativeRunnable.run
    Handler.java:751 android.os.Handler.handleCallback
    Handler.java:95 android.os.Handler.dispatchMessage
    MessageQueueThreadHandler.java:31 com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage
    Looper.java:154 android.os.Looper.loop
    MessageQueueThreadImpl.java:196 com.facebook.react.bridge.queue.MessageQueueThreadImpl$3.run
    Thread.java:761 java.lang.Thread.run

If you think this is a problem, I can also investigate further investigate this issue :)

@Salakar
Copy link
Member

Salakar commented May 26, 2017

Ok I see what you mean, looks like this is an inconsistency between the official firebase iOS and Android SDK's.

As you can see from the below screenshots - ios when the email is empty correctly goes into the completion block. Android however throws an exception without event getting into the OnFailureListener - it errors on the line mAuth.signInWithEmailAndPassword(email, password) when it should instead complete as a failure without throwing an exception.

I'd suggest raising a support request with firebase and raise this inconsistency issue please - I'm sure they've had enough of me raising the issues =] You can use this issue and this comment in it as a reference. By the looks of it it's not just limited to this method on android either - I'd imagine methods such as createUserWithEmailAndPassword would be affected also.

iOS

image

Android

image

Let me know how the support request goes and if they plan on resolving this.

Thanks

@Salakar Salakar added the Type: Firebase Issue or PR that addresses external Firebase behaviour issues or changes, e.g. a Firebase SDK issue label May 26, 2017
@ohtangza
Copy link
Author

@Salakar I reported this issue with a detailed message to Firebase support team through "Bug Report or Feature Request" channel. Keep you updated if there is a response! Thanks for your quick response.

@ohtangza
Copy link
Author

I am sharing here the response from Firebase team. In short, it was correct that the SDK was designed inconsistently. I will keep updating if there is any other news.


Hi Jeungmin,

My apologies for not getting back to you sooner.

I tried to replicate the issue on our Firebase Android SDK, and I did encountered the same error. I have informed our engineers regarding this one and I'll update you once I have more information to share.

Sincerely,
Hazel

@ohtangza
Copy link
Author

ohtangza commented Jun 2, 2017

I finally got the decision, which turned out that this issue should be handled RN wrapper level for the time being. Given that the motto of this wrapper is to mimic the behavior of the official web SDK, I think Android should prevent from crashing by this library. Any thoughts on this?


"I have talked to our engineers. Here's our perspective regarding this one, the behavior that you're seeing was an explicit design choice made by the FirebaseAuth team. In general, these types of errors are ones that the developer such as yourself should be explicitly handling in all cases, so we fail proactively to let you know during development. As you mentioned, it's possible for you to resolve this at the RN wrapper level."

@chrisbianca
Copy link
Contributor

@ohtangza If it was an explicit design choice made by the FirebaseAuth team, then why does the behaviour differ between iOS and Android? That's not aimed at you, but at them!

I'm loathe to introduce platform specific handling because the SDKs throw an internal error rather than deal with the behaviour correctly.

Can I suggest that you add some protection in your app to prevent a user being able to submit an empty username or password? This is a pretty standard user experience for any app that I've ever used, and certainly is what I'm doing with my apps...

@Sparragus
Copy link
Contributor

Ugh. So sad that was FirebaseAuth's response.

@ohtangza Check out validator. It has a ton of validation functions you can use, including validator.isEmail and validator.isEmpty.

@Ehesp Ehesp closed this as completed Jun 5, 2017
@edlovejoy
Copy link

i had same issue and i did a temp fix where i went to the activity_login and add
android:text="email" plus the hint too. so the user have to delete this email before they put
in the new one..so if the login without any input then they will get the regular login error instead of
a crash.. because the text is not making the it empty any more.. but i will look into it further with a proper way to validate .

@ChkBuk
Copy link

ChkBuk commented Apr 5, 2021

I finally got the decision, which turned out that this issue should be handled RN wrapper level for the time being. Given that the motto of this wrapper is to mimic the behavior of the official web SDK, I think Android should prevent from crashing by this library. Any thoughts on this?

"I have talked to our engineers. Here's our perspective regarding this one, the behavior that you're seeing was an explicit design choice made by the FirebaseAuth team. In general, these types of errors are ones that the developer such as yourself should be explicitly handling in all cases, so we fail proactively to let you know during development. As you mentioned, it's possible for you to resolve this at the RN wrapper level."

Hi @ohtangza,
Have you got any feedback from firebase?

@mikehardy
Copy link
Collaborator

@ChkBuk this issue has been closed for almost 4 years and the feedback was in there.
I'm not sure we still crash on this at all actually, but if we do, a PR would be okay to prevent the crash.
There will be no more activity on this closed issue though :-)

@invertase invertase locked as resolved and limited conversation to collaborators Apr 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Firebase Issue or PR that addresses external Firebase behaviour issues or changes, e.g. a Firebase SDK issue
Projects
None yet
Development

No branches or pull requests

8 participants