search

wneessen / go-mail

📧 Easy to use, yet comprehensive library for sending mails with Go
https://go-mail.dev
MIT License
571 stars 44 forks source link

Rework TLSPortPolicy() #181

Closed wneessen closed 5 months ago

wneessen commented 6 months ago 
          This change makes no sense to me? It hard-codes port to 25 for non-TLS connections, it seems to me? There is no way to set `fallbackPort`?

Previous behavior made more sense to me. With TLSOpportunistic, I want to connect to port X, if it supports STARTTLS, use TLS, otherwise, use the same port, but just do not use TLS. Currently, if I use SetTLSPortPolicy(TLSOpportunistic), if the server does not support STARTTLS on port X, it tries to connect to port 25 on that server - which might not even be available (or accessible - firewalls and stuff).

I do not get issue #105 either. Is this about behavior if one does not call WithPort? I think if WithPort is not made, then default should be to connect to port 25 (or 587?) and try STARTTLS on it, if it fails and TLSOpportunistic is set, use no TLS on same port. The behavior to try different ports makes no sense to me. You use one port, but potentially negotiate different configuration of how you use it. And WithPort is then used to configure which port is the one the client uses.

Luckily, WithTLSPolicy still works and if I use WithTLSPolicy it is exactly how I would want. Maybe just remove deprecation on it?

But I do think that connecting to multiple ports is a strange approach. That might be done by some higher-level library to auto-detect configuration of an unknown SMTP server. But not by a low-level library like this one where you generally know the server you are connecting to.

Originally posted by @mitar in https://github.com/wneessen/go-mail/issues/170#issuecomment-1965595454

9214 commented 6 months ago

Hey. Just wanted to chime in and note that current client configuration behavior is order-dependent. Consider:

options := []mail.Option{
  mail.WithTLSPortPolicy(mail.NoTLS),
  mail.WithPort(1025),
}
sender, err := mail.NewClient("localhost", options...)

This works as intended (trying to connect to localhost:1025), because WithTLSPortPolicy(mail.NoTLS) sets port to 25, and then WithPort rewrites it back to 1025. But if you swap the options around it obviously fails.

wneessen commented 6 months ago

That's good information to take into consideration. Thanks @9214