server: DATA command timeout should close the connection
- Dominant language
- Go
- Stars
- 2.1k
- Forks
- 274
- Avg merge
- 6d 22h
- Merged PRs (30d)
- 1
Description
Basically go-smtp issues two timeouts, configured by `ReadTimeout`: for DATA command, for the idle connection:
```
354 2.0.0 Go ahead. End your data with .
# doing nothing in DATA command
421 4.0.0 read tcp 127.0.0.1:1025->127.0.0.1:56290: i/o timeout # timeout triggered after X time
221 2.4.2 Idle timeout, bye bye # another timeout triggered after another X time
# TCP connection is closed
```
For instance AWS SES closes the connection on data timeout:
```
354 End data with .
# doing nothing in DATA command
451 4.4.2 Timeout waiting for data from client.
# TCP connection is closed
```
See [RFC5321](https://www.rfc-editor.org/rfc/rfc5321.html#section-4.2.2) `451 Requested action aborted: error in processing`.
Not closing the connection on DATA timeout causes a slow postfix client talking to go-smtp server to send the data even, when the go-smtp server responds with `421 4.0.0` and the DATA's leftover data is considered as a mangled SMTP command in go-smtp server.
I suggest to catch the DATA timeout exception and close the connection, e.g.
```diff
--- a/conn.go
+++ b/conn.go
@@ -673,18 +673,25 @@ func (c *Conn) handleData(arg string) {
// We have recipients, go to accept data
c.WriteResponse(354, EnhancedCode{2, 0, 0}, "Go ahead. End your data with .")
- defer c.reset()
-
if c.server.LMTP {
c.handleDataLMTP()
+ c.reset()
return
}
r := newDataReader(c)
- code, enhancedCode, msg := toSMTPStatus(c.Session().Data(r))
+ err := c.Session().Data(r)
+ code, enhancedCode, msg := toSMTPStatus(err)
r.limited = false
io.Copy(ioutil.Discard, r) // Make sure all the data has been consumed
c.WriteResponse(code, enhancedCode, msg)
+
+ c.reset()
+
+ // close a connection on data command timeout
+ if err == ErrDataTimeout {
+ c.Close()
+ }
}
func (c *Conn) handleBdat(arg string) {
--- a/data.go
+++ b/data.go
@@ -3,6 +3,7 @@ package smtp
import (
"bufio"
"io"
+ "net"
)
type EnhancedCode [3]int
@@ -42,6 +43,12 @@ var ErrDataTooLarge = &SMTPError{
Message: "Maximum message size exceeded",
}
+var ErrDataTimeout = &SMTPError{
+ Code: 451,
+ EnhancedCode: EnhancedCode{4, 4, 2},
+ Message: "Timeout waiting for data from client",
+}
+
type dataReader struct {
r *bufio.Reader
state int
@@ -93,6 +100,9 @@ func (r *dataReader) Read(b []byte) (n int, err error) {
if err == io.EOF {
err = io.ErrUnexpectedEOF
}
+ if e, ok := err.(net.Error); ok && e.Timeout() {
+ err = ErrDataTimeout
+ }
break
}
switch r.state {
```
Ideally it would be better to preserve the original err inside the SMTPError struct, e.g. implement some kind of err wrapping like it's done with https://go.dev/blog/go1.13-errors#errors-in-go-113
Contributor guide
Research direction
Start in conn.go at Conn.handleData and in data.go at dataReader.Read; trace how read timeouts become SMTP responses and how the connection is reset. Done means a DATA timeout returns the 451 response and closes the connection, while normal and LMTP handling retain their existing behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100