Update lib.go: Changing `mailFromRE` and `rcptToRE` regex #409

ThomasLandauer · 2024-12-13T11:52:04Z

I don't know how to rebase so I started from scratch ;-)

Most important change is ( |$) in the middle, this removes the nested parentheses in the end; and therefore the entire regex is more readable I think (if a regex can be readable at all g...)

Anyway, this now fails on foo>@example.com.

I don't know how to rebase so I started from scratch ;-) 2 changes compared to what you said at axllent#406 (comment) * I did the same for `rcptToRE` * I replaced the `*` quantifier with `+`, for consistency

ThomasLandauer · 2024-12-13T11:58:17Z

One more question: This is the first time that I'm seeing the SIZE keyword used somewhere (or do you know popular MTA's using it?). Why did you implement it at all?

axllent · 2024-12-13T17:26:28Z

server/smtpd/lib.go

@@ -27,8 +27,8 @@ import (
 var (
 	// Debug `true` enables verbose logging.
 	Debug      = false
-	rcptToRE   = regexp.MustCompile(`[Tt][Oo]:\s?<(.+)>`)
-	mailFromRE = regexp.MustCompile(`[Ff][Rr][Oo][Mm]:\s?<(.*)>(\s(.*))?`) // Delivery Status Notifications are sent with "MAIL FROM:<>"
+	rcptToRE   = regexp.MustCompile(`[Tt][Oo]: ?<([^<>\v]+)>( |$)(.*)?`)


The closing ( |$)(.*)? should be just ?$ as we want to account for one possible space, and then the end of the line. Unlike MAIL FROM, I am not aware of any other arguments that are allowed to be passed to RCPT TO (unless you know something I don't?).

NOTIFY and ORCPT, see https://datatracker.ietf.org/doc/html/rfc3461#section-4.1

Good catch! OK, leave rcptToRE as you had it, we just ignore any parameters 👍

axllent · 2024-12-13T17:49:22Z

All good. Rebasing is like taking the changes from another branch and then "merging" your changes over the to of them, but in your current branch. Sometimes with mixed results... so no problem, whatever works :)

One more question: This is the first time that I'm seeing the SIZE keyword used somewhere (or do you know popular MTA's using it?). Why did you implement it at all?

MAIL FROM:<sender@example.com> SIZE=1000 - The SIZE parameter is an optional argument that can be appended to an SMTP transaction to get an early rejection because a message is too big, rather than having to send all the DATA first and then be rejected. I cannot say how often it is used, only that it is definitely used, and is the one argument Mailpit cares about. Technically it's more of a guideline from what I read, an estimate of the size (eg: new headers will get appended), but we use it as-is because it will be close enough.

The other one I came across a couple of days ago is MAIL FROM:<sender@example.com> BODY=8BITMIME - coming via the Cloudflare proxy (#407). This was new to me, but it is a perfectly valid argument, and from what I can tell, used to provide the downstream SMTP server with the message encoding in the event of a bounce (I assume for the attached bounce message). Whilst Mailpit doesn't care about this, it was failing to accept the messages because it did not recognize it (hence my changes in develop you needed to pull in).

I have added a comment regarding the updated rcptToRE regexp. I also note that a few of the tests are now failing in your branch:

FROM:<> (empty) is actually valid syntax. I'm just not sure how to change the regex to allow for that... 🤔
ignore the failing To, I may need to adjust the tests slightly.

On that note, do you know how to test in go? Running go test github.com/axllent/mailpit/server/smtpd should work for you.

axllent · 2024-12-13T17:55:19Z

server/smtpd/lib.go

-	rcptToRE   = regexp.MustCompile(`[Tt][Oo]:\s?<(.+)>`)
-	mailFromRE = regexp.MustCompile(`[Ff][Rr][Oo][Mm]:\s?<(.*)>(\s(.*))?`) // Delivery Status Notifications are sent with "MAIL FROM:<>"
+	rcptToRE   = regexp.MustCompile(`[Tt][Oo]: ?<([^<>\v]+)>( |$)(.*)?`)
+	mailFromRE = regexp.MustCompile(`[Ff][Rr][Oo][Mm]: ?<([^<>\v]+)>( |$)(.*)?`) // Delivery Status Notifications are sent with "MAIL FROM:<>"


Ahh found it, to allow an empty value: add a pipe symbol at the start of <(|[^<>\v]+)> so we allow nothing OR what you have - seems to work 🥳

axllent · 2024-12-14T02:06:52Z

I was still waiting on the final change to allow empty MAIL FROM:<> and then realised I could edit this myself in your working branch (it's a Github feature I've never used before) :) So I've done this now and merged in your change, thanks!

axllent · 2024-12-14T10:13:28Z

This has now been released in v1.21.7. Thanks for your efforts!

ThomasLandauer · 2024-12-14T10:32:51Z

Just wanted to take care of the rest today, but you already fixed it :-)

