smtpd
smtpd copied to clipboard
Don't log EOF error on sudden connection close
If a client closes a connection before the QUIT command has been issued an EOF error will be reported when logging has been enabled like so:
! command abort 0 bytes read err: EOF
Since there appear to be a lot of clients out there who behave like this, catch the io.EOF error and silently close it. The smtpd.ABORT event will still be sent when this happens if you want to catch these situations on the implementaton side of things.
I would prefer to take a variant of this change that makes optional both this error and another error that I frequently see in my logs:
reply abort: write tcp US:25->REMOTE:51240: write: broken pipe
I'm not sure where best to put the flag that controls whether these errors are reported. One plausible place is in smtpd.Config, or one could add an extra argument to smtpd.NewConn() (since the logger is already outside smtpd.Config). It's also possible that this should be two flags, one for EOF and one for the broken pipe error on replies (which happens when the client side just drops the connection on us as we're writing a message out).
Would such a variant meet your needs? Are you willing to write the code for it?
Hey Chris,
I'd be happy to supply the code for this. Adding an extra parameter to smtpd.NewConn() would break the API, so I'd advocate for one or two fields to smtpd.Config{} if you want to make it configurable.
Do you want ignoring these errors be the default, or should a developer opt-in for ignoring these errors? If it's the former, I'm thinking LogIOError bool for both or LogEOF bool and LogWriteError bool if you want it separate. SkipIOError, SkipEOF and SkipWriteError if you want the opt-in version.
What do you think?
You raise a good point about API compatibility, so adding fields to smtpd.Config{} it is. In the same spirit of API compatibility, I think it should be opt-in. For future expansion, the approach I'd prefer is a field of flags, something like NoLogFlags (or LogFlags, with the initial flags being NoEOFError and NoWriteError plus a combined NoIOError that has both). Does this make sense?