From 63cef8e3a59bd09dbca863854bec7d40c444715e Mon Sep 17 00:00:00 2001
From: Mechiel Lukkien <mechiel@ueber.net>
Date: Sat, 9 Mar 2024 09:51:24 +0100
Subject: [PATCH] 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.
---
 queue/queue.go | 8 ++++++++
 webmail/api.go | 9 +++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/queue/queue.go b/queue/queue.go
index efc842a..e464f9f 100644
--- a/queue/queue.go
+++ b/queue/queue.go
@@ -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 {
diff --git a/webmail/api.go b/webmail/api.go
index 23f6503..5a5d3c1 100644
--- a/webmail/api.go
+++ b/webmail/api.go
@@ -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
@@ -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.
@@ -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")