amphp / dns

Async DNS resolution for PHP based on Amp.
https://amphp.org/dns
MIT License
157 stars 32 forks source link

Use cloudflare DNS as fallback, and follow RFC 6761 for localhost name resolution #77

Closed danog closed 5 years ago

kelunik commented 5 years ago

Why did you add the Google fallback, i. e. in which situation does it fail?

Localhost is resolved via the hosts loader already.

danog commented 5 years ago

Loading of both the resolv.conf and the hosts files may fail on restricted environments (PHP safe mode, i.e. plain webhosts), that is why I added a fallback. Also, according to RFC 6761, DNS clients are supposed to resolve localhost to a loopback address anyway, so why rely on the hosts file:

   3.  Name resolution APIs and libraries SHOULD recognize localhost
       names as special and SHOULD always return the IP loopback address
       for address queries and negative responses for all other query
       types.  Name resolution APIs SHOULD NOT send queries for
       localhost names to their configured caching DNS server(s).
PeeHaa commented 5 years ago

IMO using google DNS as a hard-coded fallback is a bad idea.

Not only may the address be blocked in some parts of the world, but it is also a privacy issue in the rest of the world.

danog commented 5 years ago

OK then, switched to cloudflare DNS, instead, and moved the localhost fallback generation directly to the Config class (also fixing IPv6 loopback address generation).

kelunik commented 5 years ago

@danog Changing from Google to Cloudflare only changes who receives the information. We decided to remove any fallback versions ago, because if the information isn't available, this is usually a misconfiguration that should be fixed by the hoster.

I'd keep the logic in the HostsLoader, but the early return needs to be removed so the fallback for Windows can also apply for Linux and other systems not having localhost in the hosts file.

danog commented 5 years ago

PHP safe mode is not a misconfiguration, it's a PHP setting that allows more security in a shared environment, and it is used virtually by all hosting providers. Adding a fallback in case system files located in /etc cannot be accessed allows us to run this library on a much wider range of environments; if there is concern regarding the privacy of a simple DNS resolution service, the fallback could be provided by the user, instead (even if it would greatly complicate everything, since this library is actually mainly used by the socket library).

trowski commented 5 years ago

I think the best solution when the configuration file cannot be read is providing an error message that explains how to provide your own configuration. Any hard-coded fallback is undesirable.

danog commented 5 years ago

That's fine by me, as long as the socket library allows us to pass the configuration to the dns library. However, an easier option would actually be using gethostbyname as fallback for A records only (or it can be implemented as fallback in UDP sockets cannot be used).

kelunik commented 5 years ago

Merged manually without the fallback and with a slightly different implementation. Thanks!