net/mail: ParseAddress/String corrupt address
dvyukov opened this issue · 5 comments
The following program fails the panic:
package main
import (
"fmt"
"net/mail"
)
func main() {
data := "\"\\\x1f,\"<0@0>"
addr, err := mail.ParseAddress(data)
if err != nil {
return
}
_, err = mail.ParseAddress(addr.String())
if err != nil {
fmt.Printf("failed to parse addr: %q -> %q\n", data, addr)
panic(err)
}
}failed to parse addr: "\"\\\x1f,\"<0@0>" -> "=?utf-8?q?=1F,?= <0@0>"
panic: mail: no angle-addr
ParseAddress must handle result of Address.String, or else first ParseAddress must fail.
go version devel +514014c Thu Jun 18 15:54:35 2015 +0200 linux/amd64
Nice catch. I overlooked section 5.3 of RFC 2047 in mime.WordEncoder.Encode (used in addr.String()).
Since this is a new Go 1.5 method, should I fix it now?
cc @bradfitz
@alexcesaro: Go ahead and prepare a fix. We should fix bugs in new code.
I think to fix that we should:
- Add a WordEncoder to handle Q-encoding in a 'phrase' context.
- Use that new WordEncoder in net/mail instead of mime.QEncoding.
The first point concerns the new code while the second point concerns existing code and is not a regression: http://play.golang.org/p/nKIl4bflnE
Should I fix point 1 only ? both ? none ?
I see two ways to fix that issue:
- As I said in the previous message, add a new WordEncoder to Q-encode in a "phrase" context (ie. names in address fields).
- Use base64 encoding when the names in addresses use the additional characters that need to be encoded.
Solution 1 code will feel a bit hacky since WordEncoder underlying type is byte. We will need to add a new const like QPhraseEncoder = WordEncoder('Q').
Solution 2 is simpler, adds no new API and this is what Gmail do for example.
@bradfitz is solution 2 ok?
CL https://golang.org/cl/16012 mentions this issue.