webmail: fix for ignoring error about sending to invalid address

before, an error about an invalid address was not used, causing a delivery
attempt to an empty address (empty localpart/domain). delivery to that address
would fail, but we should've prevented that message from being queued at all.

additionally, an error in adding the message to the queue was ignored too.
This commit is contained in:
Mechiel Lukkien 2024-03-09 09:51:24 +01:00
parent c57aeac7f0
commit 63cef8e3a5
No known key found for this signature in database
2 changed files with 13 additions and 4 deletions

View file

@ -253,6 +253,14 @@ func Add(ctx context.Context, log mlog.Log, senderAccount string, msgFile *os.Fi
if qm.ID != 0 { if qm.ID != 0 {
return fmt.Errorf("id of queued messages must be 0") return fmt.Errorf("id of queued messages must be 0")
} }
if qm.RecipientDomainStr == "" {
return fmt.Errorf("recipient domain cannot be empty")
}
// Sanity check, internal consistency.
rcptDom := formatIPDomain(qm.RecipientDomain)
if qm.RecipientDomainStr != rcptDom {
return fmt.Errorf("mismatch between recipient domain and string form of domain")
}
} }
if Localserve { if Localserve {

View file

@ -226,7 +226,7 @@ type File struct {
func parseAddress(msghdr string) (message.NameAddress, error) { func parseAddress(msghdr string) (message.NameAddress, error) {
a, err := mail.ParseAddress(msghdr) a, err := mail.ParseAddress(msghdr)
if err != nil { if err != nil {
return message.NameAddress{}, nil return message.NameAddress{}, err
} }
// todo: parse more fully according to ../rfc/5322:959 // todo: parse more fully according to ../rfc/5322:959
@ -341,12 +341,12 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
} }
if err != nil && (errors.Is(err, mox.ErrAccountNotFound) || errors.Is(err, mox.ErrDomainNotFound)) { if err != nil && (errors.Is(err, mox.ErrAccountNotFound) || errors.Is(err, mox.ErrDomainNotFound)) {
metricSubmission.WithLabelValues("badfrom").Inc() metricSubmission.WithLabelValues("badfrom").Inc()
xcheckuserf(ctx, errors.New("address not found"), "looking from address for account") xcheckuserf(ctx, errors.New("address not found"), `looking up "from" address for account`)
} }
xcheckf(ctx, err, "checking if from address is allowed") xcheckf(ctx, err, "checking if from address is allowed")
if len(recipients) == 0 { if len(recipients) == 0 {
xcheckuserf(ctx, fmt.Errorf("no recipients"), "composing message") xcheckuserf(ctx, errors.New("no recipients"), "composing message")
} }
// Check outgoing message rate limit. // Check outgoing message rate limit.
@ -657,7 +657,8 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
} }
qml[i] = qm qml[i] = qm
} }
if err := queue.Add(ctx, log, reqInfo.AccountName, dataFile, qml...); err != nil { err = queue.Add(ctx, log, reqInfo.AccountName, dataFile, qml...)
if err != nil {
metricSubmission.WithLabelValues("queueerror").Inc() metricSubmission.WithLabelValues("queueerror").Inc()
} }
xcheckf(ctx, err, "adding messages to the delivery queue") xcheckf(ctx, err, "adding messages to the delivery queue")