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

Fix crash in additionalTokenRefreshParameters when emmSupport is nil#608
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
Comment thread GoogleSignIn/Sources/GIDEMMSupport.m

@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
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants