Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add option to convert CRLF to LF line endings for sendmail #18075

Merged
merged 7 commits into from
Jan 6, 2022
Merged

Add option to convert CRLF to LF line endings for sendmail #18075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aca9a7b
Add option to convert CRLF to LF line endings for sendmail
zeripath Dec 22, 2021
b5287ee
flip the meaning of SENDMAIL_CONVERT_CRLF
zeripath Dec 22, 2021
b61cafe
Use Replacer as code is easier to understand
zeripath Dec 26, 2021
39327ec
remove old test
zeripath Dec 26, 2021
ff4012f
Merge branch 'main' into sendmail-filter-crlf
6543 Jan 4, 2022
6ed273a
Merge branch 'main' into sendmail-filter-crlf
6543 Jan 5, 2022
c42c903
Merge branch 'main' into sendmail-filter-crlf
zeripath Jan 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions custom/conf/app.example.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,9 @@ PATH =
;;
;; Timeout for Sendmail
;SENDMAIL_TIMEOUT = 5m
;;
;; convert \r\n to \n for Sendmail
;SENDMAIL_CONVERT_CRLF = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any requirement to convert \n to \r\n ?

Copy link
Contributor

@wxiaoguang wxiaoguang Dec 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the gomail package always output CRLF for EOL. So there is no case for \n to \r\n.

The key problem is the unmaintained gomail package now. If we could maintain it by ourselves or find a replacement, we would have a better solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So let's just fork and fix that upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not going to be a solution for 1.16.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not going to be a solution for 1.16.

I see your thought. Can you help to explain the reason behind it (why it is not a solution for 1.16)? Thanks ~

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go-gomail/gomail#142

uff the only fork is https://github.com/go-mail/mail (go-gomail/gomail#108)

but now also stale as a owner kicked out the other ... who want to do heavy refactoring ( looks like they had two different ideas)

so YES we would have to fork it :X


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`: **true**: Most versions of sendmail prefer LF line endings rather than CRLF line endings. Set this to false if your version of sendmail requires CRLF line endings.
- `SEND_BUFFER_LEN`: **100**: Buffer length of mailing queue. **DEPRECATED** use `LENGTH` in `[queue.mailer]`

## Cache (`cache`)
Expand Down
12 changes: 7 additions & 5 deletions modules/setting/mailer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(true),
}
MailService.From = sec.Key("FROM").MustString(MailService.User)
MailService.EnvelopeFrom = sec.Key("ENVELOPE_FROM").MustString("")
Expand Down
11 changes: 9 additions & 2 deletions services/mailer/mailer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,20 @@ func (s *sendmailSender) Send(from string, to []string, msg io.WriterTo) error {
return err
}

_, err = msg.WriteTo(pipe)
if setting.MailService.SendmailConvertCRLF {
buf := &strings.Builder{}
_, err = msg.WriteTo(buf)
if err == nil {
_, err = strings.NewReplacer("\r\n", "\n").WriteString(pipe, buf.String())
}
} 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 {
Expand Down