aerc: send: prevent sending if multipart conversion failed v1 APPLIED

Vitaly Ovchinnikov: 1
 send: prevent sending if multipart conversion failed

 3 files changed, 35 insertions(+), 6 deletions(-)

Bence Ferdinandy, Apr 13, 2024 at 20:55:

[PATCH aerc] send: prevent sending if multipart conversion failed Export this patch

Add ConversionError field to Composer.Part to track multipart conversion
errors.

Check for conversion errors in :send, block sending if the errors are
found.

Signed-off-by: Vitaly Ovchinnikov <v@ovch.ru>
---

The patch helps with the configurations like this:

binds.conf:
	[compose::review]
	y = :multipart text/html<Enter>:send<Enter>

aerc.conf:
	[multipart-converters]
	text/html = /path/to/some/script.sh

/path/to/some/script.sh:
	#!/bin/sh
	exit 10 # falls for some reason

Without the patch, when you press `y` aerc runs `:multipart` command and
although it gets an error from the converter script, the error is
ignored and then the `:send` command actually sends a broken message.

The patch stores conversion errors for the message parts and checks for
them in `:send`. If the error is found, the sending is blocked and the
user gets an error message.

There is no way to skip this like missing attachment or empty subject.
This is done intentionally, as this is a problem of different level to
me. The user needs to update or delete the problem part before actually
sending a message.

Robin Jarry <robin@jarry.cc>

11 months ago

Acked-by: Robin Jarry <robin@jarry.cc>

Applied, thanks.

To git@git.sr.ht:~rjarry/aerc
   4cbd116f0348..65332c233880  master -> master

 app/compose.go           | 29 ++++++++++++++++++++++++++---
 commands/compose/send.go |  5 +++++
 lib/attachment.go        |  7 ++++---
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/app/compose.go b/app/compose.go
index 469f3c36..eae77e65 100644
--- a/app/compose.go
+++ b/app/compose.go
@@ -1012,6 +1012,22 @@ func (c *Composer) ShouldWarnSubject() bool {
 	return len(subject) == 0
 }
 
+func (c *Composer) CheckForMultipartErrors() error {
+	problems := []string{}
+	for _, p := range c.textParts {
+		if p.ConversionError != nil {
+			text := fmt.Sprintf("%s: %s", p.MimeType, p.ConversionError.Error())
+			problems = append(problems, text)
+		}
+	}
+
+	if len(problems) == 0 {
+		return nil
+	}
+
+	return fmt.Errorf("multipart conversion error: %s", strings.Join(problems, "; "))
+}
+
 func writeMsgImpl(c *Composer, header *mail.Header, writer io.Writer) error {
 	mimeParams := map[string]string{"Charset": "UTF-8"}
 	if config.Compose.FormatFlowed {
@@ -1761,16 +1777,23 @@ func newReviewMessage(composer *Composer, err error) *reviewMessage {
 }
 
 func (c *Composer) updateMultipart(p *lib.Part) error {
+	// conversion errors handling
+	p.ConversionError = nil
+	setError := func(e error) error {
+		p.ConversionError = e
+		return e
+	}
+
 	command, found := config.Converters[p.MimeType]
 	if !found {
 		// unreachable
-		return fmt.Errorf("no command defined for mime/type")
+		return setError(fmt.Errorf("no command defined for mime/type"))
 	}
 	// reset part body to avoid it leaving outdated if the command fails
 	p.Data = nil
 	body, err := c.GetBody()
 	if err != nil {
-		return errors.Wrap(err, "GetBody")
+		return setError(errors.Wrap(err, "GetBody"))
 	}
 	cmd := exec.Command("sh", "-c", command)
 	cmd.Stdin = body
@@ -1786,7 +1809,7 @@ func (c *Composer) updateMultipart(p *lib.Part) error {
 				stderr = fmt.Sprintf(": %.30s", stderr)
 			}
 		}
-		return fmt.Errorf("%s: %w%s", command, err, stderr)
+		return setError(fmt.Errorf("%s: %w%s", command, err, stderr))
 	}
 	p.Data = out
 	return nil
diff --git a/commands/compose/send.go b/commands/compose/send.go
index 2e7590a0..fd39b48c 100644
--- a/commands/compose/send.go
+++ b/commands/compose/send.go
@@ -64,6 +64,11 @@ func (s Send) Execute(args []string) error {
 	}
 	composer, _ := tab.Content.(*app.Composer)
 
+	err := composer.CheckForMultipartErrors()
+	if err != nil {
+		return err
+	}
+
 	config := composer.Config()
 
 	if s.CopyTo == "" {
diff --git a/lib/attachment.go b/lib/attachment.go
index aa3cd108..6f29da55 100644
--- a/lib/attachment.go
+++ b/lib/attachment.go
@@ -16,9 +16,10 @@ import (
 )
 
 type Part struct {
-	MimeType string
-	Params   map[string]string
-	Data     []byte
+	MimeType        string
+	Params          map[string]string
+	Data            []byte
+	ConversionError error
 }
 
 func NewPart(mimetype string, params map[string]string, body io.Reader,
-- 
2.43.0

builds.sr.ht <builds@sr.ht>

11 months ago

aerc/patches: SUCCESS in 1m57s

[send: prevent sending if multipart conversion failed][0] from [Vitaly Ovchinnikov][1]

[0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/50704
[1]: mailto:v@ovch.ru

✓ #1186055 SUCCESS aerc/patches/openbsd.yml     https://builds.sr.ht/~rjarry/job/1186055
✓ #1186054 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/1186054

Robin Jarry <robin@jarry.cc>

11 months ago

Vitaly Ovchinnikov <v@ovch.ru> wrote:

inwit <inwit@sindominio.net>

11 months ago

Unfortunately, this patch breaks my workflow: I used to :accept calendar invitations and, in the review screen, I used to get an error in red saying "text/calendar error: no command defined for mime/type", but still I was able to send the message (and thus accept the invitation).

After this patch, I can't :accept invitations anymore. :(