diff --git a/AppCheckCore/Sources/Core/APIService/AppCheckCoreAPIService.swift b/AppCheckCore/Sources/Core/APIService/AppCheckCoreAPIService.swift index c3a8e78..2eae6ba 100644 --- a/AppCheckCore/Sources/Core/APIService/AppCheckCoreAPIService.swift +++ b/AppCheckCore/Sources/Core/APIService/AppCheckCoreAPIService.swift @@ -92,6 +92,13 @@ public class AppCheckCoreAPIService: NSObject, // Recover Objective-C blocks that fail the dynamic cast bridging return unsafeBitCast(obj as AnyObject, to: AppCheckCoreAPIRequestHook.self) } + // Dropping a hook is otherwise invisible: the request still succeeds, + // just without whatever the hook would have contributed. + AppCheckCoreLogger.log( + code: .unknown, + logLevel: .error, + message: "Ignoring a request hook that is not a block: \(type(of: obj))." + ) return nil } ?? [] diff --git a/AppCheckCore/Tests/Unit/ObjC/AppCheckCoreObjCAPITests.m b/AppCheckCore/Tests/Unit/ObjC/AppCheckCoreObjCAPITests.m index 03e7688..5bee82a 100644 --- a/AppCheckCore/Tests/Unit/ObjC/AppCheckCoreObjCAPITests.m +++ b/AppCheckCore/Tests/Unit/ObjC/AppCheckCoreObjCAPITests.m @@ -246,25 +246,91 @@ - (void)testErrorDomainIsAvailableToObjectiveCAndUnchanged { XCTAssertEqualObjects(GACAppCheckErrors.errorDomain, @"com.google.app_check_core"); } +/// Builds a session that never touches the network. +- (NSURLSession *)stubSession { + NSURLSessionConfiguration *config = [NSURLSessionConfiguration ephemeralSessionConfiguration]; + config.protocolClasses = @[ [GACAppCheckMockURLProtocol class] ]; + return [NSURLSession sessionWithConfiguration:config]; +} + +/// Drives a request with a non-empty array of Objective-C blocks. +/// +/// Elsewhere this file passes `requestHooks:nil`, which never builds an +/// `NSArray` and so never crosses the bridge, and the Swift tests pass Swift +/// closures, which never bridge either. Neither performs the motion Firebase +/// actually performs. +/// +/// Constructing the service is not enough to catch a bridging regression: +/// `NSArray` to `Array` bridging is lazy, so the element cast is forced on +/// first access, which happens while building a request. +/// +/// The hook is shaped like `FIRHeartbeatLogger`'s App Check request hook, the +/// only non-nil hook Firebase passes in production. Asserting that its header +/// survives is stronger than asserting it ran: it shows the recovered block +/// was invoked with the correct ABI and handed a usable request, not merely +/// that control reached it. - (void)testRequestHooksBridging { XCTestExpectation *hookExpectation = [self expectationWithDescription:@"request hook called"]; + XCTestExpectation *completionExpectation = [self expectationWithDescription:@"completion called"]; - void (^hook)(NSMutableURLRequest *) = ^(NSMutableURLRequest *request) { + __block NSMutableURLRequest *capturedRequest = nil; + void (^heartbeatHook)(NSMutableURLRequest *) = ^(NSMutableURLRequest *request) { + [request setValue:@"test-heartbeat" forHTTPHeaderField:@"X-firebase-client"]; + capturedRequest = request; [hookExpectation fulfill]; }; - NSURLSessionConfiguration *config = [NSURLSessionConfiguration ephemeralSessionConfiguration]; - config.protocolClasses = @[ [GACAppCheckMockURLProtocol class] ]; - NSURLSession *stubSession = [NSURLSession sessionWithConfiguration:config]; - _GACAppCheckAPIService *apiService = - [[_GACAppCheckAPIService alloc] initWithURLSession:stubSession + [[_GACAppCheckAPIService alloc] initWithURLSession:[self stubSession] baseURL:nil APIKey:@"key" - requestHooks:@[ hook, @"not a block" ]]; + requestHooks:@[ heartbeatHook ]]; + + [apiService sendRequestWithURL:[NSURL URLWithString:@"https://test.local"] + httpMethod:@"GET" + body:nil + additionalHeaders:nil + completionHandler:^(id response, NSError *_Nullable error) { + [completionExpectation fulfill]; + }]; + + [self waitForExpectations:@[ hookExpectation, completionExpectation ] timeout:2.0]; + + XCTAssertTrue([capturedRequest isKindOfClass:[NSMutableURLRequest class]]); + XCTAssertEqualObjects([capturedRequest valueForHTTPHeaderField:@"X-firebase-client"], + @"test-heartbeat"); +} +/// Objects that are not blocks must be ignored rather than reinterpreted. +/// +/// `NSBlockOperation` is the case that matters, and the only one that tells +/// the two candidate filters apart. A class-name substring check admits it, +/// because "NSBlockOperation" contains "Block", and then bit-casts an +/// `NSOperation` into a callable; invoking that exits the process with +/// SIGSEGV. An `NSBlock` ancestry check rejects it. A non-block that is also +/// not named "...Block...", such as an `NSString`, is rejected by both and so +/// pins neither. +/// +/// The valid hook is last so that rejecting an element cannot be mistaken for +/// abandoning the rest of the array. +- (void)testNonBlockRequestHooksAreIgnored { + XCTestExpectation *hookExpectation = [self expectationWithDescription:@"request hook called"]; XCTestExpectation *completionExpectation = [self expectationWithDescription:@"completion called"]; + void (^hook)(NSMutableURLRequest *) = ^(NSMutableURLRequest *request) { + [hookExpectation fulfill]; + }; + + NSBlockOperation *blockOperation = [NSBlockOperation blockOperationWithBlock:^{ + XCTFail(@"A non-block request hook must never be invoked."); + }]; + + _GACAppCheckAPIService *apiService = + [[_GACAppCheckAPIService alloc] initWithURLSession:[self stubSession] + baseURL:nil + APIKey:@"key" + requestHooks:@[ blockOperation, @"not a block", hook ]]; + [apiService sendRequestWithURL:[NSURL URLWithString:@"https://test.local"] httpMethod:@"GET" body:nil diff --git a/AppCheckRecaptchaProvider/Sources/Public/AppCheckRecaptchaProvider.swift b/AppCheckRecaptchaProvider/Sources/Public/AppCheckRecaptchaProvider.swift index e9bfe64..61cf40d 100644 --- a/AppCheckRecaptchaProvider/Sources/Public/AppCheckRecaptchaProvider.swift +++ b/AppCheckRecaptchaProvider/Sources/Public/AppCheckRecaptchaProvider.swift @@ -46,13 +46,17 @@ public final class AppCheckRecaptchaProvider: NSObject, AppCheckCoreProvider { /// - resourceName: The name of the resource protected by App Check; for a Firebase App this is /// "projects/{project_id}/apps/{app_id}". /// - APIKey: The Google Cloud Platform API key. - /// - requestHooks: Hooks that will be invoked on requests through this service. - // `@convention(block)` is required because the Swift compiler cannot automatically - // bridge collections of closures (like an Array) to Objective-C blocks. This attribute - // changes the closure's representation to match the Objective-C block heap layout. + /// - requestHooks: Hooks invoked on each outgoing request. From Swift, pass + /// `[AppCheckCoreAPIRequestHook]`. From Objective-C, pass an `NSArray` of blocks with the + /// signature `void (^)(NSMutableURLRequest *)`; the signature is not checked at compile + /// time and a mismatch will crash when the hook is invoked. + /// + /// Typed `[Any]?` rather than `[AppCheckCoreAPIRequestHook]?` deliberately: Swift cannot + /// bridge an `NSArray` into a Swift `Array` whose element is a function type, so the typed + /// signature traps at runtime for any non-nil array passed from Objective-C. Do not + /// "simplify" this type — see PR #111. @objc public convenience init?(siteKey: String, resourceName: String, APIKey: String, - requestHooks: [@convention(block) (NSMutableURLRequest) -> Void]? = - nil) { + requestHooks: [Any]? = nil) { self.init( siteKey: siteKey, resourceName: resourceName, @@ -62,9 +66,17 @@ public final class AppCheckRecaptchaProvider: NSObject, AppCheckCoreProvider { ) } + /// - Parameter requestHooks: Hooks invoked on each outgoing request. From Swift, pass + /// `[AppCheckCoreAPIRequestHook]`. From Objective-C, pass an `NSArray` of blocks with the + /// signature `void (^)(NSMutableURLRequest *)`; the signature is not checked at compile + /// time and a mismatch will crash when the hook is invoked. + /// + /// Typed `[Any]?` rather than `[AppCheckCoreAPIRequestHook]?` deliberately: Swift cannot + /// bridge an `NSArray` into a Swift `Array` whose element is a function type, so the typed + /// signature traps at runtime for any non-nil array passed from Objective-C. Do not + /// "simplify" this type — see PR #111. @objc public convenience init?(siteKey: String, resourceName: String, APIKey: String, - requestHooks: [@convention(block) (NSMutableURLRequest) -> Void]? = - nil, + requestHooks: [Any]? = nil, actionName: String) { guard let sdk = RecaptchaEnterpriseSDKLoader(customAction: actionName) else { return nil