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

SmtpClient SendMailAsync with CancellationToken API implementation #287

Merged
merged 9 commits into from
Nov 27, 2019

Conversation

MihaZupan
Copy link
Member

3f9f0e7 is the implementation of the API introduced in dotnet/corefx#24475

class System.Net.Mail.SmtpClient
{
    public Task SendMailAsync(string from, string recipients, string subject, string body)
+   public Task SendMailAsync(string from, string recipients, string subject, string body, CancellationToken cancellationToken)
    public Task SendMailAsync(MailMessage message)
+   public Task SendMailAsync(MailMessage message, CancellationToken cancellationToken)
}

The rest of the PR are SmtpClient tests using a mock SmtpServer.

CC @dotnet/ncl

@MihaZupan
Copy link
Member Author

MihaZupan commented Nov 26, 2019

e513058#diff-e2b2d690217e90426b121345ba65abdcR377 is failing in CI as the test now actually checks that NTLM is used.

Looks like it is essentially this: https://github.com/dotnet/corefx/issues/28961.

Stack trace for reference:

System.Net.Mail.SmtpException: Failure sending mail.
 ---> System.ComponentModel.Win32Exception (0x80090020): GSSAPI operation failed with error - An unsupported mechanism was requested. NTLM authentication requires the GSSAPI plugin 'gss-ntlmssp'.
   at System.Net.Security.NegotiateStreamPal.AcquireCredentialsHandle(String package, Boolean isServer, NetworkCredential credential)
   at System.Net.NTAuthentication.Initialize(Boolean isServer, String package, NetworkCredential credential, String spn, ContextFlagsPal requestedContextFlags, ChannelBinding channelBinding)
   at System.Net.Mail.SmtpNtlmAuthenticationModule.Authenticate(String challenge, NetworkCredential credential, Object sessionCookie, String spn, ChannelBinding channelBindingToken)
   at System.Net.Mail.SmtpConnection.SetContextAndTryAuthenticate(ISmtpAuthenticationModule module, NetworkCredential credential, ContextAwareResult context)
   at System.Net.Mail.SmtpConnection.ConnectAndHandshakeAsyncResult.Authenticate()
   at System.Net.Mail.SmtpConnection.ConnectAndHandshakeAsyncResult.SendEHelloCallback(IAsyncResult result)
...

@davidsh davidsh added this to the 5.0 milestone Nov 26, 2019
@davidsh davidsh requested a review from stephentoub November 26, 2019 22:48
@davidsh
Copy link
Contributor

davidsh commented Nov 26, 2019

@stephentoub PTAL the use of CancellationTokenRegistration.

@MihaZupan MihaZupan force-pushed the smtp-client-cancellation branch from b2c94d1 to 6076372 Compare November 27, 2019 00:32
Use Interlocked.Exchange instead of locks
@MihaZupan MihaZupan force-pushed the smtp-client-cancellation branch from 6076372 to 499a07e Compare November 27, 2019 11:02
@ManickaP
Copy link
Member

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephentoub
Copy link
Member

The src changes LGTM. Thanks.

Copy link
Contributor

@davidsh davidsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. You'll want to check the Outerloop CI results to verify the failures aren't related to the PR.

@MihaZupan
Copy link
Member Author

Outerloop failures are unrelated, Mail itself doesn't have any OL tests.

@davidsh
Copy link
Contributor

davidsh commented Nov 27, 2019

Outerloop failures are unrelated, Mail itself doesn't have any OL tests.

In that case, you are free to 'Squash and merge' this unless you want additional feedback from others.

@MihaZupan
Copy link
Member Author

Thanks for the reviews!

@MihaZupan MihaZupan merged commit 40b11db into dotnet:master Nov 27, 2019
MichalStrehovsky pushed a commit to MichalStrehovsky/runtime that referenced this pull request Nov 10, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants