Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveDirectoryAuthenticationProvider.AcquireTokenAsync method fails when ConnectTimeout set to 0 or >= int.MaxValue/1000 #2563

Closed
chris5287 opened this issue Jun 11, 2024 · 1 comment · Fixed by #2651

Comments

@chris5287
Copy link

Well that was fun to debug!

cts.CancelAfter(parameters.ConnectionTimeout * 1000); // Convert to milliseconds

As the ConnectionTimeout is used to set the cancellation token's CancelAfter * 1000...

  • If ConnectionTimeout = 0 then 0 * 1000 = 0 - this causes the cancellation token to immediately expire, so all operations such as gather the tenant metadata fail with A task was canceled exception message (which is what led me down this rabbit hole).
  • If ConnectionTimeout = int.MaxValue (or any value greater than int.MaxValue / 1000) then int.MaxValue * 1000 throws as the result is larger than int.

Ideally this needs more defensive coding, eg:

  • When ConnectionTimeout == 0 we do not require a CancelAfter?
  • When ConnectionTimeout >= int.MaxValue / 1000 set the CancelAfter to int.MaxValue?
@dauinsight dauinsight added the 🆕 Triage Needed For new issues, not triaged yet. label Jun 12, 2024
@dauinsight
Copy link
Contributor

Thanks for pointing this out. We'll likely add this to our backlog

@kf-gonzalez2 kf-gonzalez2 removed the 🆕 Triage Needed For new issues, not triaged yet. label Jun 18, 2024
David-Engel added a commit to David-Engel/SqlClient that referenced this issue Jul 5, 2024
David-Engel added a commit to David-Engel/SqlClient that referenced this issue Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

3 participants