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

Invalid cast in GTMSessionFetcherService #190

Open
vladfilyakov opened this issue Mar 25, 2020 · 23 comments
Open

Invalid cast in GTMSessionFetcherService #190

vladfilyakov opened this issue Mar 25, 2020 · 23 comments

Comments

@vladfilyakov
Copy link

vladfilyakov commented Mar 25, 2020

Hello

This issue was described in several tickets but solution is still not there.

There are several third-party libraries that swizzle URLSession's delegate with their own proxy objects. For example TrustKit and SplunkMint do that. However GTMSessionFetcherService completely ignores this fact and hard-casts these proxy objects to its own class - GTMSessionFetcherSessionDelegateDispatcher:

return (GTMSessionFetcherSessionDelegateDispatcher *)fetcherDelegate;

    BOOL hasDispatcher = (fetcherDelegate != nil &&
                          ![fetcherDelegate isKindOfClass:[GTMSessionFetcher class]]);
    if (hasDispatcher) {
      GTMSESSION_ASSERT_DEBUG([fetcherDelegate isKindOfClass:[GTMSessionFetcherSessionDelegateDispatcher class]],
                              @"Fetcher delegate class: %@", [fetcherDelegate class]);
      return (GTMSessionFetcherSessionDelegateDispatcher *)fetcherDelegate;
    }

Obviously later we get crashes in different places due to "unrecognized selector sent to instance" exceptions.

The "dumb" solution is to check the class of object before casting it:

      if ([fetcherDelegate isKindOfClass:[GTMSessionFetcherSessionDelegateDispatcher class]]) {
        return (GTMSessionFetcherSessionDelegateDispatcher *)fetcherDelegate;
      }
      else {
        return nil;
      }

But having almost zero knowledge of how GTMSessionFetcher works, it's hard for me to understand if this code breaks something else. Any advice from the team?

We consume this library indirectly via FirebaseAuth.

Thanks,
Vlad

@thomasvl
Copy link
Member

This sounds like what also came up in #130, because of swizzling, these things in changing the internals of the library, but not doing it in safe ways. We landed on workaround to try an address this, but from looking at this last time, the other SDKs that are doing this should also be properly forwarding selectors when the hook things like this. The delegate in question here should be an internal detail, and if they are swapping it out without ensure everything is forwarded correctly, nothing says the library will work correctly.

@vladfilyakov
Copy link
Author

From my understanding, this library expects that the delegate for the common URLSession is never changed by anyone else. I think this is too much to expect. The only guaranteed thing here is that the delegate will implement the required methods of the delegate protocol and nothing more. This library expects the delegate to be the same internal object that was assigned there by the library. This is wrong thing to assume, in my opinion.

Other libraries, that do swizzling, do forward method calls for delegate, but they cannot and should not implement methods from GTMSessionFetcherSessionDelegateDispatcher so that this library does not crash, right?

The correct solution might be to store GTMSessionFetcherSessionDelegateDispatcher instance separately from URLSession.delegate and don't expect this delegate to do more than what is defined in its protocol type.

So returning nil when type of delegate is not what is expected isn't correct? Does it break other things?

Please correct me if I misunderstand the code of the library.

@mwyman
Copy link
Contributor

mwyman commented Mar 25, 2020

Swizzling is almost definitionally unsafe, and I don't think it's reasonable to expect libraries to bend over backward to support other developers swizzling anything and everything out from under them.

In a case like this, when code swizzles out the delegate from a system object, it should be certain to forward any unknown methods to the original delegate. It's not a matter of implementing those methods, but rather making sure to forward unknown methods, for which Objective-C has built-in facilities for. This would be an issue of forward-safety, too—if a new SDK includes delegate methods, and the swizzling library doesn't forward them correctly but is also not updated, the relevant methods will not be called.

@vladfilyakov
Copy link
Author

I see your point. I agree that object that does swizzling should forward all unknown methods back to original object. Here is TrustKit code, for example:

https://github.com/datatheorem/TrustKit/blob/master/TrustKit/Swizzling/TSKNSURLSessionDelegateProxy.m

It seems they are doing all that and it still somehow crashes. Do you see any problems with their code?

