Skip to content

Commit

Permalink
webmail: fix for ignoring error about sending to invalid address
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mjl- committed Mar 9, 2024
1 parent c57aeac commit 63cef8e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
8 changes: 8 additions & 0 deletions queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,14 @@ func Add(ctx context.Context, log mlog.Log, senderAccount string, msgFile *os.Fi
if qm.ID != 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 {
Expand Down
9 changes: 5 additions & 4 deletions webmail/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ type File struct {
func parseAddress(msghdr string) (message.NameAddress, error) {
a, err := mail.ParseAddress(msghdr)
if err != nil {
return message.NameAddress{}, nil
return message.NameAddress{}, err
}

// todo: parse more fully according to ../rfc/5322:959
Expand Down Expand Up @@ -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)) {
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")

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.
Expand Down Expand Up @@ -657,7 +657,8 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
}
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()
}
xcheckf(ctx, err, "adding messages to the delivery queue")
Expand Down

0 comments on commit 63cef8e

Please sign in to comment.