jstedfast / MimeKit

A .NET MIME creation and parser library with support for S/MIME, PGP, DKIM, TNEF and Unix mbox spools.
http://www.mimekit.net
MIT License
1.82k stars 370 forks source link

Split MimeKit into MimeKit[.Core] and MimeKit.Cryptography #820

Open wrall opened 2 years ago

wrall commented 2 years ago

Is your feature request related to a problem? Please describe. Right now, different projects can end up with weird conflicts when one dependency uses MimeKitLite and another uses MimeKit. When this happens, there can be an error about not being sure which version to use.

Describe the solution you'd like It would be conceptually simpler to have MimeKit simply depend on MimeKitLite and add the additional functionality on top of it. Of course this would probably require a major version change.

Describe alternatives you've considered The workaround at the moment is to use aliased references that allows you to select which reference to pull a type from. Another dev approach would be to use a different namespaces, but that is far uglier.

jstedfast commented 2 years ago

What projects are you talking about, specifically?

I would argue that those projects should be using MimeKit always and not MimeKitLite.

ggmueller commented 12 months ago

To add context: I just had the problem, that Wiremock.net added the dependency to MimekitLite to parse emails. I was using Mailkit to send emails. I think I can downgrade to Lite, but I will see.

Here is also a Wiremock.net issue: https://github.com/WireMock-Net/WireMock.Net/issues/990

jstedfast commented 12 months ago

This has been on my radar for a while now but I don't know of a good way to do it that wouldn't require the user manually register the fact that they have the crypto stuff available to the MimeParser.

The MimeVisitor class would also need changes because right now, MimeKitLite's MimeVisitor API is not identical to the MimeVisitor API in MimeKit.

MimeMessage also has ties to the crypto APIs that are conditionally built depending on MimeKit vs MimeKitLite build configuration.

ggmueller commented 9 months ago

Also been thinking a while of this, reading a bit into the code. Not quite sure of everything though.

At first glance, it looks to me the biggest issue is the ParserOptions.Default field. As the default options would need to be different when CRYPTO is enabled.

I guess it would be doable to dynamically load a known assembly by name and call an additional static initializer method there that would additionally register the additional mimeparts, eg via the existing RegisterMimeType method. If the Crypto assembly is not available, it would skip this initialization.

It could also set some kind of FeatureFlag or something on the ParserOptions indicating if Crypto is available.

Then MimeParser and MimeVisitor could maybe make a runtime decision if they support Crypto.

Would this make sense?

jstedfast commented 9 months ago

Yea, that's more or less what I've also concluded.

MimeParser isn't a problem - it uses ParserOptions to instantiate the various MIME type classes.

MimeVisitor is/was the biggest problem because MimeKitLite does not have any of the crypto classes like ApplicationPkcs7Mime or MultipartEncrypted or MultipartSigned and obviously that is not so easy to make work via reflection or any sort of runtime decision making.

Suneeh commented 9 months ago

Having the same issue. I am using StrongGrid v0.102.0 as well as MailKit v4.3.0 and I get conflicts everywhere now.

jstedfast commented 9 months ago

I'll put this on the roadmap for 5.0 but I don't have definite plans for that yet.

In the meantime, there is also a MailKitLite if that helps.

Or ask the developers of StrongGrid to publish a version built against MimeKit instead of MimeKitLite. That would be a LOT easier to do than rearchitect MimeKit.

jstedfast commented 9 months ago

What I would propose is that we completely get rid of MimeKitLite (mostly I hate the naming) and either:

  1. Replace MimeKitLite with a MimeKit.Core assembly/package, move the classes that depend on BouncyCastle into a MimeKit.Cryptography assembly and create a dummy MimeKit nuget that can be used to get both MimeKit.Core and MimeKit.Cryptography.
  2. Kill off MimeKitLite, keep MimeKit (as the equivalent of the Core library) and move the classes that depend on BouncyCastle into MimeKit.Cryptography.

Either way, MimeKitLite ceases to exist.

The next question is: should Microsoft.Extensions.DependencyInjection be used? Or should stuff like CryptographyContext.Register() continue to be the way the SecureMimeContext and OpenPgpContext get registered?

The problem with the .Register() approach is that we'd need something similar for DKIM/ARC.

So much of what exists in MimeKit.Cryptography is deeply tangled with BouncyCastle which is going to make this split extremely painful.

I already have some local branches that have started:

This is going to be more difficult than I thought 😦

jstedfast commented 5 months ago

The mess of interfaces is too much to handle. I think it might be time to consider an alternative option.

I'm thinking now that the MimePart subclasses need to stay in MimeKit itself (or MimeKit.Core, depending on naming conventions) while any code that touches BouncyCastle would move into a new MimeKit.Cryptography assembly.

Unfortunately, many of the MultipartEncrypted/Signed methods are actually static methods that create the multipart instances, so I'm not sure what to do about that. I'd have to somehow make them "instance" methods (actually extension methods).

Ugh.