search

flashmob / go-guerrilla

Mini SMTP server written in golang
MIT License
2.78k stars 366 forks source link

extractEmailRegex in util.go accepts some bogus local-parts, rejects some valid local-parts #120

Closed gene-hightower closed 5 years ago

gene-hightower commented 6 years ago

The regex might be drunk, for all we know.

Accepts: 🍔@example.com and: <>@digilicious.com and: "@digilicious.com

but rejects: "@"@digilicious.com

So, maybe this isn't an issues for you guys, but u r failing good old RFC-5321.

Or maybe the programmer who thought a regex was powerful enough to parse the local-part syntax was drunk.

Or maybe /I/ should be drunk, so none of this would bother me.

flashmob commented 6 years ago

Thanks for pointing this out! Indeed, that comment has a double meaning - referring to how the regex looks & also well aware of the limitations.

You're welcome to contribute if you want to fix it, prefer not to use regex and also avoid adding new dependency if possible.

On Jul 29, 2018 10:55, "Gene Hightower" notifications@github.com wrote:

The regex might be drunk, for all we know.

Accepts: 🍔@example.com and: <>@digilicious.com and: "@digilicious.com

but rejects: "."@digilicious.com and: "@"@digilicious.com

So, maybe this isn't an issues for you guys, but u r failing good old RFC-5321.

Or maybe the programmer who thought a regex was powerful enough to parse the local-part syntax was drunk.

Or maybe /I/ should be drunk, so none of this would bother me.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/flashmob/go-guerrilla/issues/120, or mute the thread https://github.com/notifications/unsubscribe-auth/AAnmP1e7JDT-dBDTTiKthOqMwkmt0rQsks5uLRYigaJpZM4VlQI1 .

gene-hightower commented 6 years ago

The "avoid adding new dependency" is the tricky part. Are dependencies so hard to manage?

flashmob commented 6 years ago

Not hard to manage if there is only a few, but they do add "technical debt" for some.

Anyhow, is strict RFC handing your requirement? Another way would be to have an interface for this function, then you could inject your own handing, how does that sound?

On Tue, Jul 31, 2018, 02:10 Gene Hightower notifications@github.com wrote:

The "avoid adding new dependency" is the tricky part. Are dependencies so hard to manage?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/flashmob/go-guerrilla/issues/120#issuecomment-408938988, or mute the thread https://github.com/notifications/unsubscribe-auth/AAnmP9TPhc-MA6HfzMhGCJshreMEY3mFks5uLz3_gaJpZM4VlQI1 .

flashmob commented 6 years ago

Err, just realized that the "technical debt" comment above is contradictory, since in fact using a regex to parse local-part syntax is indeed "technical debt" itself.

Anyhow, after going down the rabbit-hole, it looks like golang std lib has some RFC 5322 address parsing functions - not sure how that was missed. https://golang.org/pkg/net/mail/#AddressParser.Parse

TODO:

  1. Replace use of the "Drunk Regex" with https://golang.org/pkg/net/mail/#AddressParser.Parse
  2. Add test cases for examples given by @gene-hightower
gene-hightower commented 6 years ago

BTW: RFC 5322 defines different syntax from RFC 5321. At the SMTP protocol level 5321 is the standard.