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: ParseAddress ignore comment in parentheses after email address #21257

Closed
minaevmike opened this issue Aug 1, 2017 · 6 comments
Closed

Comments

@minaevmike
Copy link
Contributor

@minaevmike minaevmike commented Aug 1, 2017

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

go version go1.8.3 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/root/prog/GO"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build728818306=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

https://play.golang.org/p/3OTryRCHf8

What did you expect to see?

mark@cbosgd.ATT.COM as parsed email

What did you see instead?

error with mail: expected single address, got "(Mark Horton)"

at rfc 5322
mailbox = name-addr / addr-spec
name-addr = [display-name] angle-addr
angle-addr = [CFWS] "<" addr-spec ">" [CFWS] /obs-angle-addr
CFWS = (1*([FWS] comment) [FWS]) / FWS)
so CFWS can contian comment in parentheses and i think that it must be ingored

@smasher164
Copy link
Member

@smasher164 smasher164 commented Aug 2, 2017

RFC 5322 also mentions under section 3.2.2 that "comments may nest," and that "there are several places in this specification where comments and FWS may be freely inserted." Implementing this feature would probably require implementing folding white space (FWS). Please specify

  1. The level of nesting needed, or whether fully nested comments are required.
  2. The parts of angle-addr that need comments. Looking under section 3.4.1, addr-spec also allows comments.
addr-spec       =   local-part "@" domain
local-part      =   dot-atom / quoted-string / obs-local-part
domain          =   dot-atom / domain-literal / obs-domain
domain-literal  =   [CFWS] "[" *([FWS] dtext) [FWS] "]" [CFWS]
  1. The usage and importance of having comments in an email address. Email addresses are often parsed from network requests, and inefficient parsing could be insecure. Sources of inefficiency include the indefinite nesting, dealing with escape characters inside the comments, and interpreting multiple runs of whitespace as a single character.
@minaevmike
Copy link
Contributor Author

@minaevmike minaevmike commented Aug 2, 2017

I am not sure that i understand correclty but all spaces already ignored https://github.com/golang/go/blob/master/src/net/mail/message.go#L272 .
Also i should notice that documentation give wrong information https://golang.org/src/net/mail/message.go#L147 because this parser not full rfc5322 compatiable.
As for me one of possible decision of this problem is to add function that will skip this comments, if i understand corretly after skipSpace we should skip data between ( and ). It's not very inefficient.
I use this fucntions to parse email addresses from emails and some MTA form addresses like that because it's consistent with rfc.

@smasher164
Copy link
Member

@smasher164 smasher164 commented Aug 2, 2017

but all spaces already ignored

Whitespace is currently not folded (consuming runs of carriage returns and newlines), but I suppose single-line comments are enough.

Also i should notice that documentation give wrong information https://golang.org/src/net/mail/message.go#L147 because this parser not full rfc5322 compatible.

Nowhere does the parser say that ParseAddress is fully RFC 5322 compliant. In fact it explictly documents otherwise, notably L287: Groups, L221: FWS, and L376: domain-literal.

after skipSpace we should skip data between ( and )

Assuming that the level of nesting is bounded, and that comments are allowed before and after the address, this sounds like a reasonable request. Though in practice, I don't think more than one level is necessary.

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 7, 2017

Change https://golang.org/cl/53550 mentions this issue: net/mail: skip trailing CFWS while parsing email

@minaevmike
Copy link
Contributor Author

@minaevmike minaevmike commented Sep 21, 2017

Hi.
I create review about 1.5 month ago and nobody watch it.
What should I do to be seen by someone?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 21, 2017

You sent the CL during the freeze, sorry. We will eventually sweep it up, but pinging, as you just did, is a good idea too. Sorry for the delay.

@gopherbot gopherbot closed this in 8598396 Sep 24, 2017
@golang golang locked and limited conversation to collaborators Sep 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.