Skip to content

FUIPhoneAuth crashes when entering invalid phone number #304

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

Closed
gsabran opened this issue Sep 22, 2017 · 11 comments · Fixed by firebase/FirebaseUI-iOS#339
Closed

FUIPhoneAuth crashes when entering invalid phone number #304

gsabran opened this issue Sep 22, 2017 · 11 comments · Fixed by firebase/FirebaseUI-iOS#339

Comments

@gsabran
Copy link

gsabran commented Sep 22, 2017

  • Xcode version: 9.0
  • Firebase SDK version: 4.2
  • Library versions:
- Firebase/Auth (4.2.0):
    - Firebase/Core
    - FirebaseAuth (= 4.2.0)
  - Firebase/Core (4.2.0):
    - FirebaseAnalytics (= 4.0.3)
    - FirebaseCore (= 4.0.7)
  - FirebaseAnalytics (4.0.3):
    - FirebaseCore (~> 4.0)
    - FirebaseInstanceID (~> 2.0)
    - GoogleToolboxForMac/NSData+zlib (~> 2.1)
    - nanopb (~> 0.3)
  - FirebaseAuth (4.2.0):
    - FirebaseAnalytics (~> 4.0)
    - GoogleToolboxForMac/NSDictionary+URLArguments (~> 2.1)
    - GTMSessionFetcher/Core (~> 1.1)
  - FirebaseCore (4.0.7):
    - GoogleToolboxForMac/NSData+zlib (~> 2.1)
    - nanopb (~> 0.3)
  - FirebaseInstanceID (2.0.3)
  - FirebaseUI/Auth (4.3.0):
    - Firebase/Auth (~> 4.2)
  - FirebaseUI/Phone (4.3.0):
    - FirebaseUI/Auth
  • Firebase Product: FUIPhoneAuth and FIRPhoneAuth

Steps to reproduce:

When entering an invalid phone number, like a 9 digit phone number for the US, the error callback is called out of the main thread

Relevant Code:

let auth = FUIAuth(uiWith: Auth.auth())!
let provider = FUIPhoneAuth(authUI: auth)
auth.providers = [provider]
auth.delegate = self
provider.signIn(withPresenting: self)

enter 123456789 as phone number, with US selected as country

This triggers a callback with an error that is in a background thread and tries to change the UI and crashes:

screen shot 2017-09-21 at 5 19 55 pm

and the log is:

[Assert] Cannot be called with asCopy = NO on non-main thread.
*** Assertion failure in void _UIPerformResizeOfTextViewForTextContainer(NSLayoutManager *, UIView<NSTextContainerView> *, NSTextContainer *, NSUInteger)(), /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIFoundation/UIFoundation-543/UIFoundation/TextSystem/NSLayoutManager_Private.m:1619
*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Only run on the main thread!'
@XiangtianDai
Copy link
Contributor

This has already been fixed in https://github.com/firebase/firebase-ios-sdk/pull/298/files but the fix didn't make into the release.

@gsabran
Copy link
Author

gsabran commented Sep 22, 2017

Great, thanks. Is there an easy way to copy those changes in my pods? They seem to only include frameworks.

@sidjdev
Copy link

sidjdev commented Sep 27, 2017

Hi there. I am facing the same issue. Xcode 9, iOS 11

  • Firebase, 4.2.0
    - FirebaseAnalytics, 4.0.3
    - FirebaseAuth, 4.2.0
    - FirebaseCore, 4.0.7
    - FirebaseInstanceID, 2.0.3
    - FirebaseStorage, 2.0.2
    - FirebaseUI, 4.3.0
    How can I get it to work

@paulb777
Copy link
Member

The fix is planned for our next release. I'll update this issue when the release is available.

@paulb777 paulb777 reopened this Sep 27, 2017
@gsabran
Copy link
Author

gsabran commented Sep 27, 2017

