Skip to content

Fix crash in additionalTokenRefreshParameters when emmSupport is nil - #608

Merged
AkshatG6 merged 6 commits into
mainfrom
gandhiakshat/fix-emm-support-crash
Jul 31, 2026
Merged

AkshatG6 merged 6 commits into
mainfrom
gandhiakshat/fix-emm-support-crash

Conversation

@AkshatG6

Copy link
Copy Markdown
Contributor

Overview
This PR fixes an edge case missed by the previous fix for the Swift casting crash in additionalTokenRefreshParameters (#564).

The Problem
The previous fix correctly converted non-string values (like NSNumber) into their NSString representations to avoid a Swift type mismatch exception. However, it missed the scenario where emmSupport is nil or NO. In that case, [GIDEMMSupport parametersWithParameters:emmSupport:isPasscodeInfoRequired:] returned the original parameters dictionary early, bypassing the newly added string conversion helper.

The Fix
This change updates the early return block to apply [GIDEMMSupport dictionaryWithStringValuesFromDictionary:parameters], ensuring that the dictionary is safely stringified regardless of the emmSupport configuration.

@AkshatG6
AkshatG6 requested a review from w-goog July 23, 2026 22:43
Comment thread GoogleSignIn/Sources/GIDEMMSupport.m Resolved
Comment thread GoogleSignIn/Sources/GIDEMMSupport.m Resolved

@w-goog w-goog left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested test changes :)

@AkshatG6
AkshatG6 requested a review from w-goog July 29, 2026 18:39
Comment thread GoogleSignIn/Sources/GIDEMMSupport.m Resolved
@AkshatG6
AkshatG6 requested a review from w-goog July 29, 2026 21:59

@w-goog w-goog left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good to merge :) You can override the swift integration test to do so. Before merging, please file a follow-up issue to make the following changes:

  1. That the parametersWithParameters function drops key parameters that contain values like a nested array.
  2. We should fix the test swizzling UIDevice to Not Do That (or ensure all the tests are doing it the same way; using the real UIDevice will be non-hermetic).
  3. Consider upleveling these tests to the public API.
@AkshatG6

Copy link
Copy Markdown
Contributor Author

Merging this PR.

  1. Created Refactor and improve GIDEMMSupport parameters and tests #611 to track the requested GIDEMMSupport refactoring, UIDevice swizzling fixes, and upleveling of the unit tests.

  2. As discussed, I am overriding the failing Swift integration test to merge this PR. The failure is due to the iOS 18 simulators breaking the hardcoded XCUITest scripts, which will be fixed in a follow-up PR.

@AkshatG6
AkshatG6 merged commit 8857c24 into main Jul 31, 2026
21 of 22 checks passed
@AkshatG6
AkshatG6 deleted the gandhiakshat/fix-emm-support-crash branch July 31, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants