Skip to content

Commit

Permalink
Fixed crash when cloud shell provider timed out or was closed waiting…
Browse files Browse the repository at this point in the history
… for login (#16364)

## Summary of the Pull Request
Cloud shell connection calls out to Azure to do a device code login.
When the polling interval is exceeded or the tab is closed, the method
doing the connection polling returns `nullptr`, and `AzureConnection`
immediately tries to `GetNamedString` from it, causing a crash. This
doesn't repro on Terminal Stable or Preview, suggesting it's pretty
recent related to the update of this azureconnection.

This is just a proposed fix, not sure if you want to do more extensive
changes to the affected class or not, so marking this as a draft.

## References and Relevant Issues
* N/A - encountered this while using the terminal myself

## PR Checklist/Validation
Tested out a local dev build:

- [x] Terminal doesn't crash when cloudshell polling interval exceeded
- [x] Terminal doesn't crash when cloudshell tab closed while polling
for Azure login

(cherry picked from commit 0c4751b)
Service-Card-Id: 91232783
Service-Version: 1.18
  • Loading branch information
reynoldsa authored and DHowett committed Jan 22, 2024
1 parent 6cae665 commit e756373
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/cascadia/TerminalConnection/AzureConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,15 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

// Wait for user authentication and obtain the access/refresh tokens
auto authenticatedResponse = _WaitForUser(devCode, pollInterval, expiresIn);

// If user closed tab, `_WaitForUser` returns nullptr
// This also occurs if the connection times out, when polling time exceeds the expiry time
if (!authenticatedResponse)
{
_transitionToState(ConnectionState::Failed);
return;
}

_setAccessToken(authenticatedResponse.GetNamedString(L"access_token"));
_refreshToken = authenticatedResponse.GetNamedString(L"refresh_token");

Expand Down

0 comments on commit e756373

Please sign in to comment.