Hi @paulb777, great to hear! Do you have a sense of the timeline to expect? Also has a test been added to the PR done by @XiangtianDai ? I had to ship with this crash since the deadline to migrate from Digit is end of September :(

@XiangtianDai
Copy link
Contributor

@gsabran Since you're using FUIPhoneAuth, the workaround firebase/FirebaseUI-iOS#339 has already been merged by @yramanchuk , who can ping the thread when this is released as part of FirebaseUI.

@XiangtianDai
Copy link
Contributor

@gsabran FirebaseUI 4.3.1 that conatins the fix has just been released and on cocoapods too.

@paulb777
Copy link
Member

Re-closing here. I conflated the FirebaseAuthUI with a FirebaseAuth issue.

@XiangtianDai
Copy link
Contributor

I think it is a FirebaseAuth issue, just that FirebaseAuthUI had implemented a workaround before our fix is shipped as their release schedule is more flexible.

@paulb777
Copy link
Member

OK, I'll open again and update when the next Auth release goes out.

@paulb777 paulb777 reopened this Sep 27, 2017
@paulb777
Copy link
Member

paulb777 commented Oct 3, 2017

Fix released today in Firebase 4.3.0 and FirebaseAuth 4.2.1

@paulb777 paulb777 closed this as completed Oct 3, 2017
christibbs added a commit that referenced this issue Jan 25, 2019
* compile and run

* unit tests pass and swift target compiles

* dump build version in activity log

* standardize cross sdk imports in firebase iam code (#211)

standardize cross sdk imports & polish with style script

* using new sample app for testing non-development fiam sdk (#213)

* in-app messaging public interface cleanup (#216)

* slim down public header files

* move testing source files to their correct folders

* compiles

* style script fine-tune

* adding root object creation methods to follow the firebse iOS convension

* IAM url following refactor (#218)

* @available not supported by blaze build yet (#220)

* Clearcut integration for in-app messaging (#222)

* Update iam-master to catch up with latest master (#223)

* Support multiple triggers defined for the same iam message (#224)

* FIAM Clearcut Client Implmentation Improvement (#226)

* fixing action url bug and set auto dismiss to be 12 seconds (#227)

* use official public api dns name (#233)

* Logging firebase analytics events for fiam (#232)

* Fine tuning of fiam's code for loading data from caches (#234)

* fiam ui fine tuning before EAP (#235)

* allow fiam SDK functions to be disabled/enabled dynamically (#247)

* analytics event parameter name tuning (#253)

* migrate off @import for fiam code (#261)

* different modes for fiam sdk at runtime (#262)

* realtime clearcut when in running in simulator (#265)

* Update interface based on the Firebase API review feedback  (#267)

* check existence of fiam SDK resource bundle (#268)

* handling api keys with application restrictions (#269)

* fixing typo and refoctor for unit testing (#270)

* fix color method name clash and layout adjust (#271)

* Yongmao/layout issue and class rename (#273)

* fix long text height calc error

* rename class

* fiam modal view layout tuning (#274)

* test on device for ios client (#275)

* dynamic testing mode handling (#277)

* main header file comment update and run style.sh (#281)

* honor global firebase auto data collection flag in fiam (#283)

* honor global firebase auto data collection flag in fiam

* address comments

* tuning

* address review comments

* tuning for comments

* Rename Core global data collection switch (#287)

* Support displaying a messages defined as recurring (#286)

* continue

* Adjust some inapp messaging dependeces due to the firebase core changes from the upcoming release

* bump up version for in-app messaging

* Fix impression log clearing race condition bug. (#289)

* fix impression handling race condition bug

* podspec and interacting interfaces for new firebaseinappmessagingdisplay pod (#290)

* create inapp messaging display podspec

* Default fiam display UI implementation (#292)

* Default fiam display UI implementation. Created the new pod for FirebaseInAppMessagingDisplay

* Make FirebaseInAppMessaging headless (#293)

* import path tuning

* log test message events (#296)

* respect fetch wait time from server (#297)

* add changelog.md for fiam headless sdk (#300)

* port the fetch wait time change back into github (#304)

* remove resource bundle check for headless sdk

* fix a bug in which the test-on-device message does not follow action url

* Add category on NSString to interlace two strings, use it to obfuscate clearcut host name

* Use FIR prefix to avoid collisions

* Change method name for getting server hostname

* Delete Swift example project, its coverage is already duplicated in the FIRInAppMessagingDisplay UITests

* Miscellaneous open sourcing cleanup tasks

* Convert FIAM to use the Analytics connector (#336)

* Convert FIAM to use Analytics connector along with converting to the FIRLibrary component registration model.
* Update interop headers for missing methods
* Convert existing unit tests

* Fix style for fiam sources (#342)

* Clean up non-idiomatic ObjC method names

* Clean up Swift test target from Podfile

* Use dummy GoogleService-Info.plist and don't expose api keys (#343)

* Update plist to include fake API keys

* Remove changes in non fiam code from iam-master branch (#350)

* Fix a few method and parameter naming nits

* Fix comment typos in several files

* Delete remaining Swift project files
@firebase firebase locked and limited conversation to collaborators Nov 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants