[PATCH] Resolver: parse hosts file entries

[Thibault Charbonnier]

Hi,

Thank you Ruslan for your answer.

On 9/14/17 6:31 AM, Ruslan Ermilov wrote:
> I personally don't like the idea of parsing /etc/hosts by nginx.
> The approach choosen looks like a hack, as obviously editing the
> /etc/hosts file between configuration reloads won't take any effect,
> which is controversial.

I can understand that and even relate. If this point were to be raised 
as a blocker during review, I wanted to suggest making the hostnames 
parsed from /etc/hosts respect the 'valid' option of the nginx resolver.

When the parsed records reach this ttl, we could parse the file again, 
thus limiting I/O to a minimum and still allowing changes to /etc/hosts 
to take effect somewhat intuitively (since the 'valid' option of the 
nginx resolver is already known by users). This behavior could simply be 
documented.

> Thus, in my opinion, such an approach would raise more questions than
> answers.

One could maybe argue that not resolving /etc/hosts raises just as many 
questions, with different answers? Maybe a compromised approach as 
suggested above could help alleviate this feeling?

Documentation also plays an important role in answering those questions 
I think? Plus, this feature being disabled by default, if users want to 
learn its existence or its usage, they would have to read the 
documentation. The proposed behavior doesn't seem too complicated to 
document.

> As to the implementation, offhand I can see two problems.  Using the
> NGX_MAX_UINT32_VALUE as the maximum value won't work on systems with
> 32-bit signed time_t.  

Indeed. Although this concern seems to be nullified if we take the 
approach of using the 'valid' option as ttl?

> And the more fundamental one: the code creates
> an rbtree node for each line of /etc/hosts, thus given the following
> natural contents:
> 
> 127.0.0.1       localhost
> ::1             localhost
> 
> I can't predict which one of the two addresses such a "resolver"
> would return.

Yes. This is an edge-case I have realized shortly after proposing the 
latest version of the patch, but due to the lack of response, I haven't 
done anything about it - yet.

I believe it would be easy to override any previous node bearing the 
same hostname with the ones coming after, which is, I think, how this 
situation is conventionally handled.



All in all, I am more than willing to update the patch with any change 
you deem necessary, but if the nginx team is not convinced of its 
usefulness regardless, then that is another story.

Maybe my above proposals make this feature more suitable for the nginx 
resolver?

Thank you again for your reply!

-- Thibault