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

Jump to bottom

net/mail: ParseAddress/String corrupt address #11292

Closed
dvyukov opened this issue Jun 19, 2015 · 5 comments
Closed

net/mail: ParseAddress/String corrupt address #11292

dvyukov opened this issue Jun 19, 2015 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@dvyukov
Copy link
Member

dvyukov commented Jun 19, 2015

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
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jun 19, 2015
@alexcesaro
Copy link
Contributor

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

@dsymonds
Copy link
Contributor

@alexcesaro: Go ahead and prepare a fix. We should fix bugs in new code.

@alexcesaro
Copy link
Contributor

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 ?

@alexcesaro alexcesaro mentioned this issue Sep 29, 2015
Closed
@alexcesaro
Copy link
Contributor

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?

@gopherbot
Copy link

CL https://golang.org/cl/16012 mentions this issue.

@rsc rsc mentioned this issue Nov 25, 2015
Closed
@alexcesaro alexcesaro mentioned this issue Jan 15, 2016
Closed
@golang golang locked and limited conversation to collaborators Dec 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests
5 participants
@dsymonds @alexcesaro @dvyukov @ianlancetaylor @gopherbot