What about the fix I proposed? Will it cause additional problems if GTMSessionFetcherService.delegateDispatcherForFetcher would return nil if session.delegate is not of type GTMSessionFetcherSessionDelegateDispatcher?

We cannot get rid of TrustKit or SplunkMint and need to use FirebaseAuth, so need some solution. Any advice?

@vladfilyakov
Copy link
Author

Some more information after more debugging:

  1. TrustKit wraps URLSession's delegate into its TSKNSURLSessionDelegateProxy object via swizzling of URLSession's static method used for session creation. This proxy class stores the original delegate and forwards unrecognized messages to it.

  2. However the original delegate is of type GTMSessionFetcher, but GTMSessionFetcherService casts TrustKit's proxy object to GTMSessionFetcherSessionDelegateDispatcher and calls setFetcher:forTask: on it. The problem is that even so the proxy object forwards this call to the original delegate that delegate is GTMSessionFetcher and cannot handle this method:

(lldb) p [GTMSessionFetcherSessionDelegateDispatcher instancesRespondToSelector:@selector(setFetcher:forTask:)]
(BOOL) $14 = YES
(lldb) p [GTMSessionFetcher instancesRespondToSelector:@selector(setFetcher:forTask:)]
(BOOL) $15 = NO

The result:

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[GTMSessionFetcher setFetcher:forTask:]: unrecognized selector sent to instance 0x7f9d6b641800'
*** First throw call stack:
(
	0   CoreFoundation                      0x00007fff23b98bde __exceptionPreprocess + 350
	1   libobjc.A.dylib                     0x00007fff503b5b20 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff23bb9704 -[NSObject(NSObject) doesNotRecognizeSelector:] + 132
	3   CoreFoundation                      0x00007fff23b9d7bc ___forwarding___ + 1436
	4   CoreFoundation                      0x00007fff23b9f6c8 _CF_forwarding_prep_0 + 120
	5   PayPal                              0x0000000108560215 -[GTMSessionFetcherService fetcherDidBeginFetching:] + 421
	6   PayPal                              0x000000010853e3f2 -[GTMSessionFetcher beginFetchMayDelay:mayAuthorize:] + 10834
	7   PayPal                              0x000000010853a9d4 -[GTMSessionFetcher beginFetchWithCompletionHandler:] + 340
	8   PayPal                              0x00000001083fef38 -[FIRAuthBackendRPCIssuerImplementation asyncPostToURLWithRequestConfiguration:URL:body:contentType:completionHandler:] + 1016
	9   PayPal                              0x0000000108401ad6 -[FIRAuthBackendRPCImplementation postWithRequest:response:callback:] + 1094
	10  PayPal                              0x00000001083ffce5 -[FIRAuthBackendRPCImplementation verifyCustomToken:callback:] + 277
	11  PayPal                              0x00000001083fe15b +[FIRAuthBackend verifyCustomToken:callback:] + 123
	12  PayPal                              0x00000001083f6a42 -[FIRAuth internalSignInAndRetrieveDataWithCustomToken:completion:] + 274
	13  PayPal                              0x00000001083f0fca __44-[FIRAuth signInWithCustomToken:completion:]_block_invoke + 186
	14  libdispatch.dylib                   0x00007fff511fb888 _dispatch_call_block_and_release + 12
	15  libdispatch.dylib                   0x00007fff511fc7f9 _dispatch_client_callout + 8
	16  libdispatch.dylib                   0x00007fff51202566 _dispatch_lane_serial_drain + 707
	17  libdispatch.dylib                   0x00007fff51202f9c _dispatch_lane_invoke + 388
	18  libdispatch.dylib                   0x00007fff5120d06c _dispatch_workloop_worker_thread + 626
	19  libsystem_pthread.dylib             0x00007fff5141c611 _pthread_wqthread + 421
	20  libsystem_pthread.dylib             0x00007fff5141c3fd start_wqthread + 13
)
libc++abi.dylib: terminating with uncaught exception of type NSException

Note the class of the object where the selector was sent/forwarded to: -[GTMSessionFetcher setFetcher:forTask:].

Thoughts?

@vladfilyakov
Copy link
Author

Do you expect proxy class (TSKNSURLSessionDelegateProxy) to also implement isKindOfClass: to present itself as GTMSessionFetcher subclass?

- (BOOL)isKindOfClass:(Class)aClass
{
  return [super isKindOfClass:aClass] || [_originalDelegate isKindOfClass:aClass];
}

Where _originalDelegate is GTMSessionFetcher.

@vladfilyakov
Copy link
Author

@thomasvl @mwyman Any thoughts, advices?

@vladfilyakov
Copy link
Author

More details:

FIRAuthBackendRPCIssuerImplementation in FirebaseAuth creates GTMSessionFetcherService internally but sets reuseSession to NO and so fetcher service loses its _delegateDispatcher (it becomes nil):

https://github.com/firebase/firebase-ios-sdk/blob/683daa8bdf26add436fa3aec7875b75696930c2d/Firebase/Auth/Source/Backend/FIRAuthBackend.m#L605

So when session is created inside GTMSessionFetcher's beginFetchMayDelay:mayAuthorize:, service returns nil for serviceDelegate and session fetcher becomes the delegate for session:

Then fetcher-as-delegate gets wrapped into proxy class by different 3rd-party libraries (TrustKit, SplunkMint, etc). In my case it gets wrapped twice (by both TrustKit and SplunkMint).

So in this particular case there is no GTMSessionFetcherSessionDelegateDispatcher instance inside GTMSessionFetcherService and so its delegateDispatcherForFetcher: cannot return anything but nil. Now we only need to make it not crash :-)

@vladfilyakov
Copy link
Author

I hope this will get fixed sometime in the future. For now this is a workaround we applied:

// Code to swizzle `GTMSessionFetcherService.delegateDispatcherForFetcher:` in order to fix a crash
private extension GTMSessionFetcherService {
  private static var delegateDispatcherForFetcherIsSwizzled: Bool = false

  static func swizzleDelegateDispatcherForFetcher() {
    if delegateDispatcherForFetcherIsSwizzled {
      return
    }
    delegateDispatcherForFetcherIsSwizzled = true

    // `delegateDispatcherForFetcher:` is private and so we cannot use `#selector(..)`
    let originalSelector = sel_registerName("delegateDispatcherForFetcher:")
    let newSelector = #selector(new_delegateDispatcherForFetcher)
    if let originalMethod = class_getInstanceMethod(self, originalSelector),
      let newMethod = class_getInstanceMethod(self, newSelector) {
      method_setImplementation(originalMethod, method_getImplementation(newMethod))
    }
  }

  /*
   Modified code from GTMSessionFetcherService.m:
   https://github.com/google/gtm-session-fetcher/blob/c879a387e0ca4abcdff9e37eb0e826f7142342b1/Source/GTMSessionFetcherService.m#L382

   Original code returns GTMSessionFetcherSessionDelegateDispatcher but it's a private class
   so we are returning NSObject which is its superclass.
   */
  // Internal utility. Returns a fetcher's delegate if it's a dispatcher, or nil if the fetcher
  // is its own delegate (possibly via proxy) and has no dispatcher.
  @objc
  private func new_delegateDispatcherForFetcher(_ fetcher: GTMSessionFetcher?) -> NSObject? {
    if let fetcherDelegate = fetcher?.session?.delegate,
      let delegateDispatcherClass = NSClassFromString("GTMSessionFetcherSessionDelegateDispatcher"),
      fetcherDelegate.isKind(of: delegateDispatcherClass) {
      return fetcherDelegate as? NSObject
    }
    return nil
  }
}

Just call GTMSessionFetcherService.swizzleDelegateDispatcherForFetcher() before using Firebase. Hope this will help others.

@oailloud
Copy link

oailloud commented Sep 15, 2020

A big thank-you to @vladfilyakov, swizzling delegateDispatcherForFetcher works for me!

Here's my case, it might help others:

I use React Native and Firebase Auth and I have to do certificate pinning as it's a business requirement.
Therefore, I use TrustKit (through react-native-cert-pinner) but its swizzling conflicts with GTMSessionFetcherService's one (which is a dependency of Firebase Auth).

I can't configure TrustKit manually because it's impossible to customize the NSURLSession/NSURLConnection (see facebook/react-native#27701).

So it seems to me that it's the only way to go in a React Native environment.

@malyalavenu
Copy link

A big thank-you to @vladfilyakov, swizzling delegateDispatcherForFetcher works for me!

Here's my case, it might help others:

I use React Native and Firebase Auth and I have to do certificate pinning as it's a business requirement.
Therefore, I use TrustKit (through react-native-cert-pinner) but its swizzling conflicts with GTMSessionFetcherService's one (which is a dependency of Firebase Auth).

I can't configure TrustKit manually because it's impossible to customize the NSURLSession/NSURLConnection (see facebook/react-native#27701).

So it seems to me that it's the only way to go in a React Native environment.

Hi @oailloud I have the scenario. My question, where did you put the code suggested by @vladfilyakov

@malyalavenu
Copy link

@vladfilyakov @oailloud I found the file in .../pods/GTMSessionFetcher/Core/GTMSessionFetcherService.m I have not worked on Objective C or Swift. When I try to add the code I get the following errors :

error: unknown type name 'private' private extension GTMSessionFetcherService {
error: expected ';' after top level declarator private extension GTMSessionFetcherService {
error: expected identifier or '(' private extension GTMSessionFetcherService {
Appreciate if you can elaborate a bit more on the fix

@oailloud
Copy link

oailloud commented Jun 4, 2021

I had to create a .swift file and to add it to the project with XCode, it goes:

import Foundation

@objc extension GTMSessionFetcherService {
  private static var delegateDispatcherForFetcherIsSwizzled: Bool = false

  @objc
  static func swizzleDelegateDispatcherForFetcher() {
    // see https://github.com/google/gtm-session-fetcher/issues/190#issuecomment-607398226
  }

  @objc
  private func new_delegateDispatcherForFetcher(_ fetcher: GTMSessionFetcher?) -> NSObject? {
    // see https://github.com/google/gtm-session-fetcher/issues/190#issuecomment-607398226
  }
}

@malyalavenu
Copy link

malyalavenu commented Jun 16, 2021

@oailloud, Thank you for reply. Sorry couldn't get back to you, as I was building rest of the react-native project. I have the code as following in a .swift file under the project in Xcode 12.5. I get a Cannot find type 'GTMSessionFetcherService' in scope error (picture attached). I cleared the build, removed DerivedData as per usual suggestions. Can you help me why do I get this error ? Do I need to import anything extra ? I have no experience in coding Objective-C
Screenshot 2021-06-16 at 6 51 54 PM

import Foundation

@objc extension GTMSessionFetcherService {
  private static var delegateDispatcherForFetcherIsSwizzled: Bool = false

  @objc
  static func swizzleDelegateDispatcherForFetcher() {
    if delegateDispatcherForFetcherIsSwizzled {
      return
    }
    delegateDispatcherForFetcherIsSwizzled = true

    // `delegateDispatcherForFetcher:` is private and so we cannot use `#selector(..)`
    let originalSelector = sel_registerName("delegateDispatcherForFetcher:")
    let newSelector = #selector(new_delegateDispatcherForFetcher)
    if let originalMethod = class_getInstanceMethod(self, originalSelector),
      let newMethod = class_getInstanceMethod(self, newSelector) {
      method_setImplementation(originalMethod, method_getImplementation(newMethod))
    }
  }

  @objc
  private func new_delegateDispatcherForFetcher(_ fetcher: GTMSessionFetcher?) -> NSObject? {
    if let fetcherDelegate = fetcher?.session?.delegate,
      let delegateDispatcherClass = NSClassFromString("GTMSessionFetcherSessionDelegateDispatcher"),
      fetcherDelegate.isKind(of: delegateDispatcherClass) {
      return fetcherDelegate as? NSObject
    }
    return nil
  }
}

@oailloud
Copy link

I have pretty much the same content, maybe you need to create a bridging header to use Objective C in Swift? Take a look at https://developer.apple.com/documentation/swift/imported_c_and_objective-c_apis/importing_objective-c_into_swift

@malyalavenu
Copy link

malyalavenu commented Jun 17, 2021

@oailloud Thank you, that helped. I had to #import "GTMSessionFetcherService.h" in the bridging header and the build was successful.

@oailloud @vladfilyakov @blitzcrank @marcelo-schroeder I have one more question how do I access GTMSessionFetcherService.swizzleDelegateDispatcherForFetcher() before using Firebase, in my react-native code ? I am failing here. Many thanks for your help !

@oailloud
Copy link

I personally called it in the didFinishLaunchingWithOptions method in AppDelegate.m, just like this:
[GTMSessionFetcherService swizzleDelegateDispatcherForFetcher];

This way the swizzling should be done for Firebase calls to come.

@malyalavenu
Copy link

Thank you so much @oailloud. This solved my issue. I had to import #import "GTMSessionFetcherService.h" and also #import "<projectname>-Swift.h" as per this : https://stackoverflow.com/questions/58233526/no-known-class-method-for-selector-name/58233691#58233691

@malyalavenu
Copy link

@oailloud, Now I face the same issue with Firebase storage. We use the following from firebase. Everything works fine except @react-native-firebase/storage.

This happens when I try to invoke the getDownloadURL() of '@react-native-firebase/storage'. I get the following error in XCode :

TrustKit: Proxy-ing NSURLSessionDelegate: GTMSessionFetcherSessionDelegateDispatcher
Couldn't assign delegate.

The app hangs after this

As I already included the fix for GTMSessionFetcher don't know how to proceed further. Any thoughts ?

 "@react-native-firebase/app": "12.1.0",
    "@react-native-firebase/auth": "12.1.0",
    "@react-native-firebase/firestore": "12.1.0",
    "@react-native-firebase/messaging": "12.1.0",
    "@react-native-firebase/storage": "12.1.0",

I tried with other versions of the firebase npm modules, but the error persists

@oailloud
Copy link

I wouldn't know, I haven't used the storage module... I would expect it not to be related to modules though, GTMSessionFetcher seems pretty transversal and not on such a high level of the lib

@Aruna-Sree
Copy link

I hope this will get fixed sometime in the future. For now this is a workaround we applied:

// Code to swizzle `GTMSessionFetcherService.delegateDispatcherForFetcher:` in order to fix a crash
private extension GTMSessionFetcherService {
  private static var delegateDispatcherForFetcherIsSwizzled: Bool = false

  static func swizzleDelegateDispatcherForFetcher() {
    if delegateDispatcherForFetcherIsSwizzled {
      return
    }
    delegateDispatcherForFetcherIsSwizzled = true

    // `delegateDispatcherForFetcher:` is private and so we cannot use `#selector(..)`
    let originalSelector = sel_registerName("delegateDispatcherForFetcher:")
    let newSelector = #selector(new_delegateDispatcherForFetcher)
    if let originalMethod = class_getInstanceMethod(self, originalSelector),
      let newMethod = class_getInstanceMethod(self, newSelector) {
      method_setImplementation(originalMethod, method_getImplementation(newMethod))
    }
  }

  /*
   Modified code from GTMSessionFetcherService.m:
   https://github.com/google/gtm-session-fetcher/blob/c879a387e0ca4abcdff9e37eb0e826f7142342b1/Source/GTMSessionFetcherService.m#L382

   Original code returns GTMSessionFetcherSessionDelegateDispatcher but it's a private class
   so we are returning NSObject which is its superclass.
   */
  // Internal utility. Returns a fetcher's delegate if it's a dispatcher, or nil if the fetcher
  // is its own delegate (possibly via proxy) and has no dispatcher.
  @objc
  private func new_delegateDispatcherForFetcher(_ fetcher: GTMSessionFetcher?) -> NSObject? {
    if let fetcherDelegate = fetcher?.session?.delegate,
      let delegateDispatcherClass = NSClassFromString("GTMSessionFetcherSessionDelegateDispatcher"),
      fetcherDelegate.isKind(of: delegateDispatcherClass) {
      return fetcherDelegate as? NSObject
    }
    return nil
  }
}

Just call GTMSessionFetcherService.swizzleDelegateDispatcherForFetcher() before using Firebase. Hope this will help others.

Hi,

Actually, I too faced the same issue, and why can't TrustKit SDK and etc avoid handling of swizzling support for GTMServiceFetcher delegate methods?

[delegate class] == NSClassFromString(@"GTMSessionFetcher")

Thanks,
Aruna

@acecilia
Copy link

@mwyman Would you mind having a second look?

@mwyman
Copy link
Contributor

mwyman commented Jul 14, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants