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

System.Net.Mail.SmtpClient is marked as obsolete in docs, but not in the source code #2986

Closed
PashaPash opened this issue Apr 11, 2017 · 50 comments
Closed

Comments

@PashaPash
Copy link

@PashaPash PashaPash commented Apr 11, 2017

System.Net.Mail.SmtpClient class page is currently states that class is obsolete, and offers https://github.com/jstedfast/MailKit as a replacement.

[System.Obsolete("SmtpClient and its network of types are poorly designed, we strongly recommend you use https://github.com/jstedfast/MailKit and https://github.com/jstedfast/MimeKit instead")]
public class SmtpClient : IDisposable

That class has no Obsolete attribute in both Reference Source for 4.7 and corefx source.

Mono version of SmtpClient was marked as Obsolete few years ago by @migueldeicaza, and then that external lib link somehow made its way to official 4.7 docs. Please remove it.

@jstedfast
Copy link

@jstedfast jstedfast commented Apr 11, 2017

Sssshhhhhh!!!! This is epic for MimeKit and MailKit 😃

@rpetrusha
Copy link

@rpetrusha rpetrusha commented Apr 11, 2017

Thanks, @PashaPash, for pointing this out and providing context on how it might have happened.
Obsolete information (types and members tagged with the Obsolete attribute) is generated by reflection, so we can't simply remove it; this points to some false assumptions in the reflection process. I've opened an internal bug, #968551, to track the issue.

@dend
Copy link
Contributor

@dend dend commented Apr 27, 2017

Thanks @mairaw @rpetrusha @PashaPash - we are investigating. This is indeed a byproduct of how we auto-generate documentation.

Opened a mdoc issue:
mono/api-doc-tools#76

@svick
Copy link
Contributor

@svick svick commented Jun 26, 2017

I think there's another instance of this on Socket.​Duplicate​And​Close. All versions of the docs for that have the MonoLimitation attribute:

[System.MonoLimitation("We do not support passing sockets across processes, we merely allow this API to pass the socket across AppDomains")]
@ocdtrekkie
Copy link

@ocdtrekkie ocdtrekkie commented Jun 25, 2018

This is actually slightly more baffling if you come from the VB display, where the [System.Obsolete( bit doesn't display. The docs just appear to say "it's obsolete" and fails to offer up an alternative. I came here to seek an answer. So System.Net.Mail is not obsolete?

@mairaw
Copy link
Contributor

@mairaw mairaw commented Jun 25, 2018

Yes @ocdtrekkie, I was just discussing this with @dend last week. Here's an issue related to that request: mono/api-doc-tools#274

@joelmartinez
Copy link
Contributor

@joelmartinez joelmartinez commented Jun 25, 2018

Yes, this change is very close to landing ... will be in the next release of mdoc :)

@ocdtrekkie
Copy link

@ocdtrekkie ocdtrekkie commented Jun 25, 2018

Great to hear. :)

@Allon-Guralnek
Copy link

@Allon-Guralnek Allon-Guralnek commented Jul 11, 2018

Has the obsolete notice been removed from the docs? I can't find it on the linked page.

@mairaw
Copy link
Contributor

@mairaw mairaw commented Jul 17, 2018

Temporarily removed @Allon-Guralnek. Next time we run CI in our docs this will be added back.
We're still waiting for the fix that can distinguish attributes per .NET implementation.

@joelmartinez
Copy link
Contributor

@joelmartinez joelmartinez commented Jul 17, 2018

Just FYI, the next time it's added back, it will have the appropriate attributes (FrameworkAlternate) to indicate that the attribute only exists in a subset of frameworks (the feature became available two releases of mdoc ago) ... however, not sure when the additional upstream work will be completed, so that the front-end properly filters this attribute out for frameworks like .net desktop, and .net core :)

@af4jm
Copy link

@af4jm af4jm commented Aug 28, 2018

I'm seeing this in .NET Core 2.1.2 today... warning DE0005: SmtpClient is deprecated... does that mean there's also a bug in <PackageReference Include="Microsoft.DotNet.Analyzers.Compatibility" Version="0.2.12-alpha" PrivateAssets="All" />?

@karelz
Copy link
Member

@karelz karelz commented Oct 17, 2018

BTW: SmtpClient is obsolete API: https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md (that is what causes the warning DE0005.
We do not plan to add the Obsolete attribute on it to avoid breaking everyone's compilation (with warnings treated as errors).

@ocdtrekkie
Copy link

@ocdtrekkie ocdtrekkie commented Oct 18, 2018

@karelz I am very sad if that's the official position of Microsoft, to dump basic Internet functionality off to random third parties. Given examples from other platforms like left-pad, I am extremely hesitant to add random dependencies to software I develop because I have no reason to trust the developer.

I do see in the case of MailKit that it appears to be being maintained by a Microsoft employee, but as a personal project, which means there's really no guarantee of quality or support.

@jstedfast
Copy link

@jstedfast jstedfast commented Oct 18, 2018

Hi @ocdtrekkie,

I'm the MimeKit and MailKit maintainer.

FWIW, I think you'll find that both projects are very high quality codebases and I am very active with maintenance of both projects. This includes fixing bugs, answering questions, adding new features, etc.

Hope that helps assuage your fears... at least somewhat.

@svick
Copy link
Contributor

@svick svick commented Oct 18, 2018

@ocdtrekkie MailKit is also part of the .Net Foundation, in case that makes a difference for you.

@PashaPash
Copy link
Author

@PashaPash PashaPash commented Oct 18, 2018

@karelz @jstedfast "doesn't scale to modern requirements of the protocol" is too generic.

Is there any specific reason to use MailKit instead of SmtpClient to send mail over SMTP? Will it be faster or more reliable?
It seems that SmtpClient does not support pipelining and rfc2231 for filenames, but I'm not sure about other protocol features.

@karelz
Copy link
Member

@karelz karelz commented Oct 18, 2018

@PashaPash we do not have list of all missing RFCs and features in SmtpClient. The fact is that SmtpClient hasn't been innovated in a long time, while MailKit was.
We've also heard that the performance of SmtpClient is not as good as MailKit.

If you are sending just occasional emails (e.g. alerts, or tooling), then SmtpClient is likely good enough for you. If you need modern, high-perf, latest technology, then I would recommend to use MailKit.

@ocdtrekkie
Copy link

@ocdtrekkie ocdtrekkie commented Oct 18, 2018

@jstedfast My comment wasn't intended to be a slight at your projects, I do hope you didn't take it as one.

But I think there's definitely a strong value to features in .NET Framework code being officially managed and distributed by Microsoft, and of course, that projects (and other dependencies) declaring compatibility with a .NET Framework version are stating compatibility with all of the features included. (Watching Node-based projects talk about what they have to do to upgrade what dependencies is incredible.)

Documentation is, of course, tightly integrated with the the documentation for .NET as a whole as well, etc. And as my project currently is .NET Framework-based, of course, another huge benefit is not carrying much extra code with my program, as it comes with Windows.

In my personal code projects, I've set the general intention to solely use .NET Framework/Core as my sole "dependency" and eschew third party functionality as much as possible. In a few cases, this means I've reinvented the wheel (which has been educational!), but I've heavily relied on things like SmtpClient being taken care of for me. For an example, my project had to implement IMAP since .NET Framework doesn't. MailKit is larger than my entire project, and the rudimentary IMAP functionality I needed is only about three dozen lines of code. 10xing the size of my project and the scope of code I drag along with it for one function would be unfortunate.

I get a little bit of pride every time I can remove a NuGet package, so it's just a little disappointing that Microsoft is moving in the opposite direction. Just something I'm going to have to deal with as everything becomes irritatingly more like the JavaScript world.

(I'm gonna step off my soapbox now...)

@jstedfast
Copy link

@jstedfast jstedfast commented Oct 18, 2018

@ocdtrekkie Don't worry, I did not take any offense at your comment :-)

FWIW, I also like to use as few nugets as possible in my projects as well, so I can understand your position on that.

@sonic1981
Copy link

@sonic1981 sonic1981 commented Mar 14, 2019

It's been a year and no one seems to of fixed this?! o_O

@karelz
Copy link
Member

@karelz karelz commented Mar 14, 2019

@sonic1981 what kind of fix do you expect?
We do not plan to update source code. Is there anything else that should happen?

@ghost
Copy link

@ghost ghost commented Mar 14, 2019

@karelz Why would you not update the source code here? Having it marked obsolete in one official reference but not another is very confusing.

@joelmartinez
Copy link
Contributor

@joelmartinez joelmartinez commented Mar 14, 2019

Please note that the docs metadata now correctly reflects the fact that this attribute is only present on the version(s) of the BCL that ship with Xamarin/Mono:

https://github.com/dotnet/dotnet-api-docs/blob/master/xml/System.Net.Mail/SmtpClient.xml#L32-L34

The docs site is currently going through some behind-the-scenes updates, at which point it will display correctly on the website (however I don't have a specific ETA to share) ... so when you've got it filtered to .net framework 4.7 (or another framework/version that doesn't have it), it won't show this attribute.

@ghost
Copy link

@ghost ghost commented Mar 14, 2019

So to summarize:

  1. SmtpClient is considered obsolete in Xamarin/Mono. It has the Obsolete attribute in the source code and is documented as obsolete on Microsoft Docs.
  2. SmtpClient is not considered obsolete in .NET Standard. It does not have the Obsolete attribute in BCL or CoreFX source code, and is not documented as obsolete on Microsoft Docs.
  3. SmtpClient is considered obsolete by the Microsoft.DotNet.Analyzers.Compatibility analyzer, which is in preview and not guaranteed to be installed in all projects.

If all the above is true, then I still feel justified in calling this very confusing. @karelz Breaking builds is the risk you take when an API is marked obsolete. If you're willing to take that risk in Xamarin/Mono, why not in .NET Standard (especially for .NET Core where the .NET ecosystem is getting a nice fresh start)?

@karelz
Copy link
Member

@karelz karelz commented Mar 14, 2019

Xamarin/Mono has different compat bar than we have in .NET Framework and .NET Core.
I think we are arguing over details here. I agree that things are not perfect, but the bottom line is that SmtpClient is useful API only to certain extent. We do not plan to evolve it, which makes it legacy API (kind of obsolete).

If the Obsolete information is gone from documentation soon (per above comment), we should mention in the docs that the API is not modern and not high-perf and that we recommend use of other projects like MailKit for those purposes
Will that address your concerns?

@ghost
Copy link

@ghost ghost commented Mar 14, 2019

I think I see what you're saying. SmtpClient is not truly obsolete because it still has use cases. I.e., you are not trying to migrate all users to use MailKit for every emailing scenario. In that case, then yes I think a blurb in the docs that MailKit is better for modern/high-perf scenarios is sufficient.

However, speaking for myself, I don't want to learn two different mailing APIs; I would rather just learn the modern one that I know will work at scale. Marking SmtpClient as obsolete would give us an Intellisense warning about it. If the .NET devs truly feel that MailKit is a superior API (as they seem to given the Xamarin changes), then they should be trying to migrate all users away from SmtpClient. Just obsolete the old thing and add the new thing to the framework (as was done with Json.Net). Don't take half-measures. 😛

@TravisEz13
Copy link

@TravisEz13 TravisEz13 commented Mar 20, 2019

I saw no firm statement of if this is obsolete in DotNet Core.

For example, are there plans add support to SmtpClient in DotNet Core to Opportunistic TLS and in general keep it up to date?

@karelz
Copy link
Member

@karelz karelz commented Mar 20, 2019

@TravisEz13 no, we do not expect any modernization of the API or its implementation. See also link above: https://github.com/dotnet/docs/issues/1876#issuecomment-430805681 pointing to https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md

@soniczoo
Copy link

@soniczoo soniczoo commented Feb 9, 2020

If intellisense showed Obsolete, we probably wouldn’t have invested 100s of hours expanding our use of smtpclient recently, only to find an issue in one deployment. Then to see some rouge looking notes and a “here’s what’s popular” recommendation ( which will take 100s more hours of our teams time, if we can even get through the open source hoops and audits from our own and customer’s security teams/policies ) is a kick in the shorts.
I’m all for progress. But, pulling the plug on framework improvements / established classes is a slippery slope. On this situation microsoft is acting like the framework is used for exclusively new, practice / personal, or hosted / managed products. Folks that sell windows programs or service packages to other companies be damned.
My suggestion to save face on this one is similar to (to their credit) what was done with MSOLEDBSQL which was ”undepricated” after MS realized there are thousands of every day programs and probably billions of lines of code that need to work with tls 1.2.. fix it! - that’s what i have to do for my customers.

@mairaw
Copy link
Contributor

@mairaw mairaw commented Feb 9, 2020

@mimisasouvanh any updates on when we're going to be able to show attributes for the right version selected?

@migueldeicaza
Copy link
Contributor

@migueldeicaza migueldeicaza commented Feb 11, 2020

Folks, the story of how this attribute got added is worthy of a blog post.

That said, in a world where you want the best interop in the world, the most efficient encodings, the most efficient code, the comformance to the best protocols available at your disposal, you are not doing yourself or your users any favors by using SmtpClient. This is like embracing HTTP in an era that has moved to HTTPS.

@PashaPash
Copy link
Author

@PashaPash PashaPash commented Feb 11, 2020

@migueldeicaza what exactly is wrong with SmtpClient in terms of encoding support or protocols support? It does support TLS, it does support UTF-8, that covers 99% of common cases for most of the apps.

That is some kind of strange path of single person decision making. You put that attribute on a mono codebase, providing subjective "we strongly recommend ...", and apparently that "we" becomes official framework-wide decision. And that decision it still half-applied - class still not marked as obsolete in the framework code. It should be either marked obsolete in main .NET codebase, or unmarked as obsolete in docs. Just to avoid "100-hours invested" surprises.

The final warning in DE0005 is actually sounds like "SmtpClient is insecure. We have no standard, secure and reliable way to send mail in .NET anymore. Please use 3-rd party library with bus factor 1 and pray it will not be abandoned".

Already hit that "use 3-rd party" multiple times:

  • MS marked System.Web.Script.Serialization as obsolete and recommended 3-rd party JSON.NET instead. Ended up re-introducing built-in JSON serializer in .NET Core 3.
  • PushSharp was most efficient, community standard way to send push notifications in C#. Then author was hired by MS, and the project is now abandoned. That project is still mentioned by thousands of blogposts, and people still wasting time trying to plug in that "best" library just to find out it fails in production.

There is no difference between MailKit and PushSharp for me. They are both 3-rd party non-MS libraries with a single maintainer. The only difference is that MailKit is not abandoned (yet).
Official MS clone of MailKit could be a solution - at least it will mark MailKit as some kind of "official" library.

@svick
Copy link
Contributor

@svick svick commented Feb 11, 2020

@PashaPash

Please use 3-rd party library with bus factor 1 and pray it will not be abandoned

MailKit is part of the .NET Foundation, so if the maintainer decided to abandon it, somebody else should take their place.

(Edit: It turns out I already mentioned that before.)

@soniczoo
Copy link

@soniczoo soniczoo commented Feb 11, 2020

It would really be nice to know exactly what's wrong with system.net.mail.smtpclient but I cannot get/find a straight answer. This lack of info and professional courtesy looks bad for Microsoft and the developers of Mailkit (especially if they are affiliated).
Mailkit seems awesome in every way, and it works in the ONLY environment (of 100s) SMTPClient has had a problem in - thank god it's our own QA environment. But getting railroaded into 100s of hours of change-over without knowing why is unfair. And I hope policies change, but many people working in large companies don't even have the option to use open source - so we MUST know what is wrong with SMTPClient. Without that common courtesy, t-shooting is impossible, and a simple fixable config issue may result in all kinds of hurt.

@PashaPash
Copy link
Author

@PashaPash PashaPash commented Feb 11, 2020

MailKit is part of the .NET Foundation, so if the maintainer decided to abandon it, somebody else should take their place.

Not sure how dotnetfoundation.org membership implies change of project ownership. Dotnetfoundation is about technical services and marketing, not about code contribution.

There is a hope that a second contributor will pick up the project, but for MailKit jstedfast is the only major contributor. 2-nd contributor is Redth, the owner of now abandoned PushSharp. And he has only 9 commits. 3-rd is migueldeicaza, mostly documentation, 6 years ago. It is clear that project is owned and maintained by one person, and no chance someone else can quickly became a full grade maintainer.

@mimisasouvanh
Copy link

@mimisasouvanh mimisasouvanh commented Feb 11, 2020

@mairaw We will be rolling out the attribute versioning feature later this week.

@GF-Huang
Copy link

@GF-Huang GF-Huang commented Feb 28, 2020

So, what should I use now?

@migueldeicaza
Copy link
Contributor

@migueldeicaza migueldeicaza commented Mar 29, 2020

@migueldeicaza what exactly is wrong with SmtpClient in terms of encoding support or protocols support? It does support TLS, it does support UTF-8, that covers 99% of common cases for most of the apps.

Please read the landing page for MailKit to get an idea of how much is missing.

@soniczoo
Copy link

@soniczoo soniczoo commented Mar 29, 2020

The landing page that says smtpclient doesn’t, and mailkit does these things.. “cross-platform support and ability to save and re-load MIME messages before sending them”? That’s all I see.. not an explanation of issues with smtpclient. The whole point here is that Microsoft needs to support their “supported” software.. and I trust they will, they always have. Mailkit looks/works great. But business doesn’t care about that. It invests in software and platforms to make money. And business expects to have ample sunset warning, so it can plan its next investment. This healthy lifecycle is not happening in the case of System.Net.SmtpClient. Its like someone on a whim just yanked the plug on it.

@mairaw
Copy link
Contributor

@mairaw mairaw commented Mar 30, 2020

@mimisasouvanh @TianqiZhang the new feature does not display the wrong attribute anymore, but it continues to display the obsolete warning for all versions. That information also needs to be version/target aware.

@migueldeicaza
Copy link
Contributor

@migueldeicaza migueldeicaza commented Mar 30, 2020

Bad code and bad API design exists in the wild. When the world understands those problems, they move to new systems that prevent that.

Just like we moved away from SSL to TLS, and then to version 1.1 and 1.2 and now we are hoping to move to 1.3, there are mistakes in the past that can not be solved. This is one of those.

You should avoid using SmtpClient in my humble opinion, and do your users a favor in the process.

Or you could live dangerously and roll the dice.

@alex-jitbit
Copy link
Contributor

@alex-jitbit alex-jitbit commented May 18, 2020

@migueldeicaza you made a claim but then instead of proving it, you send everyone to read some page online.

The burden of proof is on the person who makes a claim

SmtpClient is still a robust working piece of code (tested on our SaaS app used by 1000s of companies).

MimeKit is indeed a fine, fast MIME parsing library for inbound email, that has SMTP as an addon.

@mwwhited
Copy link

@mwwhited mwwhited commented Jul 7, 2020

This is probably going to end up like json.net. An API that everyone love snit Microsoft things they can do better so they create the bastard that is text.json. Horrible library and attempt to blind port people leading to breaking changes everywhere.

@carlossanlop
Copy link
Member

@carlossanlop carlossanlop commented Sep 2, 2020

@mimisasouvanh what's the status of this?

@TianqiZhang
Copy link
Contributor

@TianqiZhang TianqiZhang commented Sep 10, 2020

@mimisasouvanh @mairaw now the page (https://docs.microsoft.com/en-us/dotnet/api/System.Net.Mail.SmtpClient?view=net-5.0) displays obsolete messages correctly, the warning will only show for 3 versions ( xamarinandroid-7.1 xamarinios-10.8 xamarinmac-3.0 ).

@mairaw
Copy link
Contributor

@mairaw mairaw commented Sep 12, 2020

Thanks @TianqiZhang. Closing this.

@mairaw mairaw closed this Sep 12, 2020
@mairaw mairaw removed the blocked label Sep 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet