[Blazor] Remove obsolete APIs from Components#16
Conversation
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if ((Navigation.HistoryEntryState != null && !ValidateSignOutRequestState()) || | ||
| // For backcompat purposes, keep SignOutManager working, even though we now use the history.state for this. | ||
| (Navigation.HistoryEntryState == null && !await SignOutManager.ValidateSignOutState())) | ||
| if (!ValidateSignOutRequestState()) |
There was a problem hiding this comment.
🔴 Logout validation always fails when HistoryEntryState is null
The change from if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState()) to if (!ValidateSignOutRequestState()) breaks logout functionality when HistoryEntryState is null.
Click to expand
Analysis
The ValidateSignOutRequestState() method at line 380-383 returns:
return GetCachedNavigationState()?.Interaction == InteractionType.SignOut;When Navigation.HistoryEntryState is null/empty, GetCachedNavigationState() returns null (line 392-395), causing ValidateSignOutRequestState() to return false.
Old behavior: if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState()) - when HistoryEntryState is null, the condition evaluates to false && ... = false, so logout proceeds normally.
New behavior: if (!ValidateSignOutRequestState()) - when HistoryEntryState is null, the condition evaluates to !false = true, causing logout to fail with "The logout was not initiated from within the page."
Impact
This is a breaking change that prevents users from logging out when HistoryEntryState is null, which contradicts the stated intent of the commit message "Fix sign-out validation logic to not fail when HistoryEntryState is null".
Recommendation: Restore the original condition: if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState())
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (_tokenResult.InteractionOptions != null) | ||
| { | ||
| _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions); |
There was a problem hiding this comment.
🟡 Potential null reference when calling NavigateToLogin with null InteractiveRequestUrl
The change removes the null check for InteractiveRequestUrl before calling NavigateToLogin, which expects a non-null string! parameter.
Click to expand
Analysis
The old code checked both properties:
if (_tokenResult.InteractionOptions != null && _tokenResult.InteractiveRequestUrl != null)The new code only checks InteractionOptions:
if (_tokenResult.InteractionOptions != null)The NavigateToLogin method signature at NavigationManagerExtensions.cs:57 requires a non-null loginPath parameter (string! in the public API). If a custom IAccessTokenProvider implementation sets InteractionOptions without setting InteractiveRequestUrl, calling NavigateToLogin(_tokenResult.InteractiveRequestUrl, ...) will pass null to a method expecting non-null.
Impact
While the default RemoteAuthenticationService always sets both properties together, custom implementations could trigger this path, causing unexpected behavior or NullReferenceException.
Recommendation: Restore the original condition: if (_tokenResult.InteractionOptions != null && _tokenResult.InteractiveRequestUrl != null)
Was this helpful? React with 👍 or 👎 to provide feedback.
Benchmark PR from qodo-benchmark#88