send: prevent sending if multipart conversion failed

Given the following configuration:

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

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.

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

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

There is no way to skip this like missing attachment or empty subject.
This is done intentionally. The user needs to update or delete the
problematic part before actually sending a message.

Signed-off-by: Vitaly Ovchinnikov <v@ovch.ru>
Acked-by: Robin Jarry <robin@jarry.cc>

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,