golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.23k stars 17.47k forks source link

net/mail: ParseAddress() accepts valid RFC 5322 addresses with domain part starting with a dash which are invalid RFC 1035 addresses, should we tighten the permissive validation? #39488

Open bmassemin opened 4 years ago

bmassemin commented 4 years ago

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

$ go version
go version go1.13 darwin/amd64
go go1.14.3 playground

Does this issue reproduce with the latest release?

Yes: https://play.golang.org/p/Z3a5zj6Qch6

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/bmassemin/Library/Caches/go-build"
GOENV="/Users/bmassemin/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/bmassemin"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/tn/b45tkwwd0dg36r6yk1bzvzv40000gn/T/go-build066110340=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/Z3a5zj6Qch6

package main

import (
    "net/mail"
    "testing"

    "github.com/stretchr/testify/assert"
)

func Test(t *testing.T) {
    _, err := mail.ParseAddress("test@--domain.com")
    assert.Error(t, err)
    _, err = mail.ParseAddress("test@domain--.com")
    assert.Error(t, err)
}

What did you expect to see?

I'm expecting the test to pass, since test@--domain.com is not a valid address. A domain can't starts or ends with a dash according to this RFC

What did you see instead?

The email address is properly parsed with the invalid domain.

odeke-em commented 4 years ago

Thank you for reporting this @bmassemin with detail, and welcome to the Go project!

Those addresses are valid according to the specification that net/mail follows aka RFC 5322, which is loose of a specification that what you are referencing in RFC 1035 and correct according to RFC 5322's syntax in which it is accepted as a dot-atom per atom Screen Shot 2020-06-09 at 3 44 28 PM

Screen Shot 2020-06-09 at 3 41 36 PM

However, in that RFC 5322 that net/mail uses, they say that enforcement of stricter address validations are left to the discretion of the implementers per https://tools.ietf.org/html/rfc5322#section-3.4.1 Screen Shot 2020-06-09 at 3 43 49 PM

This bug will then perhaps a discussion of whether we should tighten address validation to dip into the requests of RFC 1035 et al, or whether we should leave it alone. I shall retitle this bug.

Kindly /cc-ing @iWdGo @dsymonds @ianlancetaylor

dsymonds commented 4 years ago

Yes, net/mail deliberately sticks closely to the spec, and, if memory serves, actually accepts slightly more. People have very weird old mail boxes that they like to be able to parse, so being a bit more liberal in the parsing was a deliberate choice.

I'd say leave net/mail as it is.

beoran commented 4 years ago

I agree this can stay as is, however I would still add a note in the comments of ParseAddress to explain which RFC is implemented.

dsymonds commented 4 years ago

I agree this can stay as is, however I would still add a note in the comments of ParseAddress to explain which RFC is implemented.

It already does that in the package doc comment.

gopherbot commented 4 years ago

Change https://golang.org/cl/238118 mentions this issue: net/mail: declare that domain parsing is less strict than expected