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

zeripath · 2019-11-25T11:38:40Z

zeripath commented

2019-11-25 11:38:40 +00:00

The settings code for managing smtp configuration is incorrect and should generate an array of email addresses. This fixes this problem.

Fix #9151

The settings code for managing smtp configuration is incorrect and should generate an array of email addresses. This fixes this problem. Fix #9151

guillep2k (Migrated from github.com) reviewed 2019-11-25 11:57:12 +00:00

modules/setting/log.go

guillep2k (Migrated from github.com) commented

2019-11-25 11:57:12 +00:00

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.

~~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](https://medium.com/hackernoon/the-100-correct-way-to-validate-email-addresses-7c4818f24643).

zeripath reviewed 2019-11-25 12:20:48 +00:00

modules/setting/log.go

zeripath commented

2019-11-25 12:20:48 +00:00

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

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

guillep2k (Migrated from github.com) reviewed 2019-11-25 12:31:35 +00:00

modules/setting/log.go

guillep2k (Migrated from github.com) commented

2019-11-25 12:31:34 +00:00

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 ?).

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 ?).

zeripath reviewed 2019-11-25 12:42:30 +00:00

modules/setting/log.go

zeripath commented

2019-11-25 12:42:30 +00:00

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

802aa6d5f3/modules/log/smtp.go (L84)

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

OK, I've added the TrimSpace. If you start looking too hard we'll have to fix the: https://github.com/go-gitea/gitea/blob/802aa6d5f306ade08438dc07d93913daecb1d3ab/modules/log/smtp.go#L84 And make it do proper quoting of email addresses - but I think that could be done in a different PR.