Skip to content

NSURLSession swizzling incompatible with delegate usage via NSProxy #14478

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

Open
brianquinlan opened this issue Feb 21, 2025 · 11 comments
Open

NSURLSession swizzling incompatible with delegate usage via NSProxy #14478

brianquinlan opened this issue Feb 21, 2025 · 11 comments
Assignees

Comments

@brianquinlan
Copy link

Description

With firebase_performance_instrumentation_enabled set to NO, the delegate method in the repro code is called. With firebase_performance_instrumentation_enabled set to YES, the delegate method is not called.

It looks like the break in the call chain starts when respondsToSelector: is not called on ReproProxy with the URLSession:dataTask:didReceiveResponse:completionHandler: selector.

Reproducing the issue

#import <Foundation/Foundation.h>
#include <string.h>

#import "repro.h"

@interface Delegate : NSObject
- (void)URLSession:(NSURLSession *)session
              dataTask:(NSURLSessionDataTask *)dataTask
    didReceiveResponse:(NSURLResponse *)response
     completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))
                           completionHandler;
@end

@implementation Delegate {
}

- (void)URLSession:(NSURLSession *)session
              dataTask:(NSURLSessionDataTask *)dataTask
    didReceiveResponse:(NSURLResponse *)response
     completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))
                           completionHandler {
  NSLog(@"repro: Delegate method called");
  completionHandler(NSURLSessionResponseAllow);
}

@end

@interface ReproProxy : NSProxy
- (instancetype)init;
- (BOOL)respondsToSelector:(SEL)sel;
- (NSMethodSignature *)methodSignatureForSelector:(SEL)sel;
- (void)forwardInvocation:(__strong NSInvocation *)invocation;
@end

@implementation ReproProxy {
  Delegate *delegate;
}

- (instancetype)init {
  if (self) {
    delegate = [Delegate alloc];
  }
  return self;
}

- (BOOL)respondsToSelector:(SEL)sel {
  NSLog(@"repro: Responds to selector: %s", sel_getName(sel));
  return strcmp(sel_getName(sel),
                "URLSession:dataTask:didReceiveResponse:completionHandler:") ==
         0;
}

- (NSMethodSignature *)methodSignatureForSelector:(SEL)sel {
  NSLog(@"repro: methodSignatureForSelector");
  return [delegate methodSignatureForSelector:sel];
}

- (void)forwardInvocation:(NSInvocation *)invocation {
  NSLog(@"repro: forwardInvocation");
  [invocation invokeWithTarget:delegate];
}

@end

void doRequest() {
  NSURLSession *session =
      [NSURLSession sessionWithConfiguration:[NSURLSessionConfiguration
                                                 defaultSessionConfiguration]
                                    delegate:[[ReproProxy alloc] init]
                               delegateQueue:nil];
  NSURLSessionDataTask *task = [session
      dataTaskWithURL:[NSURL URLWithString:@"https://www.example.com/"]];
  [task resume];
  while ([task state] != 3)
    ;
}

Firebase SDK Version

13.31.2

Xcode Version

16.1

Installation Method

CocoaPods

Firebase Product(s)

Performance

Targeted Platforms

iOS

Relevant Log Output

Note how the selector for `URLSession:dataTask:didReceiveResponse:completionHandler:` was not queried:


