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: parseAddressList doesn't allow commas with empty email address #36959

Closed
timmydo opened this issue Feb 1, 2020 · 4 comments
Closed

net/mail: parseAddressList doesn't allow commas with empty email address #36959

timmydo opened this issue Feb 1, 2020 · 4 comments
Assignees
Milestone

Comments

@timmydo
Copy link

@timmydo timmydo commented Feb 1, 2020

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

$ go version
go version go1.13.5 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/timmy/.cache/go-build"
GOENV="/home/timmy/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/timmy/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build645469531=/tmp/go-build -gno-record-gcc-switches"

What did you do?

net/mail/message.go parseAddressList doesn't allow commas with empty email address. I use an email client that uses this library and it fails to parse some emails.

Here is an example from a mailing list:

Cc: , emacs-devel@gnu.org

What did you expect to see?

I think parseAddressList should filter out empty entries.

RFC 5322 has a section about this. I think it would be nice if empty spaces between commas were passed over.

4.4.  Obsolete Addressing

   There are four primary differences in addressing.  First, mailbox
   addresses were allowed to have a route portion before the addr-spec
   when enclosed in "<" and ">".  The route is simply a comma-separated
   list of domain names, each preceded by "@", and the list terminated
   by a colon.  Second, CFWS were allowed between the period-separated
   elements of local-part and domain (i.e., dot-atom was not used).  In
   addition, local-part is allowed to contain quoted-string in addition
   to just atom.  Third, mailbox-list and address-list were allowed to
   have "null" members.  That is, there could be two or more commas in
   such a list with nothing in between them, or commas at the beginning
   or end of the list.  Finally, US-ASCII control characters and quoted-
   pairs were allowed in domain literals and are added here.

   obs-angle-addr  =   [CFWS] "<" obs-route addr-spec ">" [CFWS]

   obs-route       =   obs-domain-list ":"

   obs-domain-list =   *(CFWS / ",") "@" domain
                       *("," [CFWS] ["@" domain])

   obs-mbox-list   =   *([CFWS] ",") mailbox *("," [mailbox / CFWS])

   obs-addr-list   =   *([CFWS] ",") address *("," [address / CFWS])

What did you see instead?

--- FAIL: TestAddressParsing (0.00s)
message_test.go:527: Failed parsing (single) " , joe@where.test": mail: no angle-addr

@timmydo

This comment has been minimized.

Copy link
Author

@timmydo timmydo commented Feb 1, 2020

--- /usr/local/go/src/net/mail/message.go	2019-12-04 14:53:53.000000000 -0800
+++ message.go	2020-01-31 22:42:03.184647817 -0800
@@ -251,6 +251,16 @@
 	var list []*Address
 	for {
 		p.skipSpace()
+
+		// obs-addr-list: allow skipping over empty entries
+		if p.consume(',') {
+			p.skipSpace();
+		}
+
+		if p.empty() {
+			break
+		}
+		
 		addrs, err := p.parseAddress(true)
 		if err != nil {
 			return nil, err


--- /usr/local/go/src/net/mail/message_test.go	2019-12-04 14:53:53.000000000 -0800
+++ message_test.go	2020-01-31 22:40:55.724863690 -0800
@@ -281,6 +281,19 @@
 			},
 		},
 		{
+			` , joe@where.test,,John <jdoe@one.test>,`,
+			[]*Address{
+				{
+					Name:    "",
+					Address: "joe@where.test",
+				},
+				{
+					Name:    "John",
+					Address: "jdoe@one.test",
+				},
+			},
+		},
+		{
 			`Group1: <addr1@example.com>;, Group 2: addr2@example.com;, John <addr3@example.com>`,
 			[]*Address{
 				{

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Feb 1, 2020

@timmydo I think this is reasonable. Want to send the patch above as a CL? See https://tip.golang.org/doc/contribute.html; I'd be happy to review.

@mvdan mvdan changed the title net/mail/message.go parseAddressList doesn't allow commas with empty email address net/mail: parseAddressList doesn't allow commas with empty email address Feb 1, 2020
timmydo pushed a commit to timmydo/go that referenced this issue Feb 1, 2020
RFC 5322 has a section 4.4 where it says that address-list could
have "null" members: "That is, there could be two or more commas in
such a list with nothing in between them, or commas at the beginning
or end of the list." This change handles such a case so that mail
clients using this method on actual email messages get a reasonable
return value when they parse email.

Fixes golang#36959
timmydo pushed a commit to timmydo/go that referenced this issue Feb 1, 2020
RFC 5322 has a section 4.4 where it says that address-list could
have "null" members: "That is, there could be two or more commas in
such a list with nothing in between them, or commas at the beginning
or end of the list." This change handles such a case so that mail
clients using this method on actual email messages get a reasonable
return value when they parse email.

Fixes golang#36959
timmydo pushed a commit to timmydo/go that referenced this issue Feb 1, 2020
RFC 5322 has a section 4.4 where it says that address-list could
have "null" members: "That is, there could be two or more commas in
such a list with nothing in between them, or commas at the beginning
or end of the list." This change handles such a case so that mail
clients using this method on actual email messages get a reasonable
return value when they parse email.

Fixes golang#36959
@timmydo

This comment has been minimized.

Copy link
Author

@timmydo timmydo commented Feb 1, 2020

Thanks @mvdan . I created one

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Feb 1, 2020

Change https://golang.org/cl/217377 mentions this issue: net/mail: skip empty entries in parseAddressList

timmydo pushed a commit to timmydo/go that referenced this issue Feb 1, 2020
RFC 5322 has a section 4.4 where it says that address-list could
have "null" members: "That is, there could be two or more commas in
such a list with nothing in between them, or commas at the beginning
or end of the list." This change handles such a case so that mail
clients using this method on actual email messages get a reasonable
return value when they parse email.

Fixes golang#36959
timmydo pushed a commit to timmydo/go that referenced this issue Feb 1, 2020
RFC 5322 has a section 4.4 where it says that address-list could
have "null" members: "That is, there could be two or more commas in
such a list with nothing in between them, or commas at the beginning
or end of the list." This change handles such a case so that mail
clients using this method on actual email messages get a reasonable
return value when they parse email.

Fixes golang#36959
@odeke-em odeke-em added this to the Go1.15 milestone Feb 5, 2020
@gopherbot gopherbot closed this in 30e3bf2 Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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