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

Add std::net::lookup_addr for reverse DNS lookup #23419

Merged
merged 1 commit into from
Mar 27, 2015
Merged

Add std::net::lookup_addr for reverse DNS lookup #23419

merged 1 commit into from
Mar 27, 2015

Conversation

murarth
Copy link
Contributor

@murarth murarth commented Mar 16, 2015

Closes #22608

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@aturon
Copy link
Member

aturon commented Mar 16, 2015

cc @alexcrichton

@bors
Copy link
Contributor

bors commented Mar 17, 2015

☔ The latest upstream changes (presumably #23352) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

For now we may want to hold off adding this as std::net is still shaking out. I've added this to the wishlist issue, however. There are extra options and parameters to getnameinfo, so we may want to think a little longer about what this interface should look like. This is pretty minor though, so I think I'd be fine with it for now.

@murarth
Copy link
Contributor Author

murarth commented Mar 17, 2015

I'm not sure how to amend this if IpAddr has been removed. Is there not going to be a standard type to express "either an IPv4 or an IPv6 address?"

@@ -76,6 +76,11 @@ impl Iterator for LookupHost {
fn next(&mut self) -> Option<io::Result<SocketAddr>> { self.0.next() }
}

/// Resolve the given address to a hostname.
pub fn lookup_addr(addr: IpAddr) -> io::Result<String> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should take a SocketAddr now instead of an IpAddr (and better yet a &SocketAddr)

@alexcrichton
Copy link
Member

Could a test be added for this?

@aturon aturon mentioned this pull request Mar 19, 2015
91 tasks
@murarth
Copy link
Contributor Author

murarth commented Mar 26, 2015

Rebased after recent changes to std::net. I created the SocketAddr::new method because I needed to use something to that effect. If we still want to hold off on a public constructor for SocketAddr, I can make it a private local function instead.

@alexcrichton: I don't know if a test can be written for it. getnameinfo looks at system files and the outside network, neither of which can be relied upon in a test. I tested it myself, if that's at all reassuring. :P

@daboross
Copy link
Contributor

Just wondering, would it be at all possible to test this using localhost, or are there some targets where that wouldn't work?

@murarth
Copy link
Contributor Author

murarth commented Mar 26, 2015

@daboross: Generally, that would work, but it's possible that the /etc/hosts file (or non-Unix equivalent) may not contain an entry for 127.0.0.1 or it could have a different name. But perhaps the definition of localhost is a reasonable assumption made by other tests.

/// This function may perform a DNS query to resolve `addr` and may also inspect
/// system configuration to resolve the specified address. If the address
/// cannot be resolved, it is returned in string format.
#[unstable(feature = "ip_addr", reason = "recent addition")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is separable enough from the ip_addr feature that this may want to be called lookup_addr instead.

@alexcrichton
Copy link
Member

Ok, this looks good to me, thanks @murarth! r=me with a separate feature name for this API.

@murarth
Copy link
Contributor Author

murarth commented Mar 27, 2015

@alexcrichton: Done.

@alexcrichton
Copy link
Member

@bors: r+ c0dd239

@bors
Copy link
Contributor

bors commented Mar 27, 2015

⌛ Testing commit c0dd239 with merge b27c254...

@bors
Copy link
Contributor

bors commented Mar 27, 2015

💔 Test failed - auto-win-64-nopt-t

@alexcrichton
Copy link
Member

@bors: retry

On Fri, Mar 27, 2015 at 3:11 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-64-nopt-t
http://buildbot.rust-lang.org/builders/auto-win-64-nopt-t/builds/2726


Reply to this email directly or view it on GitHub
#23419 (comment).

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 27, 2015
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 27, 2015
@bors
Copy link
Contributor

bors commented Mar 27, 2015

⌛ Testing commit c0dd239 with merge 0c9de81...

bors added a commit that referenced this pull request Mar 27, 2015
@bors bors merged commit c0dd239 into rust-lang:master Mar 27, 2015
@murarth murarth deleted the lookup-addr branch March 28, 2015 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std::net lacks reverse DNS lookup
6 participants