default	14:20:20.554570-0800	Runner	repro: Responds to selector: URLSession:task:didCompleteWithError:
default	14:20:20.554626-0800	Runner	repro: Responds to selector: URLSession:task:didCompleteWithError:
default	14:20:20.554666-0800	Runner	repro: Responds to selector: URLSession:task:didSendBodyData:totalBytesSent:totalBytesExpectedToSend:
default	14:20:20.554704-0800	Runner	repro: Responds to selector: URLSession:task:didSendBodyData:totalBytesSent:totalBytesExpectedToSend:
default	14:20:20.554758-0800	Runner	repro: Responds to selector: URLSession:dataTask:didReceiveData:
default	14:20:20.554800-0800	Runner	repro: Responds to selector: URLSession:dataTask:didReceiveData:
default	14:20:20.554843-0800	Runner	repro: Responds to selector: URLSession:downloadTask:didFinishDownloadingToURL:
default	14:20:20.554896-0800	Runner	repro: Responds to selector: URLSession:downloadTask:didFinishDownloadingToURL:
default	14:20:20.554934-0800	Runner	repro: Responds to selector: URLSession:downloadTask:didResumeAtOffset:expectedTotalBytes:
default	14:20:20.555001-0800	Runner	repro: Responds to selector: URLSession:downloadTask:didResumeAtOffset:expectedTotalBytes:
default	14:20:20.555063-0800	Runner	repro: Responds to selector: URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:
default	14:20:20.555113-0800	Runner	repro: Responds to selector: URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:

If using Swift Package Manager, the project's Package.resolved

Expand Package.resolved snippet
Replace this line with the contents of your Package.resolved.

If using CocoaPods, the project's Podfile.lock

PODS:

  • cupertino_http (0.0.1):
    • Flutter
    • FlutterMacOS
  • Firebase/CoreOnly (11.8.0):
    • FirebaseCore (~> 11.8.0)
  • Firebase/Performance (11.8.0):
    • Firebase/CoreOnly
    • FirebasePerformance (~> 11.8.0)
  • firebase_core (3.12.0):
    • Firebase/CoreOnly (= 11.8.0)
    • Flutter
  • firebase_performance (0.10.1-3):
    • Firebase/Performance (= 11.8.0)
    • firebase_core
    • Flutter
  • FirebaseABTesting (11.8.0):
    • FirebaseCore (~> 11.8.0)
  • FirebaseCore (11.8.1):
    • FirebaseCoreInternal (~> 11.8.0)
    • GoogleUtilities/Environment (~> 8.0)
    • GoogleUtilities/Logger (~> 8.0)
  • FirebaseCoreExtension (11.8.0):
    • FirebaseCore (~> 11.8.0)
  • FirebaseCoreInternal (11.8.0):
    • "GoogleUtilities/NSData+zlib (~> 8.0)"
  • FirebaseInstallations (11.8.0):
    • FirebaseCore (~> 11.8.0)
    • GoogleUtilities/Environment (~> 8.0)
    • GoogleUtilities/UserDefaults (~> 8.0)
    • PromisesObjC (~> 2.4)
  • FirebasePerformance (11.8.0):
    • FirebaseCore (~> 11.8.0)
    • FirebaseInstallations (~> 11.0)
    • FirebaseRemoteConfig (~> 11.0)
    • FirebaseSessions (~> 11.0)
    • GoogleDataTransport (~> 10.0)
    • GoogleUtilities/Environment (~> 8.0)
    • GoogleUtilities/MethodSwizzler (~> 8.0)
    • GoogleUtilities/UserDefaults (~> 8.0)
    • nanopb (~> 3.30910.0)
  • FirebaseRemoteConfig (11.8.0):
    • FirebaseABTesting (~> 11.0)
    • FirebaseCore (~> 11.8.0)
    • FirebaseInstallations (~> 11.0)
    • FirebaseRemoteConfigInterop (~> 11.0)
    • FirebaseSharedSwift (~> 11.0)
    • GoogleUtilities/Environment (~> 8.0)
    • "GoogleUtilities/NSData+zlib (~> 8.0)"
  • FirebaseRemoteConfigInterop (11.8.0)
  • FirebaseSessions (11.8.0):
    • FirebaseCore (~> 11.8.0)
    • FirebaseCoreExtension (~> 11.8.0)
    • FirebaseInstallations (~> 11.0)
    • GoogleDataTransport (~> 10.0)
    • GoogleUtilities/Environment (~> 8.0)
    • GoogleUtilities/UserDefaults (~> 8.0)
    • nanopb (~> 3.30910.0)
    • PromisesSwift (~> 2.1)
  • FirebaseSharedSwift (11.8.0)
  • Flutter (1.0.0)
  • GoogleDataTransport (10.1.0):
    • nanopb (~> 3.30910.0)
    • PromisesObjC (~> 2.4)
  • GoogleUtilities/Environment (8.0.2):
    • GoogleUtilities/Privacy
  • GoogleUtilities/Logger (8.0.2):
    • GoogleUtilities/Environment
    • GoogleUtilities/Privacy
  • GoogleUtilities/MethodSwizzler (8.0.2):
    • GoogleUtilities/Logger
    • GoogleUtilities/Privacy
  • "GoogleUtilities/NSData+zlib (8.0.2)":
    • GoogleUtilities/Privacy
  • GoogleUtilities/Privacy (8.0.2)
  • GoogleUtilities/UserDefaults (8.0.2):
    • GoogleUtilities/Logger
    • GoogleUtilities/Privacy
  • integration_test (0.0.1):
    • Flutter
  • nanopb (3.30910.0):
    • nanopb/decode (= 3.30910.0)
    • nanopb/encode (= 3.30910.0)
  • nanopb/decode (3.30910.0)
  • nanopb/encode (3.30910.0)
  • objective_c (0.0.1):
    • Flutter
  • PromisesObjC (2.4.0)
  • PromisesSwift (2.4.0):
    • PromisesObjC (= 2.4.0)

DEPENDENCIES:

  • cupertino_http (from .symlinks/plugins/cupertino_http/darwin)
  • firebase_core (from .symlinks/plugins/firebase_core/ios)
  • firebase_performance (from .symlinks/plugins/firebase_performance/ios)
  • Flutter (from Flutter)
  • integration_test (from .symlinks/plugins/integration_test/ios)
  • objective_c (from .symlinks/plugins/objective_c/ios)

SPEC REPOS:
trunk:
- Firebase
- FirebaseABTesting
- FirebaseCore
- FirebaseCoreExtension
- FirebaseCoreInternal
- FirebaseInstallations
- FirebasePerformance
- FirebaseRemoteConfig
- FirebaseRemoteConfigInterop
- FirebaseSessions
- FirebaseSharedSwift
- GoogleDataTransport
- GoogleUtilities
- nanopb
- PromisesObjC
- PromisesSwift

EXTERNAL SOURCES:
cupertino_http:
:path: ".symlinks/plugins/cupertino_http/darwin"
firebase_core:
:path: ".symlinks/plugins/firebase_core/ios"
firebase_performance:
:path: ".symlinks/plugins/firebase_performance/ios"
Flutter:
:path: Flutter
integration_test:
:path: ".symlinks/plugins/integration_test/ios"
objective_c:
:path: ".symlinks/plugins/objective_c/ios"

SPEC CHECKSUMS:
cupertino_http: 94ac07f5ff090b8effa6c5e2c47871d48ab7c86c
Firebase: d80354ed7f6df5f9aca55e9eb47cc4b634735eaf
firebase_core: 6cbed78b4f298ed103a9fd034e6dbc846320480f
firebase_performance: 340bc5b09f0a38a886b6f53b2406941f3c3f3f94
FirebaseABTesting: 7d6eee42b9137541eac2610e5fea3568d956707a
FirebaseCore: 99fe0c4b44a39f37d99e6404e02009d2db5d718d
FirebaseCoreExtension: 3d3f2017a00d06e09ab4ebe065391b0bb642565e
FirebaseCoreInternal: df24ce5af28864660ecbd13596fc8dd3a8c34629
FirebaseInstallations: 6c963bd2a86aca0481eef4f48f5a4df783ae5917
FirebasePerformance: 4db21c8ac5a967d38ca8625ce77f58a7abb59bc0
FirebaseRemoteConfig: f63724461fd97f0d62f20021314b59388f3e8ef8
FirebaseRemoteConfigInterop: 98897a64aa372eac3c5b3fe2816594ccfaac55ef
FirebaseSessions: c4d40a97f88f9eaff2834d61b4fea0a522d62123
FirebaseSharedSwift: 672954eac7b141d6954fab9a32d45d6b1d922df8
Flutter: e0871f40cf51350855a761d2e70bf5af5b9b5de7
GoogleDataTransport: aae35b7ea0c09004c3797d53c8c41f66f219d6a7
GoogleUtilities: 26a3abef001b6533cf678d3eb38fd3f614b7872d
integration_test: 4a889634ef21a45d28d50d622cf412dc6d9f586e
nanopb: fad817b59e0457d11a5dfbde799381cd727c1275
objective_c: 89e720c30d716b036faf9c9684022048eee1eee2
PromisesObjC: f5707f49cb48b9636751c5b2e7d227e43fba9f47
PromisesSwift: 9d77319bbe72ebf6d872900551f7eeba9bce2851

