Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor multipart boundary handling in message rendering #416

Merged
merged 6 commits into from
Jan 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 26 additions & 25 deletions msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -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
Expand Down
31 changes: 4 additions & 27 deletions msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 37 additions & 5 deletions msgwriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand All @@ -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,
Expand Down
Loading