From aca9a7bc53d68b38dc3803cec24bfdcb9c7a8b2a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 22 Dec 2021 17:35:51 +0000 Subject: [PATCH] Add option to convert CRLF to LF line endings for sendmail It appears that several versions of sendmail require that the mail is sent to them with LF line endings instead of CRLF endings - which of course they will then convert back to CRLF line endings to comply with the SMTP standard. This PR adds another setting SENDMAIL_CONVERT_CRLF which will pass the message writer through a filter. This will filter out and convert CRLFs to LFs before writing them out to sendmail. Fix #18024 Signed-off-by: Andrew Thornton --- custom/conf/app.example.ini | 3 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + modules/setting/mailer.go | 12 +-- services/mailer/mailer.go | 80 ++++++++++++++++++- services/mailer/mailer_test.go | 42 ++++++++++ 5 files changed, 131 insertions(+), 7 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 1d19a343804f..987e84a0d8cc 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1494,6 +1494,9 @@ PATH = ;; ;; Timeout for Sendmail ;SENDMAIL_TIMEOUT = 5m +;; +;; convert \r\n to \n for Sendmail +;SENDMAIL_CONVERT_CRLF = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 07655a181b1e..00816964af49 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -667,6 +667,7 @@ Define allowed algorithms and their minimum key length (use -1 to disable a type command or full path). - `SENDMAIL_ARGS`: **_empty_**: Specify any extra sendmail arguments. - `SENDMAIL_TIMEOUT`: **5m**: default timeout for sending email through sendmail +- `SENDMAIL_CONVERT_CRLF`: **false**: some versions of sendmail require LF line endings rather than CRLF line endings. Set this to true if you require this. - `SEND_BUFFER_LEN`: **100**: Buffer length of mailing queue. **DEPRECATED** use `LENGTH` in `[queue.mailer]` ## Cache (`cache`) diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index 1bcd63a914bd..19570ea979b2 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -37,9 +37,10 @@ type Mailer struct { IsTLSEnabled bool // Sendmail sender - SendmailPath string - SendmailArgs []string - SendmailTimeout time.Duration + SendmailPath string + SendmailArgs []string + SendmailTimeout time.Duration + SendmailConvertCRLF bool } var ( @@ -71,8 +72,9 @@ func newMailService() { IsTLSEnabled: sec.Key("IS_TLS_ENABLED").MustBool(), SubjectPrefix: sec.Key("SUBJECT_PREFIX").MustString(""), - SendmailPath: sec.Key("SENDMAIL_PATH").MustString("sendmail"), - SendmailTimeout: sec.Key("SENDMAIL_TIMEOUT").MustDuration(5 * time.Minute), + SendmailPath: sec.Key("SENDMAIL_PATH").MustString("sendmail"), + SendmailTimeout: sec.Key("SENDMAIL_TIMEOUT").MustDuration(5 * time.Minute), + SendmailConvertCRLF: sec.Key("SENDMAIL_CONVERT_CRLF").MustBool(false), } MailService.From = sec.Key("FROM").MustString(MailService.User) MailService.EnvelopeFrom = sec.Key("ENVELOPE_FROM").MustString("") diff --git a/services/mailer/mailer.go b/services/mailer/mailer.go index eac2b15c3c2c..70bcbca8320c 100644 --- a/services/mailer/mailer.go +++ b/services/mailer/mailer.go @@ -253,6 +253,73 @@ func (s *smtpSender) Send(from string, to []string, msg io.WriterTo) error { return client.Quit() } +type crlfConverter struct { + danglingCR bool + w io.Writer +} + +func (c *crlfConverter) Write(bs []byte) (n int, err error) { + if len(bs) == 0 { + if c.danglingCR { + _, err := c.w.Write([]byte{'\r'}) + if err != nil { + return 0, err + } + c.danglingCR = false + } + return c.w.Write(bs) + } + if c.danglingCR { + if bs[0] != '\n' { + _, err := c.w.Write([]byte{'\r'}) + if err != nil { + return 0, err + } + } + c.danglingCR = false + } + if bs[len(bs)-1] == '\r' { + c.danglingCR = true + bs = bs[:len(bs)-1] + } + idx := bytes.Index(bs, []byte{'\r', '\n'}) + for idx >= 0 { + count, err := c.w.Write(bs[:idx]) + n += count + if err != nil { + return n, err + } + count, err = c.w.Write([]byte{'\n'}) + if count == 1 { + n += 2 + } + if err != nil { + return n, err + } + bs = bs[idx+2:] + idx = bytes.Index(bs, []byte{'\r', '\n'}) + } + if len(bs) > 0 { + count, err := c.w.Write(bs) + n += count + if err != nil { + return n, err + } + } + if c.danglingCR { + n++ + } + return +} + +func (c *crlfConverter) Close() (err error) { + if c.danglingCR { + _, err = c.w.Write([]byte{'\r'}) + c.danglingCR = false + } + return +} + // Sender sendmail mail sender type sendmailSender struct { } @@ -290,13 +357,22 @@ func (s *sendmailSender) Send(from string, to []string, msg io.WriterTo) error { return err } - _, err = msg.WriteTo(pipe) + if setting.MailService.SendmailConvertCRLF { + converter := &crlfConverter{ + w: pipe, + } + _, err = msg.WriteTo(converter) + if err == nil { + err = converter.Close() + } + } else { + _, err = msg.WriteTo(pipe) + } // we MUST close the pipe or sendmail will hang waiting for more of the message // Also we should wait on our sendmail command even if something fails closeError = pipe.Close() waitError = cmd.Wait() - if err != nil { return err } else if closeError != nil { diff --git a/services/mailer/mailer_test.go b/services/mailer/mailer_test.go index 56f2eb52b0bc..08e149801eaa 100644 --- a/services/mailer/mailer_test.go +++ b/services/mailer/mailer_test.go @@ -5,6 +5,7 @@ package mailer import ( + "strings" "testing" "time" @@ -37,3 +38,44 @@ func TestGenerateMessageID(t *testing.T) { gm = m.ToMessage() assert.Equal(t, "", gm.GetHeader("Message-ID")[0]) } + +func TestCRLFConverter(t *testing.T) { + type testcaseType struct { + input []string + expected string + } + testcases := []testcaseType{ + { + input: []string{"This h\ras a \r", "\nnewline\r\n"}, + expected: "This h\ras a \nnewline\n", + }, + { + input: []string{"This\r\n has a \r\n\r", "\n\r\nnewline\r\n"}, + expected: "This\n has a \n\n\nnewline\n", + }, + { + input: []string{"This has a \r", "\nnewline\r"}, + expected: "This has a \nnewline\r", + }, + { + input: []string{"This has a \r", "newline\r"}, + expected: "This has a \rnewline\r", + }, + } + for _, testcase := range testcases { + out := &strings.Builder{} + converter := &crlfConverter{w: out} + realsum, sum := 0, 0 + for _, in := range testcase.input { + n, err := converter.Write([]byte(in)) + assert.NoError(t, err) + assert.Equal(t, len(in), n) + sum += n + realsum += len(in) + } + err := converter.Close() + assert.NoError(t, err) + assert.Equal(t, realsum, sum) + assert.Equal(t, testcase.expected, out.String()) + } +}