Merge pull request #4 from d--j/testability

Introduce interface mailfilter.Trx and a testing implementation testtrx.Trx

README.md

@@ -54,23 +54,23 @@ func main() {
 
   // create and start the mail filter
   mailFilter, err := mailfilter.New(protocol, address,
-    func(_ context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
-      // Reject message when it was sent to our SPAM trap
+    func(_ context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
+      // Quarantine mail when it is addressed to our SPAM trap
       if trx.HasRcptTo("spam-trap@スパム.example.com") {
-        return mailfilter.CustomErrorResponse(550, "5.7.1 No thank you"), nil
+        return mailfilter.QuarantineResponse("train as spam"), nil
       }
       // Prefix subject with [⚠️EXTERNAL] when user is not logged in
-      if trx.MailFrom.AuthenticatedUser() == "" {
-        subject, _ := trx.Headers.Subject()
+      if trx.MailFrom().AuthenticatedUser() == "" {
+        subject, _ := trx.Headers().Subject()
         if !strings.HasPrefix(subject, "[⚠️EXTERNAL] ") {
           subject = "[⚠️EXTERNAL] " + subject
         }
-        trx.Headers.SetSubject(subject)
+        trx.Headers().SetSubject(subject)
       }
       return mailfilter.Accept, nil
     },
-    // optimization: we do not need the body of the message for our decision
-    mailfilter.WithoutBody(),
+    // optimization: call the decision function when all headers were sent to us
+    mailfilter.WithDecisionAt(mailfilter.DecisionAtEndOfHeaders),
   )
   if err != nil {
     log.Fatal(err)

integration/README.md

@@ -104,7 +104,7 @@ import (
 
 func main() {
 	integration.RequiredTags("auth-plain", "auth-no", "tls-starttls", "tls-no")
-	integration.Test(func(ctx context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
+	integration.Test(func(ctx context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
 		return mailfilter.CustomErrorResponse(501, "Test"), nil
 	}, mailfilter.WithDecisionAt(mailfilter.DecisionAtMailFrom))
 }

integration/tests/auth/test.go

@@ -9,11 +9,11 @@ import (
 
 func main() {
 	integration.RequiredTags("auth-plain", "auth-no", "tls-starttls", "tls-no")
-	integration.Test(func(ctx context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
-		if trx.Helo.TlsVersion == "" {
+	integration.Test(func(ctx context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
+		if trx.Helo().TlsVersion == "" {
 			return mailfilter.CustomErrorResponse(500, "No starttls"), nil
 		}
-		if trx.MailFrom.AuthenticatedUser() == "[email protected]" {
+		if trx.MailFrom().AuthenticatedUser() == "[email protected]" {
 			return mailfilter.CustomErrorResponse(502, "Ok"), nil
 		}
 		return mailfilter.CustomErrorResponse(501, "No authentication"), nil

integration/tests/body/test.go

@@ -10,8 +10,8 @@ import (
 )
 
 func main() {
-	integration.Test(func(ctx context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
-		switch trx.MailFrom.Addr {
+	integration.Test(func(ctx context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
+		switch trx.MailFrom().Addr {
 		case "[email protected]":
 			b, err := io.ReadAll(trx.Body())
 			if err != nil {

integration/tests/header/test.go

@@ -10,18 +10,18 @@ import (
 )
 
 func main() {
-	integration.Test(func(ctx context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
-		switch trx.MailFrom.Addr {
+	integration.Test(func(ctx context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
+		switch trx.MailFrom().Addr {
 		case "[email protected]":
-			trx.Headers.Add("X-ADD1", "Test")
-			trx.Headers.Add("X-ADD2", "Test")
+			trx.Headers().Add("X-ADD1", "Test")
+			trx.Headers().Add("X-ADD2", "Test")
 		case "[email protected]":
-			f := trx.Headers.Fields()
+			f := trx.Headers().Fields()
 			f.Next()
 			f.InsertBefore("X-First1", "Test")
 			f.InsertBefore("X-First2", "Test")
 		case "[email protected]":
-			f := trx.Headers.Fields()
+			f := trx.Headers().Fields()
 			for f.Next() {
 				if f.CanonicalKey() == "Subject" {
 					f.InsertBefore("X-Middle1", "Test")
@@ -30,17 +30,17 @@ func main() {
 				}
 			}
 		case "[email protected]":
-			trx.Headers.SetSubject("changed")
+			trx.Headers().SetSubject("changed")
 		case "[email protected]":
-			f := trx.Headers.Fields()
+			f := trx.Headers().Fields()
 			for f.Next() {
 				if f.CanonicalKey() == "Subject" {
 					f.Del()
 					break
 				}
 			}
 		case "[email protected]":
-			f := trx.Headers.Fields()
+			f := trx.Headers().Fields()
 			first := true
 			for f.Next() {
 				if first {
@@ -55,13 +55,13 @@ func main() {
 					f.InsertBefore("X-Before-DATE", "Test")
 				}
 			}
-			trx.Headers.Add("X-ADD1", "Test")
-			trx.Headers.Add("X-ADD2", "Test")
+			trx.Headers().Add("X-ADD1", "Test")
+			trx.Headers().Add("X-ADD2", "Test")
 		default:
 			return mailfilter.CustomErrorResponse(500, "unknown mail from"), nil
 		}
-		b, _ := io.ReadAll(trx.Headers.Reader())
-		log.Printf("from %s header %q", trx.MailFrom.Addr, string(b))
+		b, _ := io.ReadAll(trx.Headers().Reader())
+		log.Printf("from %s header %q", trx.MailFrom().Addr, string(b))
 		return mailfilter.Accept, nil
 	})
 }

integration/tests/mail-from/test.go

@@ -8,28 +8,25 @@ import (
 )
 
 func main() {
-	integration.Test(func(ctx context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
-		if trx.MailFrom.Addr == "[email protected]" {
+	integration.Test(func(ctx context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
+		if trx.MailFrom().Addr == "[email protected]" {
 			return mailfilter.TempFail, nil
 		}
-		if trx.MailFrom.Addr == "[email protected]" {
+		if trx.MailFrom().Addr == "[email protected]" {
 			return mailfilter.Reject, nil
 		}
-		if trx.MailFrom.Addr == "[email protected]" {
+		if trx.MailFrom().Addr == "[email protected]" {
 			return mailfilter.Discard, nil
 		}
-		if trx.MailFrom.Addr == "[email protected]" {
+		if trx.MailFrom().Addr == "[email protected]" {
 			return mailfilter.CustomErrorResponse(555, "custom"), nil
 		}
-		if trx.MailFrom.Addr == "[email protected]" {
+		if trx.MailFrom().Addr == "[email protected]" {
 			return mailfilter.QuarantineResponse("test"), nil
 		}
-		if trx.MailFrom.Addr == "[email protected]" {
-			trx.MailFrom.Addr = "[email protected]"
-			// Sendmail might break when you do not null this out
-			if trx.MTA.IsSendmail() {
-				trx.MailFrom.Args = ""
-			}
+		if trx.MailFrom().Addr == "[email protected]" {
+			// Sendmail might break when you pass something to esmtpArgs
+			trx.ChangeMailFrom("[email protected]", "")
 		}
 		return mailfilter.Accept, nil
 	}, mailfilter.WithDecisionAt(mailfilter.DecisionAtMailFrom))

integration/tests/rcpt-to/test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func main() {
-	integration.Test(func(ctx context.Context, trx *mailfilter.Transaction) (mailfilter.Decision, error) {
+	integration.Test(func(ctx context.Context, trx mailfilter.Trx) (mailfilter.Decision, error) {
 		if trx.HasRcptTo("[email protected]") {
 			return mailfilter.TempFail, nil
 		}

internal/header/diff.go

@@ -0,0 +1,182 @@
+package header
+
+import "bytes"
+
+const (
+	KindEqual = iota
+	KindChange
+	KindInsert
+)
+
+type fieldDiff struct {
+	kind  int
+	field *Field
+	index int
+}
+
+func diffFieldsMiddle(orig []*Field, changed []*Field, index int) (diffs []fieldDiff) {
+	// either orig and changed are empty or the first element is different
+	origLen, changedLen := len(orig), len(changed)
+	changedI := 0
+	switch {
+	case origLen == 0 && changedLen == 0:
+		return nil
+	case origLen == 0:
+		// orig empty -> everything must be inserts
+		for _, c := range changed {
+			diffs = append(diffs, fieldDiff{KindInsert, c, index})
+		}
+		return
+	case changedLen == 0:
+		// This should not happen since we do not delete headerField entries
+		// but if the user completely replaces the headers it could indeed happen.
+		// Panic in this case so the programming error surfaces.
+		panic("internal structure error: do not completely replace transaction.Headers – use its methods to alter it")
+	default: // origLen > 0 && changedLen > 0
+		o := orig[0]
+		if o.Index < 0 {
+			panic("internal structure error: all elements in orig need to have an index bigger than -1: do not completely replace transaction.Headers – use its methods to alter it")
+		}
+		// find o.index in changed
+		for i, c := range changed {
+			if c.Index == o.Index {
+				index = o.Index
+				changedI = i
+				for i = 0; i < changedI; i++ {
+					diffs = append(diffs, fieldDiff{KindInsert, changed[i], index - 1})
+				}
+				if bytes.Equal(changed[changedI].Raw, o.Raw) {
+					diffs = append(diffs, fieldDiff{KindEqual, o, o.Index})
+				} else if changed[changedI].Key() == o.Key() {
+					diffs = append(diffs, fieldDiff{KindChange, changed[changedI], o.Index})
+				} else {
+					// a HeaderFields.Replace call, delete the original
+					diffs = append(diffs, fieldDiff{
+						kind: KindChange,
+						field: &Field{
+							Index:        o.Index,
+							CanonicalKey: o.CanonicalKey,
+							Raw:          []byte(o.Key() + ":"),
+						},
+						index: o.Index,
+					})
+					// insert changed in front of deleted header
+					diffs = append(diffs, fieldDiff{KindInsert, &Field{
+						Index:        -1,
+						CanonicalKey: changed[changedI].CanonicalKey,
+						Raw:          changed[changedI].Raw,
+					}, index})
+					index-- // in this special case we actually do not need to increase the index below
+				}
+				changedI++
+				break
+			} else if c.Index > o.Index {
+				panic("internal structure error: index of original was not found in changed: do not completely replace transaction.Headers – use its methods to alter it")
+			}
+		}
+		// we only consumed the first element of orig
+		index++
+		restDiffs := diffFields(orig[1:], changed[changedI:], index)
+		if len(restDiffs) > 0 {
+			diffs = append(diffs, restDiffs...)
+		}
+		return
+	}
+}
+
+func diffFields(orig []*Field, changed []*Field, index int) (diffs []fieldDiff) {
+	origLen, changedLen := len(orig), len(changed)
+	// find common prefix
+	commonPrefixLen, commonSuffixLen := 0, 0
+	for i := 0; i < origLen && i < changedLen; i++ {
+		if !bytes.Equal(orig[i].Raw, changed[i].Raw) || orig[i].Index != changed[i].Index {
+			break
+		}
+		commonPrefixLen += 1
+		index = orig[i].Index
+	}
+	// find common suffix (down to the commonPrefixLen element)
+	i, j := origLen-1, changedLen-1
+	for i > commonPrefixLen-1 && j > commonPrefixLen-1 {
+		if !bytes.Equal(orig[i].Raw, changed[j].Raw) || orig[i].Index != changed[j].Index {
+			break
+		}
+		commonSuffixLen += 1
+		i--
+		j--
+	}
+	for i := 0; i < commonPrefixLen; i++ {
+		diffs = append(diffs, fieldDiff{KindEqual, orig[i], orig[i].Index})
+	}
+	// find the changed parts, recursively calls diffFields afterwards
+	middleDiffs := diffFieldsMiddle(orig[commonPrefixLen:origLen-commonSuffixLen], changed[commonPrefixLen:changedLen-commonSuffixLen], index)
+	if len(middleDiffs) > 0 {
+		diffs = append(diffs, middleDiffs...)
+	}
+	for i := origLen - commonSuffixLen; i < origLen; i++ {
+		diffs = append(diffs, fieldDiff{KindEqual, orig[i], orig[i].Index})
+	}
+	return
+}
+
+type Op struct {
+	Kind  int
+	Index int
+	Name  string
+	Value string
+}
+
+// Diff finds differences between orig and changed.
+// The differences are expressed as change and insert operations – to be mapped to milter modification actions.
+// Deletions are changes to an empty value.
+func Diff(orig *Header, changed *Header) (changeInsertOps []Op, addOps []Op) {
+	origFields := orig.Fields()
+	origLen := origFields.Len()
+	origIndexByKeyCounter := make(map[string]int)
+	origIndexByKey := make([]int, origLen)
+	for i := 0; origFields.Next(); i++ {
+		origIndexByKeyCounter[origFields.CanonicalKey()] += 1
+		origIndexByKey[i] = origIndexByKeyCounter[origFields.CanonicalKey()]
+	}
+	diffs := diffFields(orig.fields, changed.fields, -1)
+	for _, diff := range diffs {
+		switch diff.kind {
+		case KindInsert:
+			idx := diff.index + 1
+			if idx > 0 {
+				idx += 1
+			}
+			if idx-1 >= origLen {
+				addOps = append(addOps, Op{
+					Index: idx,
+					Name:  diff.field.Key(),
+					Value: diff.field.Value(),
+				})
+			} else {
+				changeInsertOps = append(changeInsertOps, Op{
+					Kind:  KindInsert,
+					Index: idx,
+					Name:  diff.field.Key(),
+					Value: diff.field.Value(),
+				})
+			}
+		case KindChange:
+			if diff.index < origLen {
+				changeInsertOps = append(changeInsertOps, Op{
+					Kind:  KindChange,
+					Index: origIndexByKey[diff.index],
+					Name:  diff.field.Key(),
+					Value: diff.field.Value(),
+				})
+			} else { // should not happen but just make adds out of it
+				addOps = append(addOps, Op{
+					Index: diff.index + 1,
+					Name:  diff.field.Key(),
+					Value: diff.field.Value(),
+				})
+			}
+		}
+	}
+
+	return
+}