emersion / go-smtp

📤 An SMTP client & server library written in Go
MIT License
1.65k stars 212 forks source link

Custom greeting and DATA response #131

Open iredmail opened 3 years ago

iredmail commented 3 years ago
  1. Methods of Session interface have to return smtp.SMTPError{...} to customize returned smtp code and message. Although it's ok to set smtp code 250 for normal exit with SMTPError{...}, but will the logic of handling SMTPError change internally in go-smtp someday? Should we separate a normal exit (with custom return message) and error (e.g. SMTPResponse and existing SMTPError)? Should we expose something like conn.WriteResponse()?
  2. No way to customize the greeting text. We'd like to customize it and display our product name. https://github.com/emersion/go-smtp/blob/master/conn.go#L937

    How about add a new attribute like Banner in Server struct for this purpose, with default value set to current fmt.Sprintf("%v ESMTP Service Ready", c.server.Domain). PR #132 fixes this.

    Postfix MTA uses parameter smtpd_banner for this greeting text, so i think Banner should be fine.

emersion commented 3 years ago

Just don't try customize messages, they aren't shown anywhere to the user.

iredmail commented 3 years ago

How about the first request, easier / clearer SMTP response?

emersion commented 3 years ago

This also sounds like customization.

iredmail commented 3 years ago

But why doesn't allow developers to customize SMTP response?

Reasonable? :)

emersion commented 3 years ago

In both of these cases you don't reply 250, right?

iredmail commented 3 years ago

In both of these cases you don't reply 250, right?

Yes, and 4xx, 5xx smtp code should end the smtp session.

emersion commented 3 years ago

If you're hitting an error, returning an SMTPError is fine. If you're not hitting an error, return a nil error.

iredmail commented 3 years ago

We return 4xx/5xx on purpose. For example:

  1. Access control. we don't allow this sender server to contact us.
  2. Relied resource error. e.g. can not save message on disk (disk is full), SQL operation failed and return 4xx to ask sender server to retry.

For first case, it's not an "error" like "disk is full", but we still need to reject the session.

My suggestion is: Add a new struct SMTPReply, which is same as SMTPError. The word "Error" in struct name SMTPError is some kind of confusing, and it introduces a concern that the internal process to handle an error may be not what developers want. We handle a lot errors in Golang, but this SMTPError is just a normal smtp reply to end the session.

foxcpp commented 3 years ago
  1. SMTPError is treated just like any other error, even if 2xx code is used in it.
  2. Customizing non-failure responses is not supported and will not be - it is just not worth the hassle.
  3. I'm not strictly against customizing the initial response ("server banner"). Adding Server.Banner that would replace "ESMTP Service Ready" is not a big deal, isn't it? CC @emersion.
  4. Adding a special error type to terminate the connection may be a good idea. This could be used in policy mechanisms to get rid of malicious connections as fast as possible.