About SIZE again: I think it comes unexpected that you really support SIZE (with tests and everything), and ignore all the others. Maybe you could document that somewhere (if it isn't already)?

axllent · 2024-12-14T18:22:28Z

The SMTPD supports SIZE and validates the argument syntax if provided, however Mailpit doesn't use or have an option for that (ie: there is no max message size). It just keeps the options open for future development ;-)

As for documenting it what it validates and what it doesn't in this regard, I'd argue it's not important and just adds lots of words to the documentation. Mailpit was been widely used for testing in the last two+ years, and only this last week did an issue arise because of the BODY= argument - clearly a very rare option to use. SIZE on the other hand is far more common I believe a it prevents the mail server having to send the entire message before being rejected for size, so I assume also used in some mobile & desktop email clients (not just servers). I would definitely add documentation for this if I ever implemented a max size option to Mailpit though, but that's very low on my radar as it's not something people test (if your system is generating messages that are larger than mail servers can handle, then you have much bigger issues).

ThomasLandauer · 2024-12-14T23:00:05Z

FYI, I was curious, so I took a closer look at the 3 SMTP clients I have at hand:

Thunderbird 115.14.0:

MAIL FROM:<...> BODY=8BITMIME SIZE=3592

Exim 4.96:

MAIL FROM:<...>

Postfix 3.7.11:

MAIL FROM:<...> SIZE=2779

axllent · 2024-12-14T23:53:55Z

That's really good to know, thank you. Interesting how Thunderbird provides two arguments, which answers another thing I wasn't sure about - how they are strung together. I assumed it was either a space or a comma so I catered for both 😄

ThomasLandauer · 2024-12-15T11:14:11Z

Multiple Keywords are separated by a space; multiple values of the same keyword are separated by comma:

NOTIFY=success,delay,failure

And some keywords don't have values at all:

SMTPUTF8

And there's also a semicolon separating the 2 parts of one value:

ORCPT=rfc822;foo@example.com

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [axllent/mailpit](https://github.com/axllent/mailpit) | patch | `v1.21.6` -> `v1.21.8` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>axllent/mailpit (axllent/mailpit)</summary> ### [`v1.21.8`](https://github.com/axllent/mailpit/blob/HEAD/CHANGELOG.md#v1218) [Compare Source](axllent/mailpit@v1.21.7...v1.21.8) ##### Chore - Update node dependencies - Update Go dependencies ##### Fix - **db:** Remove unused FOREIGN KEY REFERENCES in message_tags table ([#374](axllent/mailpit#374)) ### [`v1.21.7`](https://github.com/axllent/mailpit/blob/HEAD/CHANGELOG.md#v1217) [Compare Source](axllent/mailpit@v1.21.6...v1.21.7) ##### Chore - Update node dependencies - Update Go dependencies - Bump Go version for automated testing - Move smtpd & pop3 modules to internal - Stricter SMTP 'MAIL FROM' & 'RCPT TO' handling ([#409](axllent/mailpit#409)) - Display "To" details in mobile messages list - Display "From" details in message sidebar (desktop) ([#403](axllent/mailpit#403)) ##### Fix - Ignore unsupported optional SMTP 'MAIL FROM' parameters ([#407](axllent/mailpit#407)) - Prevent splitting multi-byte characters in message snippets ([#404](axllent/mailpit#404)) ##### Testing - Add smtpd tests </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

ThomasLandauer added 2 commits December 13, 2024 12:31

Update lib.go: Changing mailFromRE and rcptToRE regex

f59e748

I don't know how to rebase so I started from scratch ;-) 2 changes compared to what you said at axllent#406 (comment) * I did the same for `rcptToRE` * I replaced the `*` quantifier with `+`, for consistency

Update lib.go

95682c4

ThomasLandauer mentioned this pull request Dec 13, 2024

Update lib.go: Changing mailFromRE regex #406

Closed

axllent reviewed Dec 13, 2024

View reviewed changes

Allow valid empty MAIL FROM value

97c8f15

axllent merged commit 2ea92d1 into axllent:develop Dec 14, 2024
6 checks passed

ThomasLandauer deleted the patch-3 branch December 14, 2024 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update lib.go: Changing `mailFromRE` and `rcptToRE` regex #409

Update lib.go: Changing `mailFromRE` and `rcptToRE` regex #409

ThomasLandauer commented Dec 13, 2024

ThomasLandauer commented Dec 13, 2024

axllent Dec 13, 2024

ThomasLandauer Dec 13, 2024

axllent Dec 13, 2024 •

edited

Loading

axllent commented Dec 13, 2024

axllent Dec 13, 2024

axllent commented Dec 14, 2024

axllent commented Dec 14, 2024

ThomasLandauer commented Dec 14, 2024

axllent commented Dec 14, 2024

ThomasLandauer commented Dec 14, 2024 •

edited

Loading

axllent commented Dec 14, 2024

ThomasLandauer commented Dec 15, 2024

Update lib.go: Changing mailFromRE and rcptToRE regex #409

Update lib.go: Changing mailFromRE and rcptToRE regex #409

Conversation

ThomasLandauer commented Dec 13, 2024

ThomasLandauer commented Dec 13, 2024

axllent Dec 13, 2024

Choose a reason for hiding this comment

ThomasLandauer Dec 13, 2024

Choose a reason for hiding this comment

axllent Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

axllent commented Dec 13, 2024

axllent Dec 13, 2024

Choose a reason for hiding this comment

axllent commented Dec 14, 2024

axllent commented Dec 14, 2024

ThomasLandauer commented Dec 14, 2024

axllent commented Dec 14, 2024

ThomasLandauer commented Dec 14, 2024 • edited Loading

axllent commented Dec 14, 2024

ThomasLandauer commented Dec 15, 2024

Update lib.go: Changing `mailFromRE` and `rcptToRE` regex #409

Update lib.go: Changing `mailFromRE` and `rcptToRE` regex #409

axllent Dec 13, 2024 •

edited

Loading

ThomasLandauer commented Dec 14, 2024 •

edited

Loading