Merge pull request #416 from wneessen/bug/414_fixed-boundary-messes-u…

…p-multipart-rendering

Refactor multipart boundary handling in message rendering

msg.go

@@ -120,7 +120,7 @@ type Msg struct {
 	// This count can be helpful to identify where the mail header ends and the mail body starts
 	headerCount uint
 
-	// isDelivered indicates wether the Msg has been delivered.
+	// isDelivered indicates whether the Msg has been delivered.
 	isDelivered bool
 
 	// middlewares is a slice of Middleware used for modifying or handling messages before they are processed.
@@ -131,6 +131,10 @@ type Msg struct {
 	// mimever represents the MIME version used in a Msg.
 	mimever MIMEVersion
 
+	// multiPartBoundary holds the rendered boundary strings for consistent boundary rendering
+	// in case a Msg is rendered several times
+	multiPartBoundary map[MIMEType]string
+
 	// parts is a slice that holds pointers to Part structures, which represent different parts of a Msg.
 	parts []*Part
 
@@ -183,12 +187,13 @@ type MsgOption func(*Msg)
 //   - https://datatracker.ietf.org/doc/html/rfc5321
 func NewMsg(opts ...MsgOption) *Msg {
 	msg := &Msg{
-		addrHeader:    make(map[AddrHeader][]*mail.Address),
-		charset:       CharsetUTF8,
-		encoding:      EncodingQP,
-		genHeader:     make(map[Header][]string),
-		preformHeader: make(map[Header]string),
-		mimever:       MIME10,
+		addrHeader:        make(map[AddrHeader][]*mail.Address),
+		charset:           CharsetUTF8,
+		encoding:          EncodingQP,
+		genHeader:         make(map[Header][]string),
+		preformHeader:     make(map[Header]string),
+		multiPartBoundary: make(map[MIMEType]string),
+		mimever:           MIME10,
 	}
 
 	// Override defaults with optionally provided MsgOption functions.
@@ -273,9 +278,10 @@ func WithMIMEVersion(version MIMEVersion) MsgOption {
 // WithBoundary sets the boundary of a Msg to the provided string value during its creation or
 // initialization.
 //
-// Note that by default, random MIME boundaries are created. This option should only be used if
-// a specific boundary is required for the email message. Using a predefined boundary can be
-// helpful when constructing multipart messages with specific formatting or content separation.
+// NOTE: By default, random MIME boundaries are created. This option should only be used if
+// a specific boundary is required for the email message. Using a predefined boundary will only
+// work with messages that hold a single multipart part. Using a predefined boundary with several
+// multipart parts will render the mail unreadable to the mail client.
 //
 // Parameters:
 //   - boundary: The string value that specifies the desired boundary for the Msg.
@@ -378,10 +384,12 @@ func (m *Msg) SetEncoding(encoding Encoding) {
 //
 // This method allows you to specify a custom boundary string for the MIME message. The
 // boundary is used to separate different parts of the message, especially when dealing
-// with multipart messages. By default, the Msg generates random MIME boundaries. This
-// function should only be used if you have a specific boundary requirement for the
-// message. Ensure that the boundary value does not conflict with any content within the
-// message to avoid parsing errors.
+// with multipart messages.
+//
+// NOTE: By default, random MIME boundaries are created. This option should only be used if
+// a specific boundary is required for the email message. Using a predefined boundary will only
+// work with messages that hold a single multipart part. Using a predefined boundary with several
+// multipart parts will render the mail unreadable to the mail client.
 //
 // Parameters:
 //   - boundary: The string value representing the boundary to set for the Msg, used in
@@ -1535,6 +1543,10 @@ func (m *Msg) GetAttachments() []*File {
 // particularly in multipart emails. The boundary helps to differentiate between various sections
 // such as plain text, HTML content, and attachments.
 //
+// NOTE: By default, random MIME boundaries are created. Using a predefined boundary will only
+// work with messages that hold a single multipart part. Using a predefined boundary with several
+// multipart parts will render the mail unreadable to the mail client.
+//
 // Returns:
 //   - A string representing the boundary of the message.
 //
@@ -3064,17 +3076,6 @@ func (m *Msg) signMessage() error {
 	// the S/MIME signature
 	buf := bytes.NewBuffer(nil)
 	mw := &msgWriter{writer: buf, charset: m.charset, encoder: m.encoder}
-
-	// If no boundary is set by the user, we set a fixed random boundary, so that
-	// the boundary of the parts is consistant during the different rendering
-	// phases
-	if m.boundary == "" {
-		boundary, err := randomBoundary()
-		if err != nil {
-			return fmt.Errorf("failed to generate random boundary: %w", err)
-		}
-		m.SetBoundary(boundary)
-	}
 	mw.writeMsg(m)
 
 	// Since we only want to sign the message body, we need to find the position within

msg_test.go

@@ -5932,11 +5932,11 @@ func TestMsg_WriteTo(t *testing.T) {
 		buffer := bytes.NewBuffer(nil)
 		_, err = message.WriteTo(buffer)
 		if err == nil {
-			t.Error("expected WritoTo with invalid S/MIME private key to fail")
+			t.Error("expected WriteTo with invalid S/MIME private key to fail")
 		}
-		expErr := "failed to generate random boundary: failed to read from rand.Reader: broken reader"
-		if !strings.EqualFold(err.Error(), expErr) {
-			t.Errorf("expected S/MIME signing error to be: %s, got: %s", expErr, err)
+		expErr := "broken reader"
+		if !strings.Contains(err.Error(), expErr) {
+			t.Errorf("expected S/MIME signing error to contain: %q, got: %s", expErr, err)
 		}
 	})
 }
@@ -7040,29 +7040,6 @@ func TestMsg_signMessage(t *testing.T) {
 			t.Errorf("SMIME signing with invalid header count is expected to fail with %s, but got: %s", expErr, err)
 		}
 	})
-	t.Run("signing fails with broken rand.Reader", func(t *testing.T) {
-		defaultRandReader := rand.Reader
-		t.Cleanup(func() { rand.Reader = defaultRandReader })
-		rand.Reader = &randReader{failon: 1}
-
-		keypair, err := getDummyKeyPairTLS()
-		if err != nil {
-			t.Fatalf("failed to load dummy crypto material: %s", err)
-		}
-		msg := testMessage(t)
-		if err = msg.SignWithTLSCertificate(keypair); err != nil {
-			t.Fatalf("failed to init SMIME configuration: %s", err)
-		}
-		msg.headerCount = 1000
-		err = msg.signMessage()
-		if err == nil {
-			t.Error("SMIME signing with broken rand.Reader is expected to fail")
-		}
-		expErr := "failed to generate random boundary: failed to read from rand.Reader: broken reader"
-		if !strings.EqualFold(err.Error(), expErr) {
-			t.Errorf("SMIME signing with broken rand.Reader is expected to fail with %s, but got: %s", expErr, err)
-		}
-	})
 }
 
 // uppercaseMiddleware is a middleware type that transforms the subject to uppercase.

msgwriter.go

@@ -57,7 +57,7 @@ type msgWriter struct {
 	depth           int8
 	encoder         mime.WordEncoder
 	err             error
-	multiPartWriter [3]*multipart.Writer
+	multiPartWriter [4]*multipart.Writer
 	partWriter      io.Writer
 	writer          io.Writer
 }
@@ -140,19 +140,25 @@ func (mw *msgWriter) writeMsg(msg *Msg) {
 		mw.writeString(DoubleNewLine)
 	}
 	if msg.hasMixed() {
-		mw.startMP(MIMEMixed, msg.boundary)
+		boundary := mw.getMultipartBoundary(msg, MIMEMixed)
+		boundary = mw.startMP(MIMEMixed, boundary)
+		msg.multiPartBoundary[MIMEMixed] = boundary
 		if mw.depth == 1 {
 			mw.writeString(DoubleNewLine)
 		}
 	}
 	if msg.hasRelated() {
-		mw.startMP(MIMERelated, msg.boundary)
+		boundary := mw.getMultipartBoundary(msg, MIMERelated)
+		boundary = mw.startMP(MIMERelated, boundary)
+		msg.multiPartBoundary[MIMERelated] = boundary
 		if mw.depth == 1 {
 			mw.writeString(DoubleNewLine)
 		}
 	}
 	if msg.hasAlt() {
-		mw.startMP(MIMEAlternative, msg.boundary)
+		boundary := mw.getMultipartBoundary(msg, MIMEAlternative)
+		boundary = mw.startMP(MIMEAlternative, boundary)
+		msg.multiPartBoundary[MIMEAlternative] = boundary
 		if mw.depth == 1 {
 			mw.writeString(DoubleNewLine)
 		}
@@ -247,9 +253,12 @@ func (mw *msgWriter) writePreformattedGenHeader(msg *Msg) {
 //   - mimeType: The MIME type of the multipart content (e.g., "mixed", "alternative").
 //   - boundary: The boundary string separating different parts of the multipart message.
 //
+// Returns:
+//   - The multipart boundary string
+//
 // References:
 //   - https://datatracker.ietf.org/doc/html/rfc2046
-func (mw *msgWriter) startMP(mimeType MIMEType, boundary string) {
+func (mw *msgWriter) startMP(mimeType MIMEType, boundary string) string {
 	multiPartWriter := multipart.NewWriter(mw)
 	if boundary != "" {
 		mw.err = multiPartWriter.SetBoundary(boundary)
@@ -266,6 +275,7 @@ func (mw *msgWriter) startMP(mimeType MIMEType, boundary string) {
 		mw.newPart(map[string][]string{"Content-Type": {contentType}})
 	}
 	mw.depth++
+	return multiPartWriter.Boundary()
 }
 
 // stopMP closes the multipart.
@@ -279,6 +289,28 @@ func (mw *msgWriter) stopMP() {
 	}
 }
 
+// getMultipartBoundary returns the appropriate multipart boundary for the given MIME type.
+//
+// If the Msg has a predefined boundary, it is returned. Otherwise, the function checks
+// for a MIME type-specific boundary in the Msg's multiPartBoundary map. If no boundary
+// is found, an empty string is returned.
+//
+// Parameters:
+//   - msg: A pointer to the Msg containing the boundary and MIME type-specific mappings.
+//   - mimetype: The MIMEType for which the boundary is being determined.
+//
+// Returns:
+//   - A string representing the multipart boundary, or an empty string if none is found.
+func (mw *msgWriter) getMultipartBoundary(msg *Msg, mimetype MIMEType) string {
+	if msg.boundary != "" {
+		return msg.boundary
+	}
+	if msg.multiPartBoundary[mimetype] != "" {
+		return msg.multiPartBoundary[mimetype]
+	}
+	return ""
+}
+
 // addFiles adds the attachments/embeds file content to the mail body.
 //
 // This function iterates through the list of files, setting necessary headers for each file,