Skip to content

Hosts parser change #371

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.

Sign up for GitHub

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

Merged
ibauersachs merged 2 commits into from
Jun 28, 2025
Merged

Hosts parser change #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b62222c
Reduce hosts file parsing warnings
ibauersachs Jan 18, 2025
60029c1
Reduce lock contention and disk access in hosts file parsing
ibauersachs Jan 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 105 additions & 26 deletions src/main/java/org/xbill/DNS/hosts/HostsFileParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.xbill.DNS.Address;
Expand All @@ -31,12 +34,23 @@
public final class HostsFileParser {
private final int maxFullCacheFileSizeBytes =
Integer.parseInt(System.getProperty("dnsjava.hostsfile.max_size_bytes", "16384"));
private final Duration fileChangeCheckInterval =
Duration.ofMillis(
Integer.parseInt(
System.getProperty("dnsjava.hostsfile.change_check_interval_ms", "300000")));

private final Map<String, InetAddress> hostsCache = new HashMap<>();
private final Path path;
private final boolean clearCacheOnChange;
private Clock clock = Clock.systemUTC();

@SuppressWarnings("java:S3077")
private volatile Map<String, InetAddress> hostsCache;

private Instant lastFileModificationCheckTime = Instant.MIN;
private Instant lastFileReadTime = Instant.MIN;
private boolean isEntireFileParsed;
private boolean hostsFileWarningLogged = false;
private long hostsFileSizeBytes;

/**
* Creates a new instance based on the current OS's default. Unix and alike (or rather everything
Expand Down Expand Up @@ -86,8 +100,7 @@
* @throws IllegalArgumentException when {@code type} is not {@link org.xbill.DNS.Type#A} or{@link
* org.xbill.DNS.Type#AAAA}.
*/
public synchronized Optional<InetAddress> getAddressForHost(Name name, int type)
throws IOException {
public Optional<InetAddress> getAddressForHost(Name name, int type) throws IOException {
Objects.requireNonNull(name, "name is required");
if (type != Type.A && type != Type.AAAA) {
throw new IllegalArgumentException("type can only be A or AAAA");
Expand All @@ -100,13 +113,11 @@
return Optional.of(cachedAddress);
}

if (isEntireFileParsed || !Files.exists(path)) {
if (isEntireFileParsed) {
return Optional.empty();
}

if (Files.size(path) <= maxFullCacheFileSizeBytes) {
parseEntireHostsFile();
} else {
if (hostsFileSizeBytes > maxFullCacheFileSizeBytes) {
searchHostsFileForEntry(name, type);
}

Expand All @@ -116,9 +127,11 @@
private void parseEntireHostsFile() throws IOException {
String line;
int lineNumber = 0;
AtomicInteger addressFailures = new AtomicInteger(0);
AtomicInteger nameFailures = new AtomicInteger(0);
try (BufferedReader hostsReader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
while ((line = hostsReader.readLine()) != null) {
LineData lineData = parseLine(++lineNumber, line);
LineData lineData = parseLine(++lineNumber, line, addressFailures, nameFailures);
if (lineData != null) {
for (Name lineName : lineData.names) {
InetAddress lineAddress =
Expand All @@ -129,15 +142,24 @@
}
}

isEntireFileParsed = true;
if (!hostsFileWarningLogged && (addressFailures.get() > 0 || nameFailures.get() > 0)) {
log.warn(
"Failed to parse entire hosts file {}, address failures={}, name failures={}",
path,
addressFailures.get(),
nameFailures);
hostsFileWarningLogged = true;
}
}

private void searchHostsFileForEntry(Name name, int type) throws IOException {
String line;
int lineNumber = 0;
AtomicInteger addressFailures = new AtomicInteger(0);
AtomicInteger nameFailures = new AtomicInteger(0);
try (BufferedReader hostsReader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
while ((line = hostsReader.readLine()) != null) {
LineData lineData = parseLine(++lineNumber, line);
LineData lineData = parseLine(++lineNumber, line, addressFailures, nameFailures);
if (lineData != null) {
for (Name lineName : lineData.names) {
boolean isSearchedEntry = lineName.equals(name);
Expand All @@ -151,6 +173,16 @@
}
}
}

if (!hostsFileWarningLogged && (addressFailures.get() > 0 || nameFailures.get() > 0)) {
log.warn(
"Failed to find {} in hosts file {}, address failures={}, name failures={}",
name,
path,
addressFailures.get(),
nameFailures);
hostsFileWarningLogged = true;
}
}

@RequiredArgsConstructor
Expand All @@ -160,7 +192,8 @@
final Iterable<? extends Name> names;
}

private LineData parseLine(int lineNumber, String line) {
private LineData parseLine(
int lineNumber, String line, AtomicInteger addressFailures, AtomicInteger nameFailures) {
String[] lineTokens = getLineTokens(line);
if (lineTokens.length < 2) {
return null;
Expand All @@ -174,24 +207,26 @@
}

if (lineAddressBytes == null) {
log.warn("Could not decode address {}, {}#L{}", lineTokens[0], path, lineNumber);
log.debug("Could not decode address {}, {}#L{}", lineTokens[0], path, lineNumber);
addressFailures.incrementAndGet();
return null;
}

Iterable<? extends Name> lineNames =
Arrays.stream(lineTokens)
.skip(1)
.map(lineTokenName -> safeName(lineTokenName, lineNumber))
.map(lineTokenName -> safeName(lineTokenName, lineNumber, nameFailures))
.filter(Objects::nonNull)
::iterator;
return new LineData(lineAddressType, lineAddressBytes, lineNames);
}

private Name safeName(String name, int lineNumber) {
private Name safeName(String name, int lineNumber, AtomicInteger nameFailures) {
try {
return Name.fromString(name, Name.root);
} catch (TextParseException e) {
log.warn("Could not decode name {}, {}#L{}, skipping", name, path, lineNumber);
log.debug("Could not decode name {}, {}#L{}, skipping", name, path, lineNumber);
nameFailures.incrementAndGet();
return null;
}
}
Expand All @@ -207,21 +242,61 @@
}

private void validateCache() throws IOException {
if (clearCacheOnChange) {
if (!clearCacheOnChange) {
if (hostsCache == null) {
synchronized (this) {
if (hostsCache == null) {
readHostsFile();
}
}
}

return;
}

if (lastFileModificationCheckTime.plus(fileChangeCheckInterval).isBefore(clock.instant())) {
log.debug("Checked for changes more than 5minutes ago, checking");
// A filewatcher / inotify etc. would be nicer, but doesn't work. c.f. the write up at
// https://blog.arkey.fr/2019/09/13/watchservice-and-bind-mount/
Instant fileTime =
Files.exists(path) ? Files.getLastModifiedTime(path).toInstant() : Instant.MAX;
if (fileTime.isAfter(lastFileReadTime)) {
// skip logging noise when the cache is empty anyway
if (!hostsCache.isEmpty()) {
log.info("Local hosts database has changed at {}, clearing cache", fileTime);
hostsCache.clear();

synchronized (this) {
if (!lastFileModificationCheckTime
.plus(fileChangeCheckInterval)
.isBefore(clock.instant())) {
log.debug("Never mind, check fulfilled in another thread");
return;

Check warning on line 267 in src/main/java/org/xbill/DNS/hosts/HostsFileParser.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/xbill/DNS/hosts/HostsFileParser.java#L266-L267 

Added lines #L266 - L267 were not covered by tests
}

lastFileModificationCheckTime = clock.instant();
readHostsFile();
}
}
}

private void readHostsFile() throws IOException {
if (Files.exists(path)) {
Instant fileTime = Files.getLastModifiedTime(path).toInstant();
if (!lastFileReadTime.equals(fileTime)) {
createOrClearCache();

hostsFileSizeBytes = Files.size(path);
if (hostsFileSizeBytes <= maxFullCacheFileSizeBytes) {
parseEntireHostsFile();
isEntireFileParsed = true;
}

isEntireFileParsed = false;
lastFileReadTime = fileTime;
}
} else {
createOrClearCache();
}
}

private void createOrClearCache() {
if (hostsCache == null) {
hostsCache = new ConcurrentHashMap<>();
} else {
hostsCache.clear();
}
}

Expand All @@ -231,6 +306,10 @@

// for unit testing only
int cacheSize() {
return hostsCache.size();
return hostsCache == null ? 0 : hostsCache.size();
}

void setClock(Clock clock) {
this.clock = clock;
}
}
17 changes: 16 additions & 1 deletion src/test/java/org/xbill/DNS/hosts/HostsFileParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.io.BufferedWriter;
import java.io.IOException;
Expand All @@ -18,6 +20,9 @@
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.FileTime;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.Optional;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -72,7 +77,7 @@ void testMissingFileIsEmptyResult() throws IOException {
}

@Test
void testCacheLookup() throws IOException {
void testCacheLookupAfterFileDeleteWithoutChangeChecking() throws IOException {
Path tempHosts = Files.copy(hostsFileWindows, tempDir, StandardCopyOption.REPLACE_EXISTING);
HostsFileParser hostsFileParser = new HostsFileParser(tempHosts, false);
assertEquals(0, hostsFileParser.cacheSize());
Expand All @@ -98,6 +103,10 @@ void testFileDeletionClearsCache() throws IOException {
tempDir.resolve("testFileWatcherClearsCache"),
StandardCopyOption.REPLACE_EXISTING);
HostsFileParser hostsFileParser = new HostsFileParser(tempHosts);
Clock clock = mock(Clock.class);
hostsFileParser.setClock(clock);
Instant now = Clock.systemUTC().instant();
when(clock.instant()).thenReturn(now);
assertEquals(0, hostsFileParser.cacheSize());
assertEquals(
kubernetesAddress,
Expand All @@ -106,6 +115,7 @@ void testFileDeletionClearsCache() throws IOException {
.orElseThrow(() -> new IllegalStateException("Host entry not found")));
assertTrue(hostsFileParser.cacheSize() > 1, "Cache must not be empty");
Files.delete(tempHosts);
when(clock.instant()).thenReturn(now.plus(Duration.ofMinutes(6)));
assertEquals(Optional.empty(), hostsFileParser.getAddressForHost(kubernetesName, Type.A));
assertEquals(0, hostsFileParser.cacheSize());
}
Expand All @@ -119,6 +129,10 @@ void testFileChangeClearsCache() throws IOException {
StandardCopyOption.REPLACE_EXISTING);
Files.setLastModifiedTime(tempHosts, FileTime.fromMillis(0));
HostsFileParser hostsFileParser = new HostsFileParser(tempHosts);
Clock clock = mock(Clock.class);
hostsFileParser.setClock(clock);
Instant now = Clock.systemUTC().instant();
when(clock.instant()).thenReturn(now);
assertEquals(0, hostsFileParser.cacheSize());
assertEquals(
kubernetesAddress,
Expand All @@ -134,6 +148,7 @@ void testFileChangeClearsCache() throws IOException {
}

Files.setLastModifiedTime(tempHosts, FileTime.fromMillis(10_0000));
when(clock.instant()).thenReturn(now.plus(Duration.ofMinutes(6)));
assertEquals(
InetAddress.getByAddress(testName.toString(), localhostBytes),
hostsFileParser
Expand Down
Loading