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

net/mail: Empty quoted display name should be allowed in email address #14866

Closed
mackstann opened this issue Mar 18, 2016 · 6 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mackstann
Copy link

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

1.5.3.

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

Linux, amd64

  1. What did you do?

Pass an email address with an empty quoted name, like:

"" <foo@bar.com>

to mail.Parse()

  1. What did you expect to see?

The email address should parse without error

  1. What did you see instead?

Error returned: mail: missing word in phrase: mail: empty quoted-string

  1. Further info

We don't experience this problem on Go 1.4. I found that the offending change was in commit bd1efd5. The first release containing this change was 1.5, which corroborates the difference I've seen between the behavior in 1.4 and 1.5.

The change in consumeQuotedString, given the tests that were added along with the change, intends to disallow an empty quoted string in the username part of the email address. However, it unintentionally also disallows an empty quoted string as the display name part of the email address -- but as far as I can tell, an empty quoted string there is perfectly valid. Since the tests did not address the case of an empty quoted name, I assume that it was an oversight. consumeQuotedString is used in these different contexts, and should not always disallow an empty quoted string. It should allow an empty quoted string when it's a part of the display name portion of an email address.

RFC5322 section 3.2.4 defines quoted string as:

quoted-string   =       [CFWS]
                        DQUOTE *([FWS] qcontent) [FWS] DQUOTE
                        [CFWS]

Given that qcontent is attached to a *, which allows for zero or more occurrences, then the zero occurrences case results in an empty quoted string, which is allowed by this definition. Thus, Go should allow empty quoted strings in email addresses, at least in the case of the display name.

For what it's worth, we're using the github.com/go-gomail package to create the message. If you pass it an empty name, it always puts "" before the <address> part (see responsible function here), and while this is annoyingly triggering this bug in Go, it does seem to be perfectly valid as far as the RFCs are concerned.

Even if my interpretation of the RFC were wrong and current Go behavior is right, the tests are clearly not thinking about this specific case, and they should be expanded to do so.

Here is a play.golang.org reproduction of the bug: http://play.golang.org/p/iSjSINU-Dm

@ianlancetaylor
Copy link
Contributor

CC @DenBeke

@DenBeke
Copy link
Contributor

DenBeke commented Mar 19, 2016

I can look into it if I find some time...

@trtstm
Copy link
Contributor

trtstm commented Mar 19, 2016

I think this is fixed easily by removing https://github.com/golang/go/blob/master/src/net/mail/message.go#L444.
Then adjusting some tests e.g https://github.com/golang/go/blob/master/src/net/mail/message_test.go#L612 should be valid I think because local-part can also be a quoted-string.

addr-spec       =   local-part "@" domain
local-part      =   dot-atom / quoted-string / obs-local-part

If the 'if statement' on line 444 is removed then ParseAddress(""John Doe" <""@example.com>") returns

Name: John Doe
Address: @example.com

instead of

Name: John Doeof
""@example.com

So that should be fixed too.

@DenBeke
Copy link
Contributor

DenBeke commented Mar 19, 2016

Quoted-string can indeed be empty (so I'm indeed "guilty") :)

trtstm added a commit to trtstm/go that referenced this issue Mar 19, 2016
@trtstm
Copy link
Contributor

trtstm commented Mar 19, 2016

The changes in my branch seem to work, but I am not sure if what I did is correct.
"Rob" <@> becomes "Rob" <""@>

And I don't know whether

<"\t0"@0>

Should be valid or not.

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 7, 2016
@gopherbot
Copy link

CL https://golang.org/cl/32176 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants