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

Jump to bottom

net/mail: ParseAddress error message is confusing if given a partial addr-spec #27064

Closed
scottmmjackson opened this issue Aug 17, 2018 · 3 comments
Closed

net/mail: ParseAddress error message is confusing if given a partial addr-spec #27064

scottmmjackson opened this issue Aug 17, 2018 · 3 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@scottmmjackson
Copy link

What version of Go are you using (go version)?

go1.10.2 darwin/amd64

go1.10.3 linux/amd64

Does this issue reproduce with the latest release?

1.10 is the latest release

What operating system and processor architecture are you using (go env)?

Primary platform is linux/amd64

What did you do?

net/mail's mail.ParseAddress("some.bogus.email.address.with.no.at")

What did you expect to see?

"No @ in addr-spec"

Even better, an error message that doesn't require domain knowledge of RFC 5322 to be understood.

What did you see instead?

"mail: no angle-addr"

I believe the parser tries parsing an email address without angle-brackets first, and then tries to consume angle brackets. On the second pass, upon not finding angle brackets in the email address, it complains that the address isn't an angle-addr.

In any event, the validation error of: some.bogus.email.address.with.no.at should likely be that it lacks an @ symbol, not that it's not an angle-addr.

See: https://tools.ietf.org/html/rfc5322

I would prefer if error messages were either descriptive enough to be comprehensible by end users, or properly made into package globals so they could be trapped for without string matching, or both.
@mvdan
Copy link
Member

mvdan commented Aug 17, 2018

I've had a look. I'm not sure what we could do here. We can't always return "missing @" in the case where there is no address, as we do want ParseAddress("John Doe") to error like it does now.

The closest fix that comes to mind without altering the error message for that case is to detect when the string given could be a valid addr-spec, if only followed by @domain.

That way, we could have ParseAddress("john.doe") return something like `missing @ or angle-addr'. That seems OK to me, but I wonder what other mail address parsers out there do in this case.

If you have more ideas on how to make this error more comprehensive without handling edge cases, ideas are welcome. I don't think we should mention the @ symbol in the "John Doe" case, as that's clearly wrong.

@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 17, 2018
@mvdan mvdan added this to the Go1.12 milestone Aug 17, 2018
@mvdan mvdan changed the title net/mail: ParseAddress error messages don't make sense in some cases net/mail: ParseAddress error message is confusing if given a partial addr-spec Aug 17, 2018
@matrixik
Copy link

Another way would be to provide additional API for parsing only email address without name.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.12, Unplanned Dec 19, 2018
@gopherbot
Copy link

Change https://golang.org/cl/165517 mentions this issue: net/mail: better error in ParseAddress when missing "@domain"

@golang golang locked and limited conversation to collaborators Mar 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@matrixik @scottmmjackson @ianlancetaylor @mvdan @gopherbot