PODFILE CHECKSUM: d2243213672c3c48aae53c36642ba411a6be7309

COCOAPODS: 1.16.2

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@brianquinlan
Copy link
Author

The methods that were correctly invoked through the proxy are all explicitly handled in FPRNSURLSessionDelegateInstrument.m and tested in FPRNSURLSessionInstrumentTest.m

A good starting point would probably be to add a test for the URLSession:dataTask:didReceiveResponse:completionHandler: method to FPRNSURLSessionInstrumentTest.m.

@tejasd
Copy link
Contributor

tejasd commented Feb 24, 2025

Thanks!

I've added a unit test that tries to test that behaviour and at least the unit test passes - with a caveat that the delegate is an NSObject and not an NSProxy. However, the unit test passes.

PTAL to see if there's something I'm missing.

@brianquinlan
Copy link
Author

Thanks!

I've added a unit test that tries to test that behaviour and at least the unit test passes - with a caveat that the delegate is an NSObject and not an NSProxy. However, the unit test passes.

PTAL to see if there's something I'm missing.

No, that looks convincing for the non-proxy case. If would be nice if proxies were supported.

@liamappelbe But maybe we can work around this on our end?

@liamappelbe
Copy link

Yeah, I think it makes sense to fix the bug from both ends, if possible. I'll experiment with eliminating proxies from our bindings today.

@tejasd
Copy link
Contributor

tejasd commented Feb 25, 2025

I added an additional unit test with an NSProxy for the NSURLSessionDelegate - and it still passes. Is it possible that it's some sort of a race condition? The unit test specifically has a method called waitAndRunBlockAfterResponse.

@liamappelbe
Copy link

@tejasd are you able to reproduce it with the code Brian posted above? I've been consistently reproing it with that code. Specifically, it only hits the line NSLog(@"repro: Delegate method called"); when firebase_performance is disabled.

@brianquinlan
Copy link
Author

@tejasd are you able to reproduce it with the code Brian posted above? I've been consistently reproing it with that code. Specifically, it only hits the line NSLog(@"repro: Delegate method called"); when firebase_performance is disabled.

In the PR, I suggested removing the delegate declaration, which is the only substantive difference between between my repro and the unit test. The presence of the delegate declaration may change the behavior of calls to respondsToSelector: (e.g. because the framework calls conformsToProtocol:)

@tejasd
Copy link
Contributor

tejasd commented Feb 26, 2025

I was able to reproduce it w/ NSProxy:

Image

but switching it to the Delegate shows that the delegate was called:

Image

Image

I also tried removing <NSURLSessionDelegate> in the unit test - and it passes.

@tejasd
Copy link
Contributor

tejasd commented Feb 28, 2025

I was able to find a solution that should fix the issue w/ NSProxy as a delegate.

It does work, but I want to refactor it before submitting it.

Image

@brianquinlan
Copy link
Author

Nice! FWIW, we are also working on a non-proxy solution on our side.

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

No branches or pull requests

5 participants