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: interpret comment after address as display name #22670

Closed
stapelberg opened this issue Nov 11, 2017 · 12 comments
Closed

net/mail: interpret comment after address as display name #22670

stapelberg opened this issue Nov 11, 2017 · 12 comments

Comments

@stapelberg
Copy link
Contributor

@stapelberg stapelberg commented Nov 11, 2017

In #21257, we implemented skipping comments, resulting in e.g. “foo@bar.com (Foo Bar)” parsing into Address{Name: "", Address: "foo@bar.com"}.

I’d like to propose interpreting comments as display name, as various fairly common mail user agents still use them like that these days. Here are a few examples:

  • From: md@Linux.IT (Marco d'Itri)
    Date: Thu, 5 Oct 2017 14:42:30 +0200
    User-Agent: NeoMutt/20170609 (1.8.3)
  • From: owner@bugs.debian.org (Debian Bug Tracking System)
    Date: Sat, 07 Oct 2017 05:21:04 +0000
  • From: yumkam+debian@gmail.com (Yuriy M. Kaminskiy)
    Date: Tue, 01 Aug 2017 22:42:55 +0300
    User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux)

Thoughts?

cc @minaevmike, who worked on this code last

@odeke-em odeke-em changed the title net/mail: interpret comment after address as display name proposal: net/mail: interpret comment after address as display name Nov 11, 2017
@gopherbot gopherbot added this to the Proposal milestone Nov 11, 2017
@gopherbot gopherbot added the Proposal label Nov 11, 2017
@mvdan
Copy link
Member

@mvdan mvdan commented Nov 11, 2017

I assume this would be a fallback - so Foo <email> (Bar) would parse as {Name: "Foo", Address: "email"}. Or do you have something else in mind?

Are there any cases where the name is missing and the comment is used for a different purpose?

I am also wondering if it would be useful to somehow make this difference visible to the developer. Otherwise, Foo <email> and <email> (Foo) would seem the same when parsing.

@stapelberg
Copy link
Contributor Author

@stapelberg stapelberg commented Nov 12, 2017

I assume this would be a fallback - so Foo (Bar) would parse as {Name: "Foo", Address: "email"}.

Correct

Are there any cases where the name is missing and the comment is used for a different purpose?

I haven’t seen any so far, but of course that doesn’t mean such cases don’t exist.

I am also wondering if it would be useful to somehow make this difference visible to the developer. Otherwise, Foo and (Foo) would seem the same when parsing.

I’m not sure why a developer would need to concern themselves with this subtle detail of email parsing? :)

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 12, 2017

My knowledge of email address formats is limited, so I was simply trying to clarify the proposal a bit and see if there would be any noticeable disadvantages to accepting it.

It also seems like putting display names in the comments has been discouraged since 1982:

https://wordtothewise.com/2014/12/friendly-email-addresses/:

Never do this. The text in parentheses isn’t really a display name at all, rather it’s a human readable comment. Many MUAs will display that comment as though it were the display name, but it’s not been the right way to include a display name at least since RFC 822 was released in 1982.

RFC: http://tools.wordtothewise.com/rfc/822

Nevertheless, if this practice is still common, it seems okay to me to support it as a fallback.

@stapelberg
Copy link
Contributor Author

@stapelberg stapelberg commented Nov 12, 2017

Yep, I’m aware of the RFC :). We can of course also file bugs against mutt, neomutt, gnus and debbugs to use a more modern addr-spec, but that won’t fix the messages in my archives, which is why I’m starting to work on this end of the issue first.

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 12, 2017

That makes sense. I guess the only drawback of doing it would be that we'd be encouraging the bad behavior - but it's clearly too late for that :)

The point about being able to use email archives is useful, though. golang.org/s/owners just lists @bradfitz as maintainer - any thoughts, Brad? I realise the package is frozen, but this would not increase or change its API.

@ALTree
Copy link
Member

@ALTree ALTree commented Nov 12, 2017

Wouldn't be dangerous to put in Address.Name something that is sometimes used as the display name?

Even if ( ... ) is used for the name, say, 90% of the time, we would be returning a completely bogus value in Address.Name in the remaining 10%.

I mean, it would be pretty reasonable to assume that Address.Name != "" implies that the parsed address has form display-name <addr>, has a name in it, and I can retrieve that name at Address.Name.

This is true now, but if we start parsing

"foo@bar.com (comment)"

as

Address{Name: "comment", Address: "foo@bar.com"}

that assumption breaks. Address.Name is not empty, even if we just parsed an email address with no name in it.

The point is that when you write NAME <ADDR>, NAME can only be display-name. The spec requires this, if I read it correctly. So, putting NAME into Address.Name is a safe bet. Using Address.Name to store a (...) comment, not so much.

@stapelberg
Copy link
Contributor Author

@stapelberg stapelberg commented Nov 12, 2017

Wouldn't be dangerous to put in Address.Name something that is sometimes used as the display name?

RFC822 states “Many MUAs will display that comment as though it were the display name”.

I understand this as: one should expect comments in the From header to commonly be interpreted as display name, provided no actual display name is present in the addr-spec.

I can’t quite see any danger in implementing what an RFC states is common practice, and only affects a fallback code path. Even in the unlikely event that this case actually exists anywhere (do you have any real-world examples of this edge case?), I would maintain that treating such a comment as display name is actually sensible — most likely it will identify the author of the message.

@ALTree
Copy link
Member

@ALTree ALTree commented Nov 13, 2017

If, when present, the ( ... ) comment is pretty much always intended to be used as display-name, I guess it could be reasonable to implement your proposed change. I don't have any hard data about how frequently that assumption does not hold in the wild.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 13, 2017

Per @golang/proposal-review, if there's just one comment after the address, sure, let's make it the Name. CL welcome for Go 1.11.

@rsc rsc changed the title proposal: net/mail: interpret comment after address as display name net/mail: interpret comment after address as display name Nov 13, 2017
@rsc rsc modified the milestones: Proposal, Go1.11 Nov 13, 2017
@stapelberg
Copy link
Contributor Author

@stapelberg stapelberg commented Nov 13, 2017

Thanks! I’ll try to send a CL tomorrow.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 14, 2017

Change https://golang.org/cl/77474 mentions this issue: net/mail: treat comment in address as display name

@rsc
Copy link
Contributor

@rsc rsc commented Nov 14, 2017

It looks like handling comments at all is new in Go 1.10. Given that, we might as well fix this now and avoid having a release that handles but drops comments.

@rsc rsc modified the milestones: Go1.11, Go1.10 Nov 14, 2017
@rsc rsc added the release-blocker label Nov 14, 2017
@gopherbot gopherbot closed this in fcee189 Nov 14, 2017
@golang golang locked and limited conversation to collaborators Nov 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.