golang/go

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:

  1. Add a WordEncoder to handle Q-encoding in a 'phrase' context.
  2. 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:

  1. As I said in the previous message, add a new WordEncoder to Q-encode in a "phrase" context (ie. names in address fields).
  2. 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.