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

Occasionally Missing "Content-Disposition: attachment" and Line 1 is "Content-Type: multipart/mixed" instead of Date #240

Open
jameshueston opened this issue Jun 4, 2024 · 11 comments
Assignees
Labels
bug Something isn't working in-progress

Comments

@jameshueston
Copy link

Description

200 EML files were written to disk from uniquely separate IoT devices running the same code over the last several months.
198 were "Good" - opened in Email clients from users on separate domains, viewed attachments & body.
2 were "Bad", meaning

  1. they do not show attachments in Email Clients, and
  2. their body shows multiple equal signs throughout.

The outcome is the same across 3 Email Clients:

  • Microsoft Outlook Desktop version on Windows 11
  • Microsoft Office.com Web version in Chrome Browser
  • Thunderbird Mozilla Desktop on Windows 11

"Good" emails:

  • Line 1 is the Date
  • After "To", has Content-Type: multipart/mixed;
  • After Body's opening boundary, has Content-Transfer-Encoding: quoted-printable
  • 1st attachment has:
    Content-Disposition: attachment; filename="######-XXXXXXXXXXX_XXXXXXXX_XXXXXXXX_MonthEndReport-XXX_X-##.xlsx"
    Content-Transfer-Encoding: base64
    Content-Type: application/octet-stream; name="######-XXXXXXXXXXX_XXXXXXXX_XXXXXXXX_MonthEndReport-XXX_X-##.xlsx"
    
  • 2nd attachment has:
    Content-Disposition: attachment; filename="######-XXXXXXXXXXX_XXXXXXXX_XXXXXXXX_MonthEndReport-XXX_X-##.json"
    Content-Transfer-Encoding: base64
    Content-Type: application/json; name="######-XXXXXXXXXXX_XXXXXXXX_XXXXXXXX_MonthEndReport-XXX_X-##.json"
    

"Bad" emails:

  • Line 1 is Content-Type: multipart/mixed;
  • Line 2 is boundary=[60_character_alpha_numeric] Note this boundary only appears once in the file on Line 2
  • Line 3 is the Date
  • After "To", has Content-Type: multipart/alternative;
  • After Body's opening boundary, has Content-Transfer-Encoding: without a value
  • 1st Attachment has:
    Content-Transfer-Encoding: base64
    Content-Type: application/octet-stream; charset=
    
  • 2nd Attachment has:
    Content-Transfer-Encoding: base64
    Content-Type: application/json; charset=
    
  • Note attachments are missing Content-Disposition: attachment; filename="######-XXX...

All emails (Good and Bad) were built the same - fields REDACTED / refactored for clarity:

msg := mail.NewMsg()
_ = msg.From("REDACTED@REDACTED.com")
_ = msg.To(REDACTEDConfig.Mail.ToAddresses...)   // which is []string
msg.Subject(getMailSubjectForREDACTED(...))
msg.SetBodyString(getMailBodyForREDACTED(o))

// Attach XLSX file and JSON file
for i := range filepaths {
	msg.AttachFile(filepaths[i], mail.WithFileName(filenames[i]))
}

...

func getMailBodyForREDACTED(o *REDACTED) (ct mail.ContentType, body string) {
	ct = mail.TypeTextPlain

	body = body + "Attached is the REDACTED." + mail.SingleNewLine
	body = body + mail.SingleNewLine
	body = body + "REDACTEDID: " + o.REDACTEDID + mail.SingleNewLine
	...
	// 40 lines total lines building the body as one long string
	
	return
}

To Reproduce

I haven't been able to reproduce this locally yet.

Expected behaviour

All emails would be consistently generated under the "Good" section above.

Screenshots

No response

Attempted Fixes

I haven't done this yet -- and since I haven't been able to recreate yet:

  1. On Msg Body, explicitly set:
    Content-Transfer-Encoding: quoted-printable  // ensure email clients don't print equal signs and wrap at 76 characters; instead treat as continuous text
    Content-Type: text/plain; charset=UTF-8
    
  2. On Excel XLSX Attachment, explicitly set:
    filename="######-MonthlyReport-XXXXXXXX-XXX_X-##.xlsx" // shorter than before
    Content-Transfer-Encoding: base64  // even though base64 is the default
    Content-Type: application/octet-stream // even though this hasn't been misinterpreted
    name="######-MonthlyReport-XXXXXXXX-XXX_X-##.xlsx" // shorter than before
    
  3. On JSON Attachment, explicitly set:
    filename="######-MonthlyReport-XXXXXXXX-XXX_X-##.json"
    Content-Transfer-Encoding: base64
    Content-Type: application/json name="######-MonthlyReport-XXXXXXXX-XXX_X-##.json"
    
  4. Verify after writing to disk:
    1. read the EML from disk
    2. if first four characters are not "Date" OR if "Content-Disposition: attachment"' is not contained, then
      1. Log ERROR
      2. move file with timestamp at end for later debugging
      3. re-generate MSG and write EML again
    3. repeat 1 & 2 again to see if we strike gold

Additional context

No response

@jameshueston jameshueston added the bug Something isn't working label Jun 4, 2024
@wneessen
Copy link
Owner

wneessen commented Jun 4, 2024

Hi @jameshueston,

thanks for the report. I'll have a look into it and let you know my findings.

@wneessen wneessen self-assigned this Jun 4, 2024
@wneessen
Copy link
Owner

wneessen commented Jun 5, 2024

@jameshueston Would you be able to provide me with an (redacted) EML of each a good and a bad mail? As I understand this issue you have concerns regarding the order of the header fields in the EML, but according to the RFC the order of the header fields may appear in any order, so it should not make a difference is the first field is "Date:" or "Content-Type:". If I get examples of the EML files, I might be able to find the reason why they are not interpreted correctly by the mail clients. You don't have to attach the files to the issue, you can also mail them to me (GPG encryption is supported as well). Please let me know.

@jameshueston
Copy link
Author

@wneessen - I have redacted files ready; let me know which email you prefer.
If you'd rather not post your address here, feel free to message me either
at the address I used on the message when I bought you a coffee :) or
at the address on my profile.

Thanks in advance!

@wneessen
Copy link
Owner

wneessen commented Jun 6, 2024

You can use the address in the https://go-mail.dev/security/ page.

PS: Thanks a lot for the Coffee! Much appreciated!

@wneessen
Copy link
Owner

wneessen commented Jun 7, 2024

Hi @jameshueston,

thanks for providing me with the mail examples. I was able to identify the reason for the mail clients not being able "understand" the mails, yet I cannot find a logical reason for this happening at all.

The reason for the mail appearing to be broken is a double Content-Type header. In particular there seems to be a complete mix-up of the multipart Content-Type headers.

In your example of a bad mail we have:

Content-Type: multipart/mixed;
 boundary=54793082b051293332802e7cb7085ce36ed118ecdcfa0216a0015e3c2ca6
Date: Wed, 01 May 2024 01:20:17 +0000
[...]
Content-Type: multipart/alternative;
 boundary=e1cb711bc948256b16dc8aa7d3b1c32adf396581aeb11fe64f58c25dd095

This makes absolutely no sense to even occur. The generic headers (Date, MIME-Version, etc.) are all collected in a map, then sorted and written. The thing is though, that the Content-Type should not appear in the map at this time at all. The CT is set in a later part of the code, when the generic headers are already set and sorted. Also there shouldn't be two different sets of boundaries and in fact the CT for the mail (the correct one) should not be multipart/alternative but multipart/mixed (as it is for the first header in the bad mail).

I tried a couple of different scenarios in which I intetionally corrupted headers and attachments/mail parts but in none of them I was able to reproduce this behaviour. I used the test code you provided (in similar form) and wrote 100k test mails and not one of them showed this behaviour you are experiencing.

Some things (except of what I already mentioned) about the corrupted mail I can see:

  • There is no Content-Disposition header for any of the two attachments
  • There is no charset= set for any of the attachments in the Content-Type, even though the file type was correctly identified. This makes no sense since the code dictates a fallback to the mail's default charset (which is set) if the part/attachment has no charset set.
  • The main mail body has no Content-Transfer-Encoding which also makes no sense, unless the encoding was intentionally set to an empty string, especially since the main mail body is still correctly quoted-printable encoded.

So in conclusion, the only explanation I can come up with at this point is that maybe some strange file locking or race condition must have happend that must have combined two mail generation processes into one or maybe some kind of file system corruption. Code-wise I don't see any scenario in which this could happen except maybe intentionally messing up things in the internals of the code that are not publicly accessible. I'd like to give you a better exmplanation but at this point I'm clueless.

@wneessen
Copy link
Owner

wneessen commented Jun 7, 2024

Here is the quick and dirty test code I used (based on your example) for trying to reproduce:

package main

import (
	"fmt"
	"math/rand"
	"time"

	"github.com/wneessen/apg-go"

	"github.com/wneessen/go-mail"
)

func main() {
	for i := 1; i <= 1000; i++ {
		id := rand.Int63()
		fmt.Println("Writing mail no. ", i, " with ID: ", id)
		writeMail(i, id)
	}
}

func writeMail(no int, id int64) {
	toaddr := []string{"toni@tester.com", "antonia@example.com"}
	filepaths := []string{"test.json", "test.xlsx"}
	now := time.Now()

	msg := mail.NewMsg()
	if err := msg.From("test@test.com"); err != nil {
		fmt.Printf("failed to set FROM: %s\n", err)
	}
	if err := msg.To(toaddr...); err != nil {
		fmt.Printf("failed to set TO: %s\n", err)
	}
	msg.Subject(getMailSubject(id))
	msg.SetBodyString(getMailBody(id))

	// Attach XLSX file and JSON file
	for i := range filepaths {
		msg.AttachFile(filepaths[i], mail.WithFileName(fmt.Sprintf(`attachment_%d_%d_%s`, id,
			now.UnixMilli(), filepaths[i])))
	}

	if err := msg.WriteToFile(fmt.Sprintf("output/testmail_%d.eml", no)); err != nil {
		fmt.Println("failed to write mail:", err)
	}
}

func getMailBody(id int64) (mail.ContentType, string) {
	gen := apg.New(apg.NewConfig(apg.WithFixedLength(80), apg.WithAlgorithm(apg.AlgoRandom)))
	body := "Attached is the REDACTED." + mail.SingleNewLine
	body = body + mail.SingleNewLine
	body = body + fmt.Sprintf("%s: %d%s", "REDACTEDID: ", id, mail.SingleNewLine)
	for i := 0; i < 40; i++ {
		text, err := gen.Generate()
		if err != nil {
			continue
		}
		body = body + text + mail.SingleNewLine
	}

	return mail.TypeTextPlain, body
}

func getMailSubject(id int64) string {
	return fmt.Sprintf("This is the subject for Mail: %d", id)
}

(will require the two test files being present as well as a output/ directory)

To check if any of the mails is corrupt you can use the following code snipptes:

$ grep -i alternative output/*
$ head -1 output/* | grep -v '==>' | grep -v Date | perl -ne '/^\n$/ || print'

Both should not return anything. Maybe you want to run the testcode on one of the devices that has seen the corrupt mails and see if you can re-produce it that way? You can change the amount of generated mails in the for loop of the main() method.

@jameshueston
Copy link
Author

Thanks, @wneessen, for looking into this!

I may indeed have strange file locking that I dismissed early on, because:

  • the Msg builds in memory
  • dials and sends using your err = MailClient.DialAndSend(msg)
    • in the case of bad.eml threw failed to send mail: send failed: sending message content: write tcp ###.###.###.###:#####->###.###.###.###:587: i/o timeout
  • writes failed messages on disk to a retry directory
    • aside: volume-mapped outside of docker to a VM host, sitting among other VMs, on an Edge IoT device
  • a separate go routine runs on a different ticker looking for EML in the retry folder
    • lo and behold the ticker executed within the same second as the file save
    • read the file from disk
    • used a different function to create a new msg object
    • DID reuse the same *mail.Client to dial and send that failed half a second ago,
      • could this be it?
    • successfully sent BUT
      • took 8 seconds to send, seems long, but depending on the strength of the IoT Device's mobile connection
      • resulted in bad.eml outlined previously.
    • took 24 seconds to move the file on disk to an archive folder - way too long.

@wneessen
Copy link
Owner

Hi @jameshueston,

I don't think that reusing the *mail.Client would cause this issue but I could imagine that somewhere else in the process something might have broken. Especially given that we make use of some maps in the *Msg which are known to be not concurrency-safe. I could see a concurrency issue in the "go routine monitors the folder and picks up EML" part of the process. Not sure if you're already using some kind of mutex/locking mechanismn in that process but if not maybe it's worth a try?

At this point i'm not sure if there is something in the go-mail codebase right now that would actively cause this issue - execept of it not being concurrency-safe (which could be an improvement for a future release at some point)

@wneessen
Copy link
Owner

wneessen commented Jun 22, 2024

Hi again @jameshueston,

while working on the EML code this weekend, I identified a weird case in which I was able to replicate the Content-Type: multipart/mixed header in the first line. It was basically an issue where the EML parser would create a new part with that content type, even though it must not do this, as the msgwriter will take care of setting these kind of headers.

The latest code in the branch is now more mature and that issue should be fixed. I can't 100% confirm if that was the issue causeing the weird behaviour on your end but it is a possiblity. The branch should now also be able to handle multipart message with attachments and embeds without issues.

Hope this helps.

@jameshueston
Copy link
Author

Thanks @wneessen, good discovery! More coffee sent :)
It may be some time before I can try the branch, but when I do, I'll provide an update here.

@wneessen
Copy link
Owner

wneessen commented Jul 3, 2024

Thanks again James, Meanwhile the branch has been merged into v0.4.2. So instead give that a try ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in-progress
Projects
None yet
Development

No branches or pull requests

2 participants