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

Use getaddrinfo and parse /etc/hosts on linux #460

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@gittywithexcitement
Contributor

gittywithexcitement commented Jan 13, 2014

The motivation for this change is that my application needs /etc/hosts to be parsed and used.
So I changed usage of evdns_base_resolve_ipv4/ipv6 to getaddrinfo, because evdns_base_resolve_XXX doesn't use the hosts file.

You may be wondering why I then applied a commit "Change EventDriver.resolveHost parameter..." The answer is that I can't compile the current master HEAD (see #458). So I branched from the most recent commit that compiles for me, and made my changes. Then I cherry picked d49f615 and resolved conflicts.

So far I've only tested with an /etc/hosts containing localhost. Tomorrow I will test with a more challenging /etc/hosts file that maps nameFoo to 127.0.0.1 and nameBar to a remote IP. I have every reason to believe it will work.

s-ludwig and others added some commits Jan 3, 2014

Use getaddrinfo and parse /etc/hosts on linux
TODO: for windows, look at \WINDOWS\system32\drivers\etc\hosts
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 13, 2014

Member

This is great! I didn't know that evdns_base_load_hosts exists (really should have scanned through the whole API once). So this fixes #289. your commit is still based on the old commit. I'll try to cherry pick the actual change into master and test.

Member

s-ludwig commented Jan 13, 2014

This is great! I didn't know that evdns_base_load_hosts exists (really should have scanned through the whole API once). So this fixes #289. your commit is still based on the old commit. I'll try to cherry pick the actual change into master and test.

@gittywithexcitement

This comment has been minimized.

Show comment
Hide comment
@gittywithexcitement

gittywithexcitement Jan 13, 2014

Contributor

Note that for windows, hosts file is in \windows\system32\drivers\etc\hosts. I wasn't sure how to get that path in the canonical way, so I didn't implement it.

Contributor

gittywithexcitement commented Jan 13, 2014

Note that for windows, hosts file is in \windows\system32\drivers\etc\hosts. I wasn't sure how to get that path in the canonical way, so I didn't implement it.

@gittywithexcitement

This comment has been minimized.

Show comment
Hide comment
@gittywithexcitement

gittywithexcitement Jan 13, 2014

Contributor

Actually, rather than base_load_hosts, it's probably better to call
int evdns_base_resolv_conf_parse(struct evdns_base *base, int flags, const char *filename);
With flags = DNS_OPTIONS_ALL or flags=DNS_OPTION_HOSTSFILE at the very least. I've never used resolv.conf, but this path seems the most 'full featured'.
See http://www.wangafu.net/~nickm/libevent-book/Ref9_dns.html It points out that this is a linux feature, and for windows, evdns_base_config_windows_nameservers applies. As a side benefit, it looks like evdns_base_config_windows_nameservers will do the dirty work of finding the location of Windows' nameserver settings.
Anyway, base_load_hosts is good enough for my current purposes, but I thought I'd point out resolv_conf_parse

Contributor

gittywithexcitement commented Jan 13, 2014

Actually, rather than base_load_hosts, it's probably better to call
int evdns_base_resolv_conf_parse(struct evdns_base *base, int flags, const char *filename);
With flags = DNS_OPTIONS_ALL or flags=DNS_OPTION_HOSTSFILE at the very least. I've never used resolv.conf, but this path seems the most 'full featured'.
See http://www.wangafu.net/~nickm/libevent-book/Ref9_dns.html It points out that this is a linux feature, and for windows, evdns_base_config_windows_nameservers applies. As a side benefit, it looks like evdns_base_config_windows_nameservers will do the dirty work of finding the location of Windows' nameserver settings.
Anyway, base_load_hosts is good enough for my current purposes, but I thought I'd point out resolv_conf_parse

s-ludwig added a commit that referenced this pull request Jan 13, 2014

Use getaddrinfo and parse /etc/hosts on linux - fixes #289.
TODO: for windows, look at \WINDOWS\system32\drivers\etc\hosts

See also #460.

s-ludwig added a commit that referenced this pull request Jan 13, 2014

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 13, 2014

Member

I've hard-coded the "C:\Windows..." path as a first step and will just use the %SYSTEMROOT% env var to get it correctly.

Thanks for the additional evdns_ suggestions. I'll look into it.

Member

s-ludwig commented Jan 13, 2014

I've hard-coded the "C:\Windows..." path as a first step and will just use the %SYSTEMROOT% env var to get it correctly.

Thanks for the additional evdns_ suggestions. I'll look into it.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Aug 11, 2017

Member

This PR was already integrated into master manually and can be closed.

Member

s-ludwig commented Aug 11, 2017

This PR was already integrated into master manually and can be closed.

@s-ludwig s-ludwig closed this Aug 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment