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/smtp: Address schemes #9140

Closed
gopherbot opened this issue Nov 20, 2014 · 4 comments
Closed

net/smtp: Address schemes #9140

gopherbot opened this issue Nov 20, 2014 · 4 comments
Milestone

Comments

@gopherbot
Copy link

by Ephrones:

Dial() wants as addr a scheme as defined in http://golang.org/pkg/net/#Dial.

SendMail() calls Dial() internally, but without looking at its source one can only guess
from the docstring what scheme it needs as addr.

It gets fun when you create a PlainAuth(), which wants a host (that is basically addr
but without the colon separated port). I thought: “Ah, already got that!” and put in
my addr from before. Now you get a rather meaningless “wrong host name” error.
As it turns out this one is thrown in net/smtp/auth.go because host doesn’t like the
port.

imho Dial() should have host and port as separate arguments, but I see that the API
can’t be changed now.

So I suggest the following:
1. Clarify the docstrings (reference to Dial() in net in all Dial() docstrings and in
SendMail)
2. Improve “wrong host name” to Sprintf("wrong host name (%s)", hostname)

Pull request coming, will reference this issue.
@gopherbot
Copy link
Author

Comment 1:

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

@minux
Copy link
Member

minux commented Nov 20, 2014

Comment 2:

i think it's obvious what Dial takes, we
shouldn't need to specify that every Dial
function/method uses the convention of
net.Dail.
the 2nd point about error message seems
plausible though.
we're in 1.4 release freeze, please ping
after 1.4 is out. Thanks.

@gopherbot
Copy link
Author

Comment 3 by Ephrones:

> i think it's obvious what Dial takes
Well, it took me about half an hour to find that out. Sharing the same name doesn’t
intrinsically mean they take the same arguments. Plus, how should I know I need to look
at net.Dial when all I wanted to do was use smtp.Dial to send a mail?
Btw, some Dial-methods already reference to net.Dial
I think it’s only sensible to add that information because it’s not obvious what
addr wants/accepts (e.g. named ports possible).
> we're in 1.4 release freeze
Will 1.5 use Github? It was quite interesting getting this to work.

@gopherbot gopherbot added the new label Nov 20, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc
Copy link
Contributor

rsc commented Apr 10, 2015

Maybe the docs could be made clearer.

@rsc rsc added this to the Go1.5Maybe milestone Apr 10, 2015
@rsc rsc closed this as completed in 29f03a3 Jul 15, 2015
@mikioh mikioh modified the milestones: Go1.5, Go1.5Maybe Jul 16, 2015
@golang golang locked and limited conversation to collaborators Jul 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants