Use newer method overloads in auth handlers #30715
Conversation
Use new overloads of methods added in .NET 5.0 that accept a CancellationToken.
Use StringBuilder.Append(char) instead of StringBuilder.Append(string) when there is only a single character.
Use WriteAsync() overload that accepts a span and a CancellationToken, instead of an array and indexes to write all bytes.
Make methods that do not access instance data static.
Remove an unused method and an unused parameter.
Use the compound assignment operator.
Fix copy-paste from Facebook tests.
ghost
added
the
| @@ -276,7 +276,7 @@ public async virtual Task SignOutAsync(AuthenticationProperties properties) | |||
| Response.Headers[HeaderNames.Pragma] = "no-cache"; | |||
| Response.Headers[HeaderNames.Expires] = HeaderValueEpocDate; | |||
|
|
|||
| await Response.Body.WriteAsync(buffer, 0, buffer.Length); | |||
| await Response.Body.WriteAsync(buffer, Context.RequestAborted); | |||
There was a problem hiding this comment.
It's better not to pass the CT to WriteAsync. The write will fail silently if the client disconnects, but if you pass the CT then it will throw. We don't care if this write succeeds or fails.
Remove the CancellationToken from the WriteAsync() call to address review feedback.
|
Hello @Tratcher! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
|
Thanks |
| @@ -105,7 +105,7 @@ protected virtual async Task<bool> HandleRemoteSignOutAsync() | |||
| && Request.ContentType.StartsWith("application/x-www-form-urlencoded", StringComparison.OrdinalIgnoreCase) | |||
| && Request.Body.CanRead) | |||
| { | |||
| var form = await Request.ReadFormAsync(); | |||
| var form = await Request.ReadFormAsync(Context.RequestAborted); | |||
There was a problem hiding this comment.
Why do we need this? This happens automagically without passing in the token.
There was a problem hiding this comment.
TIL - I didn't realise this was equivalent to the default behaviour with CancellationToken.None/default.
| @@ -148,7 +148,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync | |||
| && Request.ContentType.StartsWith("application/x-www-form-urlencoded", StringComparison.OrdinalIgnoreCase) | |||
| && Request.Body.CanRead) | |||
| { | |||
| var form = await Request.ReadFormAsync(); | |||
| var form = await Request.ReadFormAsync(Context.RequestAborted); | |||
|
In general we shouldn't need to pass the request aborted token reading the request. The server implementation already has knowledge of the so we don't need to pay the cost here. |
PR Title
Use newly added overloads of already-used methods in authentication handlers.
PR Description
I noticed that some of the new overloads added in .NET 5.0 weren't used, so I've switched various usages within the Authentication part of the solution to use them. I also applies some minor improvements in the files I touched that Visual Studio suggested.
CancellationToken.WriteAsync()overload that accepts a spanand a, instead of an array and indexes.CancellationTokenStringBuilder.Append(char)instead ofStringBuilder.Append(string)when there is only a single character.static.