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: rename Resolver.Dial to Resolver.DialContext before Go1.9rc1? #19910

Closed
rsc opened this issue Apr 10, 2017 · 8 comments
Closed

net: rename Resolver.Dial to Resolver.DialContext before Go1.9rc1? #19910

rsc opened this issue Apr 10, 2017 · 8 comments

Comments

@rsc
Copy link
Contributor

rsc commented Apr 10, 2017

CL 37260 proposes add a new field to net.Resolver:

	// Dial is used by Go's built-in DNS resolver to make TCP and
	// UDP connections to DNS services. The Resolver will never call this
	// with names, only valid IP addresses. The Conn returned must be
	// a *TCPConn or *UDPConn as requested by the network parameter.
	// If nil the default dialer is used.
	Dial func(ctx context.Context, network, hostport string) (Conn, error)

Elevating that CL to a proposal, hopefully a quick one.

This would allow control over the source address for DNS lookups in multi-homed systems (#17404).

Discussion on #19268 suggests that the new Dial field would be an acceptable solution for "do DNS lookups using a custom server", by installing a Dial func that lies about what server it has connected to. Of course that only works if /etc/resolv.conf lists some name server. If there are no resolvers then Dial will never be called. This particular use seems a bit kludgy, but fine if it resolves that need.

/cc @bradfitz

@bradfitz
Copy link
Contributor

I'm fine with this. But should it be named DialContext to be consistent with:

?

/cc @nerdatmath

@rsc
Copy link
Contributor Author

rsc commented Apr 17, 2017

SGTM.

@rsc rsc modified the milestones: Go1.9, Proposal Apr 17, 2017
@rsc rsc changed the title proposal: net: allow Resolver to use a custom dialer net: allow Resolver to use a custom dialer Apr 17, 2017
@benburkert
Copy link
Contributor

CL 37260 added this method as Dial, should it be renamed to DialContext?

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Jun 8, 2017
Allow the Resolver.Dial func to return instances of Conn other than
*TCPConn and *UDPConn. If the Conn is also a PacketConn, assume DNS
messages transmitted over the Conn adhere to section 4.2.1. "UDP usage".
Otherwise, follow section 4.2.2. "TCP usage".

Provides a hook mechanism so that DNS queries generated by the net
package may be answered or modified before being sent to over the
network.

Updates #19910

Change-Id: Ib089a28ad4a1848bbeaf624ae889f1e82d56655b
Reviewed-on: https://go-review.googlesource.com/45153
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@odeke-em
Copy link
Member

@bradfitz and @rsc, @benburkert raises a great point that it seems like @nerdatmath's CL https://go-review.googlesource.com/c/37260/8/src/net/lookup.go#106 already added the proposed field and perhaps we just need to rename it to DialContext?
screen shot 2017-07-17 at 10 34 51 pm
we just forgot to tag this issue in that CL.

Also by superficially skimming through the issue, I think after that rename, we'd have addressed @bradfitz's request in #19910 (comment) and then the issue can be marked as resolved. What do y'all think?

@bradfitz
Copy link
Contributor

I think Dial is fine. Our other DialContext methods are named so because there was already a Dial in the way hogging the good name.

@bradfitz
Copy link
Contributor

But I could see the consistency argument too. We're days away from an rc1, though, so it's a bit late (but still possible) to change.

@rsc, @ianlancetaylor?

@bradfitz bradfitz changed the title net: allow Resolver to use a custom dialer net: rename Resolver.Dial to Resolver.DialContext before Go1.9rc1? Jul 22, 2017
@ianlancetaylor
Copy link
Contributor

I think Dial is OK.

@golang golang locked and limited conversation to collaborators Jul 24, 2018
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

6 participants