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

Feat: Add message token to return path #2855

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jmdunsing
Copy link

@jmdunsing jmdunsing commented Mar 6, 2024

This PR incorporates a minor change to the way the return path is generated to allow for additional bounce processing and message forwarding. Those using postal strictly as a transactional email solution should notice no difference in performance. Those who wish to be able to capture all hard bounces and those who use postal as a marketing solution will have another method besides the X-Postal-MsgID header to identify bounces and replies, since the message token will be included in the return path.

Background:
Some mail servers return a bounce but omit identifiable information and strip the X-Postal-MsgID header from the email, leaving postal users unable to prevent future hard bounces. This change will provide the ability for a subsequent coding solution to tie bounces and replies to emails to their original sender.

This design change is consistent with the current bounce design, which splits the domain path on "@" then subsequently on "+", and checks the uname to see if the server was a bounce (specifically line 328), see

def rcpt_to(data)
unless in_state(:mail_from_received, :rcpt_to_received)
increment_error_count("rcpt-to-out-of-order")
return "503 EHLO/HELO and MAIL FROM first please"
end
rcpt_to = data.gsub(/RCPT TO\s*:\s*/i, "").gsub(/.*</, "").gsub(/>.*/, "").strip
if rcpt_to.blank?
increment_error_count("empty-rcpt-to")
return "501 RCPT TO should not be empty"
end
uname, domain = rcpt_to.split("@", 2)
if domain.blank?
increment_error_count("invalid-rcpt-to")
return "501 Invalid RCPT TO"
end
uname, tag = uname.split("+", 2)
if domain == Postal::Config.dns.return_path_domain || domain =~ /\A#{Regexp.escape(Postal::Config.dns.custom_return_path_prefix)}\./
# This is a return path
@state = :rcpt_to_received
if server = ::Server.where(token: uname).first
if server.suspended?
increment_error_count("server-suspended")
"535 Mail server has been suspended"
else
logger&.debug "Added bounce on server #{server.id}"
@recipients << [:bounce, rcpt_to, server]
"250 OK"
end
else
increment_error_count("invalid-server-token")
"550 Invalid server token"
end
:

These bounces will still not be identified by postal as bounces, and the incoming messages will still hardfail, because the current code still searches exclusively for the X-Postal-MsgID header

#
# Return a message object that this message is a reply to
#
def original_messages
return nil unless bounce
other_message_ids = raw_message.scan(/\X-Postal-MsgID:\s*([a-z0-9]+)/i).flatten
if other_message_ids.empty?
[]
else
database.messages(where: { token: other_message_ids })
end
end
:

* Fix: allow build for jmdunsing version of postal

Signed-off-by: jmdunsing <[email protected]>

* Fix: Revise version number for jmdunsing version of postal

Signed-off-by: jmdunsing <[email protected]>

---------

Signed-off-by: jmdunsing <[email protected]>
Adding the message token to the return path would be beneficial for users to link bounces to email addresses when X-Postal-MsgID was not included in the bounce message's headers.

Signed-off-by: jmdunsing <[email protected]>
@jmdunsing
Copy link
Author

I created an image in my fork of postal and emails are being sent correctly using the new method.

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@pedroluccasc
Copy link

i like this

@jmdunsing
Copy link
Author

What needs to be done to get this change incorporated?

@schueffi
Copy link

In our current setup (not using postal, but "exim"), we are using the exact same approach to have a "dedicated" bounce address per sent message. This way, we unambiguously can match incoming bounces to the corresponding outgoing emails without relying on still-included headers or message-ids.
The commit in this pull request just fulfills the "first half" of the process: adding individual bounce addresses. The "recognition" of bounces still relies on the X-Postal-MsgID header. This even could be supplemented by an additional search by the "tag" (as given in the new bounce-address after the "+" sign) - then, no "special" / "custom" processing of all "unmatched" bounces would be required. Optionally, this additional tag-based mail search could be enabled by some environmental / config setting.

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants