* Deliver callbacks to App Delegates that use fast forwarding
Proxying a delegate that handles a selector via forwardingTargetForSelector:
instead of implementing it caused the callback to be dropped silently.
createSubclassWithObject: records the original implementation with
class_getInstanceMethod(realClass, sel), which is NULL for such a delegate.
Adding the donor method then makes message lookup succeed, so the runtime no
longer consults the forwarding target, and each donor's trailing
`if (realIMP)` is false. The message is neither delivered nor diagnosed.
This is the shape of the delegate SwiftUI installs for
UIApplicationDelegateAdaptor: SwiftUI.AppDelegate implements only a handful of
selectors -- didFinishLaunchingWithOptions:, configurationForConnecting:options:,
handleEventsForBackgroundURLSession:, respondsToSelector: and
forwardingTargetForSelector: -- and forwards the rest to the app's own delegate.
An app using that adaptor together with FirebaseMessaging or FirebaseAuth
therefore stops receiving the APNS callbacks on its own delegate, and any app
using a GoogleUtilities-based SDK stops receiving continueUserActivity: and
openURL:options:.
Each donor now falls back to the forwarding target when the original
implementation is NULL, so the behaviour is unchanged for delegates that
implement the selector themselves.
* Address review: forward via NSInvocation, add tests and changelog
Forward through NSInvocation rather than by messaging the target directly, so
that the APNS App Delegate selectors are not referenced as symbols. Referencing
them directly triggers an Apple review warning about a missing Push Notification
Entitlement even when the code never runs, which is why the surrounding code
builds those selectors from strings.
Adds tests for the value returned by a forwarded selector, for the completion
handlers of application:handleEventsForBackgroundURLSession:completionHandler:
and application:didReceiveRemoteNotification:fetchCompletionHandler:, and for a
forwarding delegate whose target does not implement the selector at all.
Adds the changelog entry.
* Guard iOS/tvOS-only selectors in forwarding delegate tests
Fixes the macOS build: openURL:options:, handleEventsForBackgroundURLSession:
and the fetchCompletionHandler: variant aren't part of NSApplicationDelegate.
* Fix unused variable warning in macOS pod lib lint
* Address review: balance dispatch group, harden forwarding tests
- Enter the callback group and call forwardSelector: once, leaving the group
if nothing was forwarded, so the system completion handler always runs.
- Guard stored completion handlers against nil in tests and assert the
forwarded fetch result.
- Add a test merging an interceptor's fetch result with a forwarding delegate.
- Wrap the CHANGELOG entry at 80 columns.
`+[GULAppEnvironmentUtil isAppStoreReceiptSandbox]` read
`-[NSBundle appStoreReceiptURL]` on every call. That property makes a
synchronous XPC call to the LaunchServices daemon, which can block the
calling thread for seconds when the daemon is busy. `+isFromAppStore`
repeated the call because it only cached `IsAppEncrypted()`. Firebase
calls these from the main thread (heartbeat user agent, Messaging APNs
token handling), which caused hangs and watchdog terminations.
Compute both values once per process with `dispatch_once`. Their inputs
(receipt type, bundle contents, Mach-O encryption info, and the
Info.plist opt-out key) can't change while the app is running. The
original bodies move unchanged into file-local helpers.
Part of firebase/firebase-ios-sdk#16726
* fix(userdefaults): check stored value types in typed getters
The typed getters sent NSNumber selectors (integerValue, boolValue, ...)
to whatever object was stored, so a persisted value of an unexpected type
(e.g. an array or data written from a server response) crashed with an
unrecognized selector. stringForKey:, arrayForKey: and dictionaryForKey:
returned the stored object unchecked, handing callers the wrong type.
Mirror NSUserDefaults: numeric getters accept NSNumber or NSString,
stringForKey: converts an NSNumber to its stringValue, and the collection
getters return nil for other types.
* fix(network): handle non-HTTP responses and nil request URLs
URLSession:task:didCompleteWithError: asserted that the task response is
an NSHTTPURLResponse. The assertion is compiled out of release builds, so
a non-HTTP response (possible when a custom NSURLProtocol intercepts the
request) was cast and passed to callers, who then crash calling
statusCode or allHeaderFields. Replace the assertion with a runtime check
and report a GULErrorCodeNetworkInvalidResponse error instead.
Also avoid inserting a nil request URL into the array literal used to log
a server trust evaluation error.
* fix(logger): look up cached os_log objects by category
GULOSLogBasic stored os_log objects under the category key but looked
them up by subsystem, so the cache never hit and os_log_create ran for
every log message.
* docs(changelog): note userdefaults, network and logger fixes
* fix(network): keep system errors for non-HTTP responses
When a task finished with a non-HTTP response and an error, the error
was replaced with GULErrorCodeNetworkInvalidResponse. Pass the more
specific system error through and only create an invalid-response error
when there is none.
Also fall back to the task's original request URL when logging a server
trust evaluation error, so the log still names the host if _request is
unset.
* fix(keychain): reject nil objects and invalid keys in GULKeychainStorage
Validate arguments on the caller's thread and report an error through the
completion handler, instead of dispatching them to the internal queues.
Previously, a nil object made NSCache throw inside a dispatch_async block
on the in-memory cache queue. The caller could not catch the exception, so
the app crashed with a stack attributed to GoogleUtilities, the same
pattern as #248. A nil key did not throw, but it removed kSecAttrAccount
from the keychain query, so get/remove could match other items stored for
the same service.
Add GULKeychainStorageInvalidInputTests. The rejected input never reaches
the keychain, so these tests run under SwiftPM and on macOS, unlike
GULKeychainStorageTests.
* fix(userdefaults): validate entire value graph in GULUserDefaults
Use NSPropertyListSerialization to validate the whole value before writing
it to NSUserDefaults, instead of checking only the top-level class.
Previously, a collection containing non-property-list content, such as an
NSNull decoded from a JSON null or a dictionary with non-string keys,
passed the check. NSUserDefaults then aborted the process, which callers
cannot catch. Such values are now rejected with the existing warning log
and the stored value is left unchanged.
* test(keychain): avoid capturing self in GULKeychainStorage completion blocks
Record results in __block variables inside the completion handlers and
assert after waiting for the expectation. The blocks are retained by
self.storage, and capturing self in them (directly or through XCTAssert*
macros) triggered -Warc-retain-cycles in the CocoaPods lint.
* fix(keychain): report exceptions from decoding keychain items as errors
+[NSKeyedUnarchiver unarchivedObjectOfClass:fromData:error:] returns an
error for malformed archives, but an exception raised by the decoded
class's -initWithCoder: (for example while decoding corrupted keychain
data), or by a nil objectClass, still propagates. GULKeychainStorage
decodes on keychainQueue, where the caller cannot catch the exception, so
the app crashed.
Decode through a helper that catches the exception and returns an NSError,
which is passed to the completion handler.
* fix(keychain): allow empty string keys
An empty string is a valid, distinct keychain account: it does not throw
in NSCache and keeps kSecAttrAccount in the query, so it is not a
wildcard like a nil key. Rejecting it would make items already stored
under @"" unreadable and undeletable, and turn previously working saves
into errors. Only reject nil and non-string keys.
* test(network): add failing tests for GULMutableDictionary key copying
GULMutableDictionary's setters capture the caller's key in an async block,
and NSMutableDictionary copies the key only when the block runs on the
internal queue. A mutable key changed right after the call is stored under
the changed value. Add regression tests for setObject:forKey: and keyed
subscripting that mutate the key right after the set.
These tests fail until the fix in the next commit.
* fix(network): copy GULMutableDictionary keys before dispatching writes
setObject:forKey: and setObject:forKeyedSubscript: passed the caller's key
into an async block, so NSMutableDictionary copied it only when the write
ran on the internal queue. A mutable key changed right after the call was
stored under the changed value.
Copy the key on the caller's thread before dispatching. NSMutableDictionary
already copies keys, so this only moves the copy earlier. If a key's copy is
nil, ignore the write and log a warning so the internal queue doesn't
throw.
removeObjectForKey: is unchanged: its key is `id`, not `id<NSCopying>`, and
NSMutableDictionary doesn't copy keys on removal.
Also add CHANGELOG entries for this fix and for the Swift-visible
nullability changes to the getters and the subscript setter.
* fix(network): log ignored GULMutableDictionary writes and document nil handling
Follow-up to the nil-key fix in #249.
- Log a warning (I-NET902000 for a nil key, I-NET902001 for a nil object)
when a write is ignored. Silently dropping the write hid the caller's bug;
the log keeps it visible without crashing.
- Mark the object parameter of -setObject:forKeyedSubscript: as nullable. A
nil object removes the key, as with NSMutableDictionary, and the tests rely
on this. It also lets Swift callers write `dict[key] = nil`.
- Document the nil handling in the header.
- Make testNilObjectIsIgnored check that an existing entry is kept, and pass
nil directly in testKeyedNilObjectRemovesKey.
* fix(network): move GULMutableDictionary log codes to 903xxx
I-NET902000-I-NET902006 are already used by GULReachabilityMessageCode, so
the new GULMutableDictionary warnings collided with reachability messages.
Use 903000/903001, which are unused in GoogleUtilities and
firebase-ios-sdk, and note the reserved reachability range next to the
enum.
* add more nil key tests
* fix(network): return nil early for nil keys in GULMutableDictionary getters
NSDictionary happens to return nil for a nil key, but this is not
documented. Return early from -objectForKey: and
-objectForKeyedSubscript: so the behavior does not depend on Foundation
and matches the nil-key guards on the write paths.
- Document the nil-key behavior of both getters in the header.
- Add tests that a nil-key lookup returns nil when the dictionary has
entries.
* test(network): add defensive GULMutableDictionary tests
- Check that -dictionary returns a snapshot that later writes don't change.
- Split the nil-key write test into one test per write path so a failure
names the method that caused it, and cover `dict[nil] = nil`.
- Cover removing a missing key, -removeAllObjects on an empty dictionary,
and -description.
* changelog and spec
* test(network): assert empty precondition in testRemoveAllWhenEmpty
* test(network): verify setup in GULMutableDictionary tests
Several tests could pass even if their setup silently failed. Check that
the setup took effect:
- testDictionaryReturnsSnapshot: the later writes changed the live
dictionary, so the snapshot assertions are meaningful.
- testKeyedNilObjectRemovesKey: the entry exists before it is removed.
- testObjectForNilKey(KeyedSubscript): the entry exists, so the tests
show a nil key does not match it.
* fix(network): mark GULMutableDictionary getters as returning nullable
objectForKey: and objectForKeyedSubscript: are documented to return nil for a missing or nil key, but were declared non-null, so Swift imported objectForKey(_:) as Any. Declare both as nullable id so the signatures match the documented behavior. Swift callers of objectForKey(_:) now get Any?; the subscript already read as Any? because its setter is nullable.
* test(network): add GULMutableDictionary concurrent access stress test
GULMutableDictionary is documented as providing atomic accessors and
mutators and is shared across threads by GULNetwork and the app delegate
swizzlers, but no test exercised it concurrently. Mix reads, writes,
removes, and nil-key calls across threads with dispatch_apply and check
that the final state is consistent.
Passed 200 repeated runs and a Thread Sanitizer run on the iOS simulator
with no reports.
---------
Co-authored-by: Nick Cooke <nickcooke@google.com>
GULMutableDictionary forwards writes to NSMutableDictionary asynchronously,
so a nil key (or nil object in setObject:forKey:) throws on the internal
queue and crashes the app instead of the caller. This took down apps using
Firebase Analytics when a malformed experiment response produced a nil key
(firebase/firebase-ios-sdk#16728).
* Update [GULSwizzler ivarObjectsForObject] to correctly get the class to fetch ivars from and update logic to exclude weak ivars
* Add unit tests for [GULSwizzler ivarObjectsForObject]
* test: migrate away from OCMock
Replace dynamic OCMock mocks with handwritten fake classes across the test suite to improve Swift interoperability, address concurrency issues, and reduce test brittleness. Removed OCMock from podspec and SPM dependencies.
* gemfile
* Remove other files and deps
* Apply suggestion from @ncooke3
* Apply suggestion from @ncooke3
* Fix EXC_BAD_ACCESS in GULNetworkURLSession cleanup
This commit replaces the manual O(N) cleanup loop in `setSessionInFetcherMap:forSessionID:` with an O(1) passive self-cleanup mechanism using Associated Objects.
It introduces `GULSessionDeallocTracker`, which is tied to the lifecycle of the `NSURLSession`. To prevent `objc_loadWeakRetained` crashes when the session is being destroyed, the tracker holds a strong reference to the `GULNetworkURLSessionWeakHolder` wrapper and uses direct pointer comparison during `dealloc` instead of reading the weak reference. It also ensures old trackers are deallocated outside of the `NSLock` to prevent deadlocks.
* Refactor: explicitly nil out old tracker to silence potential unused variable warnings
* Refactor: remove redundant comment
* Fixing linter and style
* Fix dead store warning for static analysis in GULNetworkURLSession
* Fix nil key exception in GULRuntimeSnapshot by verifying classString
* Address PR feedback: Add deadlock prevention comment and parameter assert
* Increase timeout for testSessionMapStress to 60s
* Refactor: remove misplaced stress test and add explicit test for passive ARC cleanup in GULNetworkURLSession
* Increase Keychain test expectation timeouts to reduce CI flakiness
The macos-15 runner image lacks the iOS 18.2 simulator runtime for Xcode 16.2, causing CI jobs to fail.
This adds a workflow step to explicitly download the required simulator, ensuring tests can run with the minimum supported Xcode version.
Nick Cooke