Update lib.go: Changing mailFromRE regex

[ThomasLandauer]

• First change: Strictly speaking, there's no whitespace allowed: https://datatracker.ietf.org/doc/html/rfc5321#section-3.3

  Since it has been a common source of errors, it is worth noting that
  spaces are not permitted on either side of the colon following FROM
  in the MAIL command or TO in the RCPT command. The syntax is exactly
  as given above.

  But I agree that it's OK to allow it, but I'd restrict it to horizontal whitespace.

• Second change: The pattern is still too liberal, since not all characters are allowed in an email address. But what's really important is that you can't accept an > or vertical whitespace here.

• Third change: certainly no vertical whitespace allowed here.

If you agree, I'd do the same for the other regex'es

[axllent]

Thanks @ThomasLandauer. I definitely want to look at this, but I must put it temporarily on pause until I get my head clear. I have just had shoulder surgery 14h ago - so I'm pumped up on pain medication and not thinking clearly right now.

I'll just note two thinks here while I think of it:

1. I really should have included the tests from the original smtpd library which I copied and modified into the Mailpit codebase - which I have now done. I can't recall why I didn't do this earlier, but it will be very beneficial in cases like this. Could you please rebase your branch off the latest develop as this has to do with my second point directly below?
2. Can you briefly provide some examples of what you believe is and is not valid syntax? I need to investigate this further, but based on these tests](https://github.com/axllent/mailpit/blob/develop/server/smtpd/smtpd_test.go#L164-L193) I think your fix may break the logic? MAIL FROM:<[email protected]> SIZE=0 is seemingly valid (I need to research more when my brain is clear). I also see a potentially related issue (501 5.5.4 Syntax error in parameters or arguments (invalid SIZE parameter) #407) which is using a (currently?) unsupported feature MAIL FROM:<...> BODY=8BITMIME which uses the same appended syntax to the MAIL FROM. A bit more investigation reveals that the SMTP RFC actually allows parameters to be applied after MAIL FROM, and it's not limited to just SIZE= (meaning a bigger change is needed in Mailpit's implementation, as per the bug I just linked).

It would be great, if you are willing, to look into the second point above while I am recovering and share your thoughts? I'm not asking you to fix that other related bug, but consider whether your regex may break that, and maybe your thoughts on the optional additional syntax. Thanks!

[axllent]

Also worth noting that \h is not supported in Go ;-) I have enabled tests for this MR to hopefully help 👍

[ThomasLandauer]

First and foremost: All the best and take your time!!

The thing is: The list of allowed SMTP keywords is long! There is MTRK, ENVID and ORCPT here: https://datatracker.ietf.org/doc/html/rfc3885#section-2 And there's also RET defined somewhere, and maybe some more. Each with their own allowed values.
So the first question would be: Do you really want to validate them all? IMO, you could just (basically) "accept" (i.e. ignore) anything after the address. After all, the purpose of this library is so see if emails are being sent - this is no SMTP reference implementation or validator ;-)

What I wanted to address (i.e. forbid) is (note the >):

MAIL FROM:<foo>@example.com>

and (note the line break):

MAIL FROM:
<[email protected]>

While still allowing (note the space after :)

MAIL FROM: <[email protected]>

[axllent]

Thank you, I appreciate it.

I think [Ff][Rr][Oo][Mm]: ?<([^>\v]*)>( (.*))? is closer to what you are wanting here (just replace those two \h with a space ). It's not perfect as it'll detect just the <foo> in MAIL FROM:<foo>@example.com> and ignore the rest as that doesn't match the remainder of the regex, but that depends on what the better solution is. Unless I'm mistaken, the existing regex would result in an error as foo>@example.com is an invalid address which is, I think, is more important than unintentionally extracting just a part of it. Maybe the better solution would be to just substitute the \s with a space in the original code, and then let the code handle if it is an invalid address?

As for those optional parameters, we definitely do not want to validate all of those, however there is currently one we're interested in. Please leave that part as-is and I'll handle it as a separate task 👍

[ThomasLandauer]

Closing here in favor of #409