Repository: netty/netty
Issue: The resolver does not re-read /etc/hosts upon change
Labels: help wanted

### Expected behavior
When /etc/hosts is changed, the netty resolver should re-read it.

### Actual behavior
io.netty.resolver.DefaultHostsFileEntriesResolver parses /etc/hosts only once, upon creation.

### Netty version
4.1.67

### JVM version (e.g. `java -version`)
any

### OS version (e.g. `uname -a`)
any


The WatchService (https://docs.oracle.com/javase/tutorial/essential/io/notification.html) can be used to monitor /etc/hosts and re-read it immediately after it is changed. The same Java API can be used in UnixResolverDnsServerAddressStreamProvider for re-reading /etc/resolv.conf.
At the very least, /etc/hosts should be re-read every few minutes (like the current approach for /etc/resolv.conf).


Issue comments:
Sounds good! Can you do a PR?
Careful with the current JDK `WatchService` as it will miss changes if the file is _bind mounted_. In container based productions this is likely `/etc/hosts` is bind mounted.

I wrote a bit a few years ago : https://blog.arkey.fr/2019/09/13/watchservice-and-bind-mount/
@bric3 I think we should just re-read after X seconds. 
Agreed, polling is the cheapest thing to do and it's portable.
That's what I ended up doing.
It would be good if we keep this non-blocking ...
I think what we should do is very similar to here : https://github.com/netty/netty/blob/4.1/resolver-dns-classes-macos/src/main/java/io/netty/resolver/dns/macos/MacOSDnsServerAddressStreamProvider.java#L170
I am new to this project and I'd like to take this up. Do I wait until this is assigned to me or can I start working on it immediately? Thanks!
There is already a pending PR for this: https://github.com/netty/netty/pull/11922
> There is already a pending PR for this: #11922

ok, I will look into other open issues. Thanks!

Pull request review comments:
Honestly I think every 10 seconds might be a bit aggressive... What about 60 seconds as default + maybe a system property to change it . 
please don't use wildcard imports 
all of these can be static 
nit: remove the public keyword 
I just wanted to add the question as a separate comment, didn't expect you will be so fast! 
I wasn't able to find a typical pattern across system property names, so pls let me know if I need to change it.
fixed
fixed
fixed
just do this in a init block like we do in other places:

https://github.com/netty/netty/blob/netty-4.1.72.Final/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L117
nit: store `System.nanoTime()` in a local variable and reuse it in the compareAndSet. No need to call the function two times. 
I would still allow to pass in the `refreshInterval` so we can use it for testing.
See above... if we add a constructor that takes the interval we can directly construct the `DefaultHostsFileEntriesResolver` in tests and set the correct interval. 
moved to static init block, also added `debug` log :+1:, thanks
fixed
added `refreshInterval` back to the constuctor
replaced this `beforeAll` with `refreshInterval` in constructor params

Patch that resolved the issue:
diff --git a/resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java b/resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java
index 378fd98d87e..d48fe096c37 100644
--- a/resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java
+++ b/resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java
@@ -17,6 +17,9 @@
 
 import io.netty.util.CharsetUtil;
 import io.netty.util.internal.PlatformDependent;
+import io.netty.util.internal.SystemPropertyUtil;
+import io.netty.util.internal.logging.InternalLogger;
+import io.netty.util.internal.logging.InternalLoggerFactory;
 
 import java.net.InetAddress;
 import java.nio.charset.Charset;
@@ -24,42 +27,49 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * Default {@link HostsFileEntriesResolver} that resolves hosts file entries only once.
  */
 public final class DefaultHostsFileEntriesResolver implements HostsFileEntriesResolver {
 
-    private final Map<String, List<InetAddress>> inet4Entries;
-    private final Map<String, List<InetAddress>> inet6Entries;
+    private static final InternalLogger logger =
+            InternalLoggerFactory.getInstance(DefaultHostsFileEntriesResolver.class);
+    private static final long DEFAULT_REFRESH_INTERVAL;
+
+    private final long refreshInterval;
+    private final AtomicLong lastRefresh = new AtomicLong(System.nanoTime());
+    private final HostsFileEntriesProvider.Parser hostsFileParser;
+    private volatile Map<String, List<InetAddress>> inet4Entries;
+    private volatile Map<String, List<InetAddress>> inet6Entries;
+
+    static {
+        DEFAULT_REFRESH_INTERVAL = SystemPropertyUtil.getLong(
+                "io.netty.hostsFileRefreshInterval", TimeUnit.SECONDS.toNanos(60));
+
+        if (logger.isDebugEnabled()) {
+            logger.debug("-Dio.netty.hostsFileRefreshInterval: {}", DEFAULT_REFRESH_INTERVAL);
+        }
+    }
 
     public DefaultHostsFileEntriesResolver() {
-        this(parseEntries());
+        this(HostsFileEntriesProvider.parser(), DEFAULT_REFRESH_INTERVAL);
     }
 
     // for testing purpose only
-    DefaultHostsFileEntriesResolver(HostsFileEntriesProvider entries) {
+    DefaultHostsFileEntriesResolver(HostsFileEntriesProvider.Parser hostsFileParser, long refreshInterval) {
+        this.hostsFileParser = hostsFileParser;
+        this.refreshInterval = refreshInterval;
+        HostsFileEntriesProvider entries = parseEntries(hostsFileParser);
         inet4Entries = entries.ipv4Entries();
         inet6Entries = entries.ipv6Entries();
     }
 
     @Override
     public InetAddress address(String inetHost, ResolvedAddressTypes resolvedAddressTypes) {
-        String normalized = normalize(inetHost);
-        switch (resolvedAddressTypes) {
-            case IPV4_ONLY:
-                return firstAddress(inet4Entries.get(normalized));
-            case IPV6_ONLY:
-                return firstAddress(inet6Entries.get(normalized));
-            case IPV4_PREFERRED:
-                InetAddress inet4Address = firstAddress(inet4Entries.get(normalized));
-                return inet4Address != null ? inet4Address : firstAddress(inet6Entries.get(normalized));
-            case IPV6_PREFERRED:
-                InetAddress inet6Address = firstAddress(inet6Entries.get(normalized));
-                return inet6Address != null ? inet6Address : firstAddress(inet4Entries.get(normalized));
-            default:
-                throw new IllegalArgumentException("Unknown ResolvedAddressTypes " + resolvedAddressTypes);
-        }
+        return firstAddress(addresses(inetHost, resolvedAddressTypes));
     }
 
     /**
@@ -72,6 +82,8 @@ public InetAddress address(String inetHost, ResolvedAddressTypes resolvedAddress
      */
     public List<InetAddress> addresses(String inetHost, ResolvedAddressTypes resolvedAddressTypes) {
         String normalized = normalize(inetHost);
+        ensureHostsFileEntriesAreFresh();
+
         switch (resolvedAddressTypes) {
             case IPV4_ONLY:
                 return inet4Entries.get(normalized);
@@ -90,6 +102,18 @@ public List<InetAddress> addresses(String inetHost, ResolvedAddressTypes resolve
         }
     }
 
+    private void ensureHostsFileEntriesAreFresh() {
+        long last = lastRefresh.get();
+        long currentTime = System.nanoTime();
+        if (currentTime - last > refreshInterval) {
+            if (lastRefresh.compareAndSet(last, currentTime)) {
+                HostsFileEntriesProvider entries = parseEntries(hostsFileParser);
+                inet4Entries = entries.ipv4Entries();
+                inet6Entries = entries.ipv6Entries();
+            }
+        }
+    }
+
     // package-private for testing purposes
     String normalize(String inetHost) {
         return inetHost.toLowerCase(Locale.ENGLISH);
@@ -108,14 +132,13 @@ private static InetAddress firstAddress(List<InetAddress> addresses) {
         return addresses != null && !addresses.isEmpty() ? addresses.get(0) : null;
     }
 
-    private static HostsFileEntriesProvider parseEntries() {
+    private static HostsFileEntriesProvider parseEntries(HostsFileEntriesProvider.Parser parser) {
         if (PlatformDependent.isWindows()) {
             // Ony windows there seems to be no standard for the encoding used for the hosts file, so let us
             // try multiple until we either were able to parse it or there is none left and so we return an
             // empty instance.
-            return HostsFileEntriesProvider.parser()
-                    .parseSilently(Charset.defaultCharset(), CharsetUtil.UTF_16, CharsetUtil.UTF_8);
+            return parser.parseSilently(Charset.defaultCharset(), CharsetUtil.UTF_16, CharsetUtil.UTF_8);
         }
-        return HostsFileEntriesProvider.parser().parseSilently();
+        return parser.parseSilently();
     }
 }


Test patch:
diff --git a/resolver/src/test/java/io/netty/resolver/DefaultHostsFileEntriesResolverTest.java b/resolver/src/test/java/io/netty/resolver/DefaultHostsFileEntriesResolverTest.java
index b9e8c25ea41..794d2566a0b 100644
--- a/resolver/src/test/java/io/netty/resolver/DefaultHostsFileEntriesResolverTest.java
+++ b/resolver/src/test/java/io/netty/resolver/DefaultHostsFileEntriesResolverTest.java
@@ -15,24 +15,37 @@
  */
 package io.netty.resolver;
 
+import com.google.common.collect.Maps;
 import io.netty.util.NetUtil;
 import org.junit.jupiter.api.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
+import java.nio.charset.Charset;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.any;
 
 public class DefaultHostsFileEntriesResolverTest {
+    private static final Map<String, List<InetAddress>> LOCALHOST_V4_ADDRESSES =
+            Collections.singletonMap("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
+    private static final Map<String, List<InetAddress>> LOCALHOST_V6_ADDRESSES =
+            Collections.singletonMap("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
+    private static final long ENTRIES_TTL = TimeUnit.MINUTES.toNanos(1);
 
     /**
      * show issue https://github.com/netty/netty/issues/5182
@@ -47,13 +60,12 @@ public void testCaseInsensitivity() {
 
     @Test
     public void shouldntFindWhenAddressTypeDoesntMatch() {
-        Map<String, List<InetAddress>> inet4Entries = new HashMap<String, List<InetAddress>>();
-        Map<String, List<InetAddress>> inet6Entries = new HashMap<String, List<InetAddress>>();
+        HostsFileEntriesProvider.Parser parser = givenHostsParserWith(
+                LOCALHOST_V4_ADDRESSES,
+                Collections.<String, List<InetAddress>>emptyMap()
+        );
 
-        inet4Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
-
-        DefaultHostsFileEntriesResolver resolver =
-                new DefaultHostsFileEntriesResolver(new HostsFileEntriesProvider(inet4Entries, inet6Entries));
+        DefaultHostsFileEntriesResolver resolver = new DefaultHostsFileEntriesResolver(parser, ENTRIES_TTL);
 
         InetAddress address = resolver.address("localhost", ResolvedAddressTypes.IPV6_ONLY);
         assertNull(address, "Should pick an IPv6 address");
@@ -61,14 +73,12 @@ public void shouldntFindWhenAddressTypeDoesntMatch() {
 
     @Test
     public void shouldPickIpv4WhenBothAreDefinedButIpv4IsPreferred() {
-        Map<String, List<InetAddress>> inet4Entries = new HashMap<String, List<InetAddress>>();
-        Map<String, List<InetAddress>> inet6Entries = new HashMap<String, List<InetAddress>>();
+        HostsFileEntriesProvider.Parser parser = givenHostsParserWith(
+                LOCALHOST_V4_ADDRESSES,
+                LOCALHOST_V6_ADDRESSES
+        );
 
-        inet4Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
-        inet6Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
-
-        DefaultHostsFileEntriesResolver resolver =
-                new DefaultHostsFileEntriesResolver(new HostsFileEntriesProvider(inet4Entries, inet6Entries));
+        DefaultHostsFileEntriesResolver resolver = new DefaultHostsFileEntriesResolver(parser, ENTRIES_TTL);
 
         InetAddress address = resolver.address("localhost", ResolvedAddressTypes.IPV4_PREFERRED);
         assertThat("Should pick an IPv4 address", address, instanceOf(Inet4Address.class));
@@ -76,14 +86,12 @@ public void shouldPickIpv4WhenBothAreDefinedButIpv4IsPreferred() {
 
     @Test
     public void shouldPickIpv6WhenBothAreDefinedButIpv6IsPreferred() {
-        Map<String, List<InetAddress>> inet4Entries = new HashMap<String, List<InetAddress>>();
-        Map<String, List<InetAddress>> inet6Entries = new HashMap<String, List<InetAddress>>();
+        HostsFileEntriesProvider.Parser parser = givenHostsParserWith(
+                LOCALHOST_V4_ADDRESSES,
+                LOCALHOST_V6_ADDRESSES
+        );
 
-        inet4Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
-        inet6Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
-
-        DefaultHostsFileEntriesResolver resolver =
-                new DefaultHostsFileEntriesResolver(new HostsFileEntriesProvider(inet4Entries, inet6Entries));
+        DefaultHostsFileEntriesResolver resolver = new DefaultHostsFileEntriesResolver(parser, ENTRIES_TTL);
 
         InetAddress address = resolver.address("localhost", ResolvedAddressTypes.IPV6_PREFERRED);
         assertThat("Should pick an IPv6 address", address, instanceOf(Inet6Address.class));
@@ -91,13 +99,12 @@ public void shouldPickIpv6WhenBothAreDefinedButIpv6IsPreferred() {
 
     @Test
     public void shouldntFindWhenAddressesTypeDoesntMatch() {
-        Map<String, List<InetAddress>> inet4Entries = new HashMap<String, List<InetAddress>>();
-        Map<String, List<InetAddress>> inet6Entries = new HashMap<String, List<InetAddress>>();
+        HostsFileEntriesProvider.Parser parser = givenHostsParserWith(
+                LOCALHOST_V4_ADDRESSES,
+                Collections.<String, List<InetAddress>>emptyMap()
+        );
 
-        inet4Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
-
-        DefaultHostsFileEntriesResolver resolver =
-                new DefaultHostsFileEntriesResolver(new HostsFileEntriesProvider(inet4Entries, inet6Entries));
+        DefaultHostsFileEntriesResolver resolver = new DefaultHostsFileEntriesResolver(parser, ENTRIES_TTL);
 
         List<InetAddress> addresses = resolver.addresses("localhost", ResolvedAddressTypes.IPV6_ONLY);
         assertNull(addresses, "Should pick an IPv6 address");
@@ -105,14 +112,12 @@ public void shouldntFindWhenAddressesTypeDoesntMatch() {
 
     @Test
     public void shouldPickIpv4FirstWhenBothAreDefinedButIpv4IsPreferred() {
-        Map<String, List<InetAddress>> inet4Entries = new HashMap<String, List<InetAddress>>();
-        Map<String, List<InetAddress>> inet6Entries = new HashMap<String, List<InetAddress>>();
+        HostsFileEntriesProvider.Parser parser = givenHostsParserWith(
+                LOCALHOST_V4_ADDRESSES,
+                LOCALHOST_V6_ADDRESSES
+        );
 
-        inet4Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
-        inet6Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
-
-        DefaultHostsFileEntriesResolver resolver =
-                new DefaultHostsFileEntriesResolver(new HostsFileEntriesProvider(inet4Entries, inet6Entries));
+        DefaultHostsFileEntriesResolver resolver = new DefaultHostsFileEntriesResolver(parser, ENTRIES_TTL);
 
         List<InetAddress> addresses = resolver.addresses("localhost", ResolvedAddressTypes.IPV4_PREFERRED);
         assertNotNull(addresses);
@@ -123,14 +128,12 @@ public void shouldPickIpv4FirstWhenBothAreDefinedButIpv4IsPreferred() {
 
     @Test
     public void shouldPickIpv6FirstWhenBothAreDefinedButIpv6IsPreferred() {
-        Map<String, List<InetAddress>> inet4Entries = new HashMap<String, List<InetAddress>>();
-        Map<String, List<InetAddress>> inet6Entries = new HashMap<String, List<InetAddress>>();
+        HostsFileEntriesProvider.Parser parser = givenHostsParserWith(
+                LOCALHOST_V4_ADDRESSES,
+                LOCALHOST_V6_ADDRESSES
+        );
 
-        inet4Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
-        inet6Entries.put("localhost", Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
-
-        DefaultHostsFileEntriesResolver resolver =
-                new DefaultHostsFileEntriesResolver(new HostsFileEntriesProvider(inet4Entries, inet6Entries));
+        DefaultHostsFileEntriesResolver resolver = new DefaultHostsFileEntriesResolver(parser, ENTRIES_TTL);
 
         List<InetAddress> addresses = resolver.addresses("localhost", ResolvedAddressTypes.IPV6_PREFERRED);
         assertNotNull(addresses);
@@ -138,4 +141,54 @@ public void shouldPickIpv6FirstWhenBothAreDefinedButIpv6IsPreferred() {
         assertThat("Should pick an IPv6 address", addresses.get(0), instanceOf(Inet6Address.class));
         assertThat("Should pick an IPv4 address", addresses.get(1), instanceOf(Inet4Address.class));
     }
+
+    @Test
+    public void shouldNotRefreshHostsFileContentBeforeRefreshIntervalElapsed() {
+        Map<String, List<InetAddress>> v4Addresses = Maps.newHashMap(LOCALHOST_V4_ADDRESSES);
+        Map<String, List<InetAddress>> v6Addresses = Maps.newHashMap(LOCALHOST_V6_ADDRESSES);
+        DefaultHostsFileEntriesResolver resolver =
+                new DefaultHostsFileEntriesResolver(givenHostsParserWith(v4Addresses, v6Addresses), ENTRIES_TTL);
+        String newHost = UUID.randomUUID().toString();
+
+        v4Addresses.put(newHost, Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
+        v6Addresses.put(newHost, Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
+
+        assertNull(resolver.address(newHost, ResolvedAddressTypes.IPV4_ONLY));
+        assertNull(resolver.address(newHost, ResolvedAddressTypes.IPV6_ONLY));
+    }
+
+    @Test
+    public void shouldRefreshHostsFileContentAfterRefreshInterval() {
+        Map<String, List<InetAddress>> v4Addresses = Maps.newHashMap(LOCALHOST_V4_ADDRESSES);
+        Map<String, List<InetAddress>> v6Addresses = Maps.newHashMap(LOCALHOST_V6_ADDRESSES);
+        DefaultHostsFileEntriesResolver resolver =
+                new DefaultHostsFileEntriesResolver(givenHostsParserWith(v4Addresses, v6Addresses), -1);
+        String newHost = UUID.randomUUID().toString();
+
+        InetAddress address = resolver.address(newHost, ResolvedAddressTypes.IPV6_ONLY);
+        assertNull(address);
+
+        v4Addresses.put(newHost, Collections.<InetAddress>singletonList(NetUtil.LOCALHOST4));
+        v6Addresses.put(newHost, Collections.<InetAddress>singletonList(NetUtil.LOCALHOST6));
+
+        assertEquals(NetUtil.LOCALHOST4, resolver.address(newHost, ResolvedAddressTypes.IPV4_ONLY));
+        assertEquals(NetUtil.LOCALHOST6, resolver.address(newHost, ResolvedAddressTypes.IPV6_ONLY));
+    }
+
+    private HostsFileEntriesProvider.Parser givenHostsParserWith(final Map<String, List<InetAddress>> inet4Entries,
+                                                                 final Map<String, List<InetAddress>> inet6Entries) {
+        HostsFileEntriesProvider.Parser mockParser = mock(HostsFileEntriesProvider.Parser.class);
+
+        Answer<HostsFileEntriesProvider> mockedAnswer = new Answer<HostsFileEntriesProvider>() {
+            @Override
+            public HostsFileEntriesProvider answer(InvocationOnMock invocation) {
+                return new HostsFileEntriesProvider(inet4Entries, inet6Entries);
+            }
+        };
+
+        when(mockParser.parseSilently()).thenAnswer(mockedAnswer);
+        when(mockParser.parseSilently(any(Charset.class))).thenAnswer(mockedAnswer);
+
+        return mockParser;
+    }
 }