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

Fix #9151 - smtp logger configuration sendTos should be an array #9154

Merged
merged 4 commits into from
Nov 25, 2019
Merged

Fix #9151 - smtp logger configuration sendTos should be an array #9154

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d83fb9
Fix #9151 - sendTos should be an array
zeripath Nov 25, 2019
36b36ad
Merge branch 'master' into fix-9151
zeripath Nov 25, 2019
67427b0
trimspace from the addresses
zeripath Nov 25, 2019
6c317bc
Merge branch 'fix-9151' of github.com:zeripath/gitea into fix-9151
zeripath Nov 25, 2019
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
2 changes: 1 addition & 1 deletion modules/setting/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func generateLogConfig(sec *ini.Section, name string, defaults defaultLogOptions
logConfig["username"] = sec.Key("USER").MustString("example@example.com")
logConfig["password"] = sec.Key("PASSWD").MustString("******")
logConfig["host"] = sec.Key("HOST").MustString("127.0.0.1:25")
logConfig["sendTos"] = sec.Key("RECEIVERS").MustString("[]")
logConfig["sendTos"] = strings.Split(sec.Key("RECEIVERS").MustString(""), ",")
Copy link
Member

Choose a reason for hiding this comment

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

What about a regexp? That could suport several kinds of syntaxes. What is it that we expect here? The standard full-name format for an e-mail is:
"John Snow" <john@thewall.com>. Entries are usually separated by ; but , could also be supported.

Anyway, we should validate the addresses and avoid using anything that will make the server fail.

Validation, however is something really tricky.

Copy link
Contributor Author

@zeripath zeripath Nov 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Validating email addresses is extremely non-trivial as just about the only thing you have to have is @ - possibly not even that depending on your local SMTP server config.

Even splitting potential lists of them by "," is incorrect as "," can be in there too.

In any case it can't blow up the server - SendMail will simply return an error and any error whilst logging an event just ends being printed blandly to stdout.

See modules/log/event.go

Copy link
Member

Choose a reason for hiding this comment

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

Mmm.... not even TrimSpace() on each value? Anyway, this is a closed list, so admins should be responsible and use whatever Gitea supports (i.e. not e-mails with , in it 😄).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've added the TrimSpace. If you start looking too hard we'll have to fix the:

gitea/modules/log/smtp.go

Line 84 in 802aa6d

mailmsg := []byte("To: " + strings.Join(log.RecipientAddresses, ";") + "\r\nFrom: " + log.Username + "<" + log.Username +

And make it do proper quoting of email addresses - but I think that could be done in a different PR.

logConfig["subject"] = sec.Key("SUBJECT").MustString("Diagnostic message from Gitea")
}

Expand Down