mbox: add unit tests and patchwork signature

Add unit tests for the Format and SanitizeFilename functions
covering headers, body structure, postscript handling, comment tag
extraction, list From replacement and mbox envelope framing.

Append a signature block (-- \npatchwork $version) after the diff
when no signature is already present. Always insert a --- separator
before the diff even when there is no postscript section.

Remove the CRLF normalization hack since go-mbox canonicalizes
line endings in both directions: the Writer normalizes to LF for
storage, the Reader normalizes to CRLF for parsing.

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

Author: rjarry

cmd/pw/http/main.go

@@ -20,6 +20,7 @@ import (
 	"github.com/getpatchwork/patchwork/pkg/db/migrations"
 	"github.com/getpatchwork/patchwork/pkg/events"
 	"github.com/getpatchwork/patchwork/pkg/log"
+	"github.com/getpatchwork/patchwork/pkg/mbox"
 	"github.com/getpatchwork/patchwork/pkg/web"
 )
 
@@ -37,6 +38,8 @@ func (c *CLI) Run(ctx *pw.Context) error {
 	bus := events.Start(ctx, ctx.DB)
 	defer bus.Shutdown()
 
+	mbox.Version = ctx.Version
+
 	router := web.NewRouter(ctx.Config, ctx.DB, bus, ctx.Version)
 	router.Mount("/", api.NewRouter(ctx.Config, ctx.DB, ctx.Config.Http.BaseURL, bus))
 

pkg/mbox/mbox.go

@@ -9,7 +9,6 @@ import (
 	"bufio"
 	"bytes"
 	"context"
-	"fmt"
 	"regexp"
 	"strconv"
 	"strings"
@@ -24,9 +23,12 @@ import (
 	"github.com/getpatchwork/patchwork/pkg/log"
 )
 
+var Version string
+
 var (
 	postscriptRe   = regexp.MustCompile(`(?m)^-{2,3} ?$`)
 	mboxResponseRe = regexp.MustCompile(`(?mi)^(Tested|Reviewed|Acked|Signed-off|Nacked|Reported)-by:.*$`)
+	signatureRe    = regexp.MustCompile(`(?m)^-- $`)
 	safeFilenameRe = regexp.MustCompile(`[^a-z0-9_.-]+`)
 )
 
@@ -111,10 +113,11 @@ func Format(sub Submission) []byte {
 		body += "\n" + sub.Diff
 	}
 
-	sc := bufio.NewScanner(strings.NewReader(body))
-	for sc.Scan() {
-		fmt.Fprintf(w, "%s\r\n", sc.Text())
+	if Version != "" && !signatureRe.MatchString(body) {
+		body += "-- \npatchwork " + Version + "\n"
 	}
+
+	_, _ = w.Write([]byte(body))
 	mw.Close()
 
 	return buf.Bytes()

pkg/mbox/mbox_test.go

@@ -96,19 +96,13 @@ func TestFormatBasicPatch(t *testing.T) {
 	}
 }
 
-func TestFormatCRLF(t *testing.T) {
+func TestFormatParseable(t *testing.T) {
 	sub := baseSubmission()
 	out := Format(sub)
 
-	_, body := parseMbox(t, out)
-	for i, line := range strings.Split(body, "\n") {
-		if line == "" {
-			continue
-		}
-		if !strings.HasSuffix(line, "\r") {
-			t.Errorf("line %d not CRLF terminated: %q", i+1, line)
-			break
-		}
+	mr := gombox.NewReader(bytes.NewReader(out))
+	if _, err := mr.NextMessage(); err != nil {
+		t.Errorf("output not parseable as mbox: %v", err)
 	}
 }
 
@@ -244,6 +238,48 @@ func TestFormatMboxEnvelope(t *testing.T) {
 	}
 }
 
+func TestFormatSignature(t *testing.T) {
+	Version = "4.0.0"
+	defer func() { Version = "" }()
+
+	sub := baseSubmission()
+	out := Format(sub)
+
+	_, body := parseMbox(t, out)
+	body = strings.ReplaceAll(body, "\r\n", "\n")
+	if !strings.Contains(body, "-- \npatchwork 4.0.0\n") {
+		t.Errorf("body missing signature, got:\n%q", body)
+	}
+}
+
+func TestFormatNoSignatureWhenPresent(t *testing.T) {
+	Version = "4.0.0"
+	defer func() { Version = "" }()
+
+	sub := baseSubmission()
+	sub.Diff = "diff --git a/f b/f\n-- \nsome tool 1.0\n"
+	out := Format(sub)
+
+	_, body := parseMbox(t, out)
+	body = strings.ReplaceAll(body, "\r\n", "\n")
+	if strings.Contains(body, "patchwork 4.0.0") {
+		t.Error("should not add signature when one already exists")
+	}
+}
+
+func TestFormatNoSignatureWhenVersionEmpty(t *testing.T) {
+	Version = ""
+
+	sub := baseSubmission()
+	out := Format(sub)
+
+	_, body := parseMbox(t, out)
+	body = strings.ReplaceAll(body, "\r\n", "\n")
+	if strings.Contains(body, "-- \n") {
+		t.Error("should not add signature when Version is empty")
+	}
+}
+
 func TestSanitizeFilename(t *testing.T) {
 	tests := []struct {
 		input string