[GH-ISSUE #149] ReturnPath contains the 822.From, not the 821.From #102

Closed
opened 2026-03-15 12:33:28 +03:00 by kerem · 3 comments
Owner

Originally created by @wttw on GitHub (Aug 4, 2023).
Original GitHub issue: https://github.com/axllent/mailpit/issues/149

The response to /api/v1/message/{ID} contains a field ReturnPath. I would expect that field to contain the return path - the email address provided via MAIL FROM: in the SMTP transaction. Instead it contains the address from the From: header.

e.g. when delivering an email like this

<-  220 jiji.local Mailpit ESMTP Service ready
 -> EHLO jiji.local
<-  250-jiji.local greets jiji.local
<-  250-SIZE 0
<-  250 ENHANCEDSTATUSCODES
 -> MAIL FROM:<steve@jiji.local>
<-  250 2.1.0 Ok
 -> RCPT TO:<steve@blighty.com>
<-  250 2.1.5 Ok
 -> DATA
<-  354 Start mail input; end with <CR><LF>.<CR><LF>
 -> Date: Fri, 04 Aug 2023 12:59:01 +0100
 -> To: steve@blighty.com
 -> From: bodyfrom@example.com
 -> Subject: test Fri, 04 Aug 2023 12:59:01 +0100
 -> Message-Id: <20230804125901.044574@jiji.local>
 -> X-Mailer: swaks v20201014.0 jetmore.org/john/code/swaks/
 ->
 -> This is a test mailing
 ->
 ->
 -> .
<-  250 2.0.0 Ok: queued
 -> QUIT
<-  221 2.0.0 jiji.local Mailpit ESMTP Service closing transmission channel

I would expect ReturnPath to contain steve@jiji.local , while it actually contains bodyfrom@example.com

It looks like MailPit is expecting to read the return path from a Return-Path header in the stored mail, but it isn't populating that header from the contents of MAIL FROM: when it's received. Then when it can't find the header it uses the contents of the From: header instead.

Originally created by @wttw on GitHub (Aug 4, 2023). Original GitHub issue: https://github.com/axllent/mailpit/issues/149 The response to /api/v1/message/{ID} contains a field ReturnPath. I would expect that field to contain the return path - the email address provided via MAIL FROM: in the SMTP transaction. Instead it contains the address from the From: header. e.g. when delivering an email like this ``` <- 220 jiji.local Mailpit ESMTP Service ready -> EHLO jiji.local <- 250-jiji.local greets jiji.local <- 250-SIZE 0 <- 250 ENHANCEDSTATUSCODES -> MAIL FROM:<steve@jiji.local> <- 250 2.1.0 Ok -> RCPT TO:<steve@blighty.com> <- 250 2.1.5 Ok -> DATA <- 354 Start mail input; end with <CR><LF>.<CR><LF> -> Date: Fri, 04 Aug 2023 12:59:01 +0100 -> To: steve@blighty.com -> From: bodyfrom@example.com -> Subject: test Fri, 04 Aug 2023 12:59:01 +0100 -> Message-Id: <20230804125901.044574@jiji.local> -> X-Mailer: swaks v20201014.0 jetmore.org/john/code/swaks/ -> -> This is a test mailing -> -> -> . <- 250 2.0.0 Ok: queued -> QUIT <- 221 2.0.0 jiji.local Mailpit ESMTP Service closing transmission channel ``` I would expect ReturnPath to contain `steve@jiji.local `, while it actually contains `bodyfrom@example.com` It looks like MailPit is expecting to read the return path from a Return-Path header in the stored mail, but it isn't populating that header from the contents of MAIL FROM: when it's received. Then when it can't find the header it uses the contents of the From: header instead.
kerem closed this issue 2026-03-15 12:33:33 +03:00
Author
Owner

@axllent commented on GitHub (Aug 4, 2023):

That's a valid point and a good observation @wttw. The only potential flaw I can find in your PR is that a Return-Path header will always gets added to the mail ~ but what if a Return-Path already exists in the mail headers, as this would lead to multiple Return-Paths which isn't compliant?

I must admit that since I wrote Mailpit, I've come to discover that every mail server and SMTP client variant seems to work slightly differently, and many SMTP servers have fallback solutions to handle these to achieve greater compatibility and fewer delivery issues. It is also often very difficult to find black&white explanations as to how to "officially" deal with these situations. From what I can tell, a receiving SMTP server should always overwrite the Return-Path using the From envelope (as you have suggested).

I actively try to avoid removing any headers in the SMTPD as this creates complications because it reorders and groups the message headers (due to Go's map ordering). Using regular expressions to strip out a header is an option, but can lead to complications too. I wonder if the safest approach here would be to only add the Return-Path header (as you have done) when there isn't already one set? Whilst not strictly compliant, it does then cater for other "mail injection" situations (for instance I use Mailpit to "archive" my regular mail accounts for testing, so emails are read from plain text files and "pushed" to the SMTP server).

Keen to hear your thoughts before I make any decision on approach.

<!-- gh-comment-id:1666198753 --> @axllent commented on GitHub (Aug 4, 2023): That's a valid point and a good observation @wttw. The only potential flaw I can find in your PR is that a `Return-Path` header will always gets added to the mail ~ but what if a `Return-Path` already exists in the mail headers, as this would lead to multiple Return-Paths which isn't compliant? I must admit that since I wrote Mailpit, I've come to discover that every mail server and SMTP client variant seems to work slightly differently, and many SMTP servers have fallback solutions to handle these to achieve greater compatibility and fewer delivery issues. It is also often very difficult to find black&white explanations as to how to "officially" deal with these situations. From what I can tell, a receiving SMTP server should [always overwrite the Return-Path](https://stackoverflow.com/questions/4367358/whats-the-difference-between-sender-from-and-return-path#comment48064981_25873119) using the From envelope (as you have suggested). I actively try to avoid **removing** any headers in the SMTPD as this creates complications because it reorders and groups the message headers (due to Go's map ordering). Using regular expressions to strip out a header is an option, but can lead to complications too. I wonder if the safest approach here would be to only add the `Return-Path` header (as you have done) when there isn't already one set? Whilst not strictly compliant, it does then cater for other "mail injection" situations (for instance I use Mailpit to "archive" my regular mail accounts for testing, so emails are read from plain text files and "pushed" to the SMTP server). Keen to hear your thoughts before I make any decision on approach.
Author
Owner

@axllent commented on GitHub (Aug 4, 2023):

Actually I think the following solution would work far better (to replace your solution):

	returnPath := strings.Trim(msg.Header.Get("Return-Path"), "<>")
	if returnPath != from {
		if returnPath != "" {
			// remove incorrect Return-Path
			re := regexp.MustCompile(`(?i)(^|\n)(Return\-Path: .*\n)`)
			replaced := false
			data = re.ReplaceAllFunc(data, func(r []byte) []byte {
				if replaced {
					return r
				}
				replaced = true // only replace first occurrence

				return re.ReplaceAll(r, []byte(""))
			})
		}
		// add Return-Path
		data = append([]byte("Return-Path: <"+from+">\r\n"), data...)
	}

This seems to cover all bases:

  1. If the Return-Path is already correctly set, do nothing, else
  2. If the Return-Path exists remove it
  3. Add a new Return-Path

If my logic makes sense, please update your MR with the above code and I'll merge it in, thanks.

<!-- gh-comment-id:1666274965 --> @axllent commented on GitHub (Aug 4, 2023): Actually I think the following solution would work far better (to replace your solution): ```go returnPath := strings.Trim(msg.Header.Get("Return-Path"), "<>") if returnPath != from { if returnPath != "" { // remove incorrect Return-Path re := regexp.MustCompile(`(?i)(^|\n)(Return\-Path: .*\n)`) replaced := false data = re.ReplaceAllFunc(data, func(r []byte) []byte { if replaced { return r } replaced = true // only replace first occurrence return re.ReplaceAll(r, []byte("")) }) } // add Return-Path data = append([]byte("Return-Path: <"+from+">\r\n"), data...) } ``` This seems to cover all bases: 1. If the `Return-Path` is already correctly set, do nothing, else 2. If the `Return-Path` exists remove it 3. Add a new `Return-Path` If my logic makes sense, please update your MR with the above code and I'll merge it in, thanks.
Author
Owner

@axllent commented on GitHub (Aug 6, 2023):

I've actually gone ahead and made the change, as what I provided in my earlier comment had a few issues. This has been released as v1.8.1. Please feel free to reopen if it does not do what you were expecting. I'll also close your PR as that has been superseded by the change I made. Thanks though for your info/contribution, I appreciate it!

<!-- gh-comment-id:1666723771 --> @axllent commented on GitHub (Aug 6, 2023): I've actually gone ahead and made the change, as what I provided in my earlier comment had a few issues. This has been released as v1.8.1. Please feel free to reopen if it does not do what you were expecting. I'll also close your PR as that has been superseded by the change I made. Thanks though for your info/contribution, I appreciate it!
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/mailpit#102
No description provided.