diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 4df26069b..fce5fa3c2 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,4 +1,4 @@ -## 5.1.0 +## 5.1.2 * Changed resolv.conf handling format to make it compatible with alpine linux see [#627][5_1_0_1][1]. * Refactoring Linux amd64 static build to work on Github Actions * Creating the docs for config v3 diff --git a/gradle.properties b/gradle.properties index af1ed0e58..ee85810fd 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1 +1 @@ -version=5.1.0-snapshot +version=5.1.2-snapshot diff --git a/src/main/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2.java b/src/main/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2.java index 9c0ab58fe..410d75627 100644 --- a/src/main/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2.java +++ b/src/main/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2.java @@ -26,8 +26,11 @@ public static void process(final Path confFile, final IpAddr addr) { process(confFile, addr, true); } - public static void process(final Path confFile, final IpAddr addr, - final boolean overrideNameServers) { + public static void process( + final Path confFile, + final IpAddr addr, + final boolean overrideNameServers + ) { Dns.validateIsDefaultPort(addr); @@ -52,38 +55,46 @@ public static void restore(final Path confFile) { // Build outputs // ------------------------------------------------------------------------- + /** + * overrideNameServers=true: + * - write dps-entries after header comments + * - create dps-comments with ALL existing active/commented nameservers (dedup), excluding dps server + * - preserve any non-nameserver lines (comments/options/search/etc.) + * - do NOT emit inline "# ... # dps-comment" (tests expect only blocks) + */ private static String buildOverrideOutput( final String dpsNameserverHost, final CleanedContent cleaned ) { - final var nameserversToComment = - collectNameserversToComment(dpsNameserverHost, cleaned); + final var nameserversToComment = collectNameserversToComment(dpsNameserverHost, cleaned); - final var out = new StringBuilder(); + final var lines = cleaned.originalLines(); + final var insertionIndex = indexAfterHeaderComments(lines); - append(out, BEGIN_ENTRIES); - append(out, nameserverLine(dpsNameserverHost)); - append(out, END_ENTRIES); + final var prefix = new ArrayList<>(lines.subList(0, insertionIndex)); + final var suffix = removeActiveNameserverLines(lines.subList(insertionIndex, lines.size())); - if (nameserversToComment.isEmpty()) { - return out.toString(); - } + final var outLines = new ArrayList(); + outLines.addAll(prefix); - append(out, ""); - append(out, BEGIN_COMMENTS); + outLines.add(BEGIN_ENTRIES); + outLines.add(nameserverLine(dpsNameserverHost)); + outLines.add(END_ENTRIES); - for (final var ns : nameserversToComment) { - append(out, commentedNameserverLine(ns)); + if (!nameserversToComment.isEmpty()) { + outLines.add(BEGIN_COMMENTS); + for (final var ns : nameserversToComment) { + outLines.add(commentedNameserverLine(ns)); + } + outLines.add(END_COMMENTS); } - append(out, END_COMMENTS); + // preserve remaining config (options/search/etc.) after the blocks + outLines.addAll(suffix); - return out.toString(); - } - - private static void append(final StringBuilder out, final String value) { - out.append(value) - .append(LINE_BREAK); + normalizeBlankLinesAroundDpsMarkers(outLines); + trimTrailingBlankLines(outLines); + return joinLines(outLines) + LINE_BREAK; } private static String buildNonOverrideOutput( @@ -92,19 +103,29 @@ private static String buildNonOverrideOutput( final var lines = cleaned.originalLines(); final var insertionIndex = indexAfterHeaderComments(lines); - final var out = new ArrayList<>(lines.subList(0, insertionIndex)); - ensureBlankLine(out); - out.add(BEGIN_ENTRIES); - out.add(nameserverLine(dpsNameserverHost)); - out.add(END_ENTRIES); - out.add(""); + final var outLines = new ArrayList(lines.subList(0, insertionIndex)); + + outLines.add(BEGIN_ENTRIES); + outLines.add(nameserverLine(dpsNameserverHost)); + outLines.add(END_ENTRIES); - final var remainderStart = skipBlankLines(lines, insertionIndex); - out.addAll(lines.subList(remainderStart, lines.size())); + outLines.addAll(lines.subList(insertionIndex, lines.size())); - trimTrailingBlankLines(out); - return joinLines(out) + LINE_BREAK; + normalizeBlankLinesAroundDpsMarkers(outLines); + trimTrailingBlankLines(outLines); + return joinLines(outLines) + LINE_BREAK; + } + + private static List removeActiveNameserverLines(final List lines) { + final var out = new ArrayList(); + for (final var line : lines) { + if (extractActiveNameserver(line) != null) { + continue; + } + out.add(line); + } + return out; } private static List collectNameserversToComment( @@ -112,14 +133,19 @@ private static List collectNameserversToComment( ) { final var nameservers = new LinkedHashSet(); - // inline "# ... # dps-comment" captured during cleanup + // candidates from inline "# ... # dps-comment" and from previous dps-comments blocks nameservers.addAll(cleaned.inlineCommentCandidates()); - // remaining active nameservers in the file for (final var line : cleaned.originalLines()) { - final var ns = extractActiveNameserver(line); - if (ns != null) { - nameservers.add(ns); + final var active = extractActiveNameserver(line); + if (active != null) { + nameservers.add(active); + continue; + } + + final var commented = extractCommentedNameserver(line); + if (commented != null) { + nameservers.add(commented); } } @@ -130,8 +156,7 @@ private static List collectNameserversToComment( private static int indexAfterHeaderComments(final List lines) { int i = 0; while (i < lines.size()) { - final var trimmed = lines.get(i) - .trim(); + final var trimmed = lines.get(i).trim(); if (!trimmed.startsWith("#")) { break; } @@ -143,25 +168,85 @@ private static int indexAfterHeaderComments(final List lines) { return i; } - private static int skipBlankLines(final List lines, final int startIndex) { - int i = startIndex; - while (i < lines.size() && lines.get(i) - .isBlank()) { - i++; - } - return i; - } - - private static void ensureBlankLine(final List lines) { + /** + * Regra: Antes de todo BEGIN e depois de todo END terá exatamente 1 linha em branco, + * exceto se BEGIN for a 1ª linha do arquivo ou END for a última linha do arquivo. + * + * Também remove duplicações de linhas em branco nesses pontos. + */ + private static void normalizeBlankLinesAroundDpsMarkers(final List lines) { if (lines.isEmpty()) { return; } - if (!lines.getLast() - .isBlank()) { - lines.add(""); + + // 1) normalize "before BEGIN" + for (int i = 0; i < lines.size(); i++) { + if (!isBeginMarker(lines.get(i))) { + continue; + } + if (i == 0) { + continue; // start of file + } + + // collapse blanks immediately before BEGIN to exactly one + var j = i - 1; + while (j >= 0 && lines.get(j).isBlank()) { + lines.remove(j); + i--; + j--; + } + + // ensure one blank line before BEGIN (unless start) + if (i > 0 && !lines.get(i - 1).isBlank()) { + lines.add(i, ""); + i++; + } + } + + // 2) normalize "after END" + for (int i = 0; i < lines.size(); i++) { + if (!isEndMarker(lines.get(i))) { + continue; + } + if (i == lines.size() - 1) { + continue; // end of file + } + + // remove all blanks immediately after END + while (i + 1 < lines.size() && lines.get(i + 1).isBlank()) { + lines.remove(i + 1); + } + + // ensure one blank after END if it's not end-of-file + if (i + 1 < lines.size()) { + lines.add(i + 1, ""); + } + } + + // 3) final cleanup: remove leading blanks and collapse multiple consecutive blanks globally + trimLeadingBlankLines(lines); + collapseConsecutiveBlankLines(lines); + } + + private static void collapseConsecutiveBlankLines(final List lines) { + for (int i = 1; i < lines.size(); i++) { + if (lines.get(i).isBlank() && lines.get(i - 1).isBlank()) { + lines.remove(i); + i--; + } } } + private static boolean isBeginMarker(final String line) { + final var trimmed = line.trim(); + return trimmed.equals(BEGIN_ENTRIES) || trimmed.equals(BEGIN_COMMENTS); + } + + private static boolean isEndMarker(final String line) { + final var trimmed = line.trim(); + return trimmed.equals(END_ENTRIES) || trimmed.equals(END_COMMENTS); + } + // ------------------------------------------------------------------------- // Remove / Restore DPS artifacts // ------------------------------------------------------------------------- @@ -170,7 +255,7 @@ private static CleanedContent removeDpsArtifacts(final String normalizedContent) final var lines = splitLines(normalizedContent); final var cleaned = new ArrayList(); - final var inlineCommentCandidates = new LinkedHashSet(); + final var commentCandidates = new LinkedHashSet(); boolean insideEntriesBlock = false; boolean insideCommentsBlock = false; @@ -195,8 +280,18 @@ private static CleanedContent removeDpsArtifacts(final String normalizedContent) continue; } - // drop managed blocks completely (both entries and comments) - if (insideEntriesBlock || insideCommentsBlock) { + // Collect nameservers from previous dps-comments blocks (idempotency) + if (insideCommentsBlock) { + final var uncommented = uncommentNameserverIfPresent(line); // "# nameserver X" -> "nameserver X" + final var ns = uncommented == null ? null : extractActiveNameserver(uncommented); + if (ns != null) { + commentCandidates.add(ns); + } + continue; + } + + // drop managed entries block completely + if (insideEntriesBlock) { continue; } @@ -210,7 +305,7 @@ private static CleanedContent removeDpsArtifacts(final String normalizedContent) final var restored = restoreInlineDpsComment(line); final var ns = restored == null ? null : extractActiveNameserver(restored); if (ns != null) { - inlineCommentCandidates.add(ns); + commentCandidates.add(ns); } continue; } @@ -221,7 +316,7 @@ private static CleanedContent removeDpsArtifacts(final String normalizedContent) trimLeadingBlankLines(cleaned); trimTrailingBlankLines(cleaned); - return new CleanedContent(cleaned, new ArrayList<>(inlineCommentCandidates)); + return new CleanedContent(cleaned, new ArrayList<>(commentCandidates)); } private static String restoreFromContent(final String normalizedContent) { @@ -231,6 +326,9 @@ private static String restoreFromContent(final String normalizedContent) { boolean insideEntriesBlock = false; boolean insideCommentsBlock = false; + // nameservers read from the dps-comments block; emitted when block ends + final var nameserversFromBlock = new ArrayList(); + for (final var line : lines) { final var trimmed = line.trim(); @@ -244,10 +342,19 @@ private static String restoreFromContent(final String normalizedContent) { } if (trimmed.equals(BEGIN_COMMENTS)) { insideCommentsBlock = true; + nameserversFromBlock.clear(); continue; } if (trimmed.equals(END_COMMENTS)) { insideCommentsBlock = false; + + // emit restored nameservers exactly where the block was + final var seen = new LinkedHashSet(); + for (final var ns : nameserversFromBlock) { + if (seen.add(ns)) { + restored.add(nameserverLine(ns)); + } + } continue; } @@ -256,9 +363,10 @@ private static String restoreFromContent(final String normalizedContent) { } if (insideCommentsBlock) { - final var restoredLine = uncommentNameserverIfPresent(line); - if (restoredLine != null && !restoredLine.isBlank()) { - restored.add(restoredLine); + final var uncommented = uncommentNameserverIfPresent(line); // "# nameserver X" -> "nameserver X" + final var ns = uncommented == null ? null : extractActiveNameserver(uncommented); + if (ns != null) { + nameserversFromBlock.add(ns); } continue; } @@ -275,11 +383,15 @@ private static String restoreFromContent(final String normalizedContent) { continue; } - if (!line.isBlank()) { - restored.add(line); + // IMPORTANT: tests expect blank lines removed on restore + if (line.isBlank()) { + continue; } + + restored.add(line); } + trimTrailingBlankLines(restored); return joinLines(restored) + LINE_BREAK; } @@ -300,24 +412,30 @@ private static String extractActiveNameserver(final String line) { return parts.length >= 2 ? parts[1].trim() : null; } + private static String extractCommentedNameserver(final String line) { + final var trimmed = line.trim(); + if (!trimmed.startsWith("#")) { + return null; + } + final var uncommented = trimmed.substring(1).trim(); // remove '#' + return extractActiveNameserver(uncommented); // "nameserver X" + } + private static String uncommentNameserverIfPresent(final String line) { var trimmed = line.trim(); if (!trimmed.startsWith("#")) { return null; } - trimmed = trimmed.substring(1) - .trim(); + trimmed = trimmed.substring(1).trim(); return trimmed.startsWith("nameserver") ? trimmed : null; } private static String restoreInlineDpsComment(final String line) { // "# nameserver 8.8.8.8 # dps-comment" -> "nameserver 8.8.8.8" - final var withoutSuffix = line.replace(DPS_COMMENT_SUFFIX, "") - .trim(); + final var withoutSuffix = line.replace(DPS_COMMENT_SUFFIX, "").trim(); var trimmed = withoutSuffix.trim(); if (trimmed.startsWith("#")) { - trimmed = trimmed.substring(1) - .trim(); + trimmed = trimmed.substring(1).trim(); } return trimmed; } @@ -377,8 +495,7 @@ private static void writeString(final Path path, final String content) { } private static String normalizeNewlines(final String s) { - return s.replace("\r\n", LINE_BREAK) - .replace("\r", LINE_BREAK); + return s.replace("\r\n", LINE_BREAK).replace("\r", LINE_BREAK); } private static List splitLines(final String normalizedContent) { @@ -403,15 +520,13 @@ private static String joinLines(final List lines) { } private static void trimLeadingBlankLines(final List lines) { - while (!lines.isEmpty() && lines.getFirst() - .isBlank()) { + while (!lines.isEmpty() && lines.getFirst().isBlank()) { lines.removeFirst(); } } private static void trimTrailingBlankLines(final List lines) { - while (!lines.isEmpty() && lines.getLast() - .isBlank()) { + while (!lines.isEmpty() && lines.getLast().isBlank()) { lines.removeLast(); } } diff --git a/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvConfTemplates.java b/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvConfTemplates.java new file mode 100644 index 000000000..9c7e191df --- /dev/null +++ b/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvConfTemplates.java @@ -0,0 +1,93 @@ +package com.mageddo.dnsproxyserver.dnsconfigurator.linux; + +public class ResolvConfTemplates { + public static String defaultSystemResolvedConfWithDpsV1() { + return """ + # This is /run/systemd/resolve/stub-resolv.conf managed by man:systemd-resolved(8). + # Do not edit. + # + # This file might be symlinked as /etc/resolv.conf. If you're looking at + # /etc/resolv.conf and seeing this text, you have followed the symlink. + # + # This is a dynamic resolv.conf file for connecting local clients to the + # internal DNS stub resolver of systemd-resolved. This file lists all + # configured search domains. + # + # Run "resolvectl status" to see details about the uplink DNS servers + # currently in use. + # + # Third party programs should typically not access this file directly, but only + # through the symlink at /etc/resolv.conf. To manage man:resolv.conf(5) in a + # different way, replace this symlink by a static file or a different symlink. + # + # See man:systemd-resolved.service(8) for details about the supported modes of + # operation for /etc/resolv.conf. + + # nameserver 127.0.0.53 # dps-comment + options edns0 trust-ad + search . + nameserver 192.168.0.227 # dps-entry + """; + } + public static String defaultSystemResolvedConf() { + return """ + # This is /run/systemd/resolve/stub-resolv.conf managed by man:systemd-resolved(8). + # Do not edit. + # + # This file might be symlinked as /etc/resolv.conf. If you're looking at + # /etc/resolv.conf and seeing this text, you have followed the symlink. + # + # This is a dynamic resolv.conf file for connecting local clients to the + # internal DNS stub resolver of systemd-resolved. This file lists all + # configured search domains. + # + # Run "resolvectl status" to see details about the uplink DNS servers + # currently in use. + # + # Third party programs should typically not access this file directly, but only + # through the symlink at /etc/resolv.conf. To manage man:resolv.conf(5) in a + # different way, replace this symlink by a static file or a different symlink. + # + # See man:systemd-resolved.service(8) for details about the supported modes of + # operation for /etc/resolv.conf. + nameserver 127.0.0.53 + options edns0 trust-ad + search . + """; + } + + public static String defaultSystemResolvedConfDpsChanged() { + return """ + # This is /run/systemd/resolve/stub-resolv.conf managed by man:systemd-resolved(8). + # Do not edit. + # + # This file might be symlinked as /etc/resolv.conf. If you're looking at + # /etc/resolv.conf and seeing this text, you have followed the symlink. + # + # This is a dynamic resolv.conf file for connecting local clients to the + # internal DNS stub resolver of systemd-resolved. This file lists all + # configured search domains. + # + # Run "resolvectl status" to see details about the uplink DNS servers + # currently in use. + # + # Third party programs should typically not access this file directly, but only + # through the symlink at /etc/resolv.conf. To manage man:resolv.conf(5) in a + # different way, replace this symlink by a static file or a different symlink. + # + # See man:systemd-resolved.service(8) for details about the supported modes of + # operation for /etc/resolv.conf. + + # BEGIN dps-entries + nameserver 10.10.0.1 + # END dps-entries + + # BEGIN dps-comments + # nameserver 127.0.0.53 + # END dps-comments + + options edns0 trust-ad + search . + """; + } +} diff --git a/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2Test.java b/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2Test.java index 309279be7..535cefcff 100644 --- a/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2Test.java +++ b/src/test/java/com/mageddo/dnsproxyserver/dnsconfigurator/linux/ResolvconfConfiguratorV2Test.java @@ -34,7 +34,6 @@ void mustConfigureDpsServerOnEmptyFile(@TempDir Path tmpDir) throws Exception { } - @Test void mustCleanUpDpsCommentsAndEntriesBeforeApply(@TempDir Path tmpDir) throws Exception { @@ -55,7 +54,8 @@ void mustCleanUpDpsCommentsAndEntriesBeforeApply(@TempDir Path tmpDir) throws Ex # nameserver 8.8.4.4 # dps-comment nameserver 8.8.8.8 - """); + """ + ); // act ResolvconfConfiguratorV2.process(resolvFile, ip); @@ -68,6 +68,7 @@ void mustCleanUpDpsCommentsAndEntriesBeforeApply(@TempDir Path tmpDir) throws Ex # END dps-entries # BEGIN dps-comments + # nameserver 5.5.5.5 # nameserver 8.8.8.8 # nameserver 8.8.4.4 # END dps-comments @@ -261,4 +262,71 @@ void mustCreateExactlyOneDpsEntryEvenWhenCalledTwice(@TempDir Path tmpDir) Files.readString(resolvFile) ); } + + @Test + void mustPreserveOriginalSettingsAndCommentsAfterRestore( + @TempDir Path tmpDir + ) throws Exception { + + // arrange + final var resolvFile = tmpDir.resolve("resolv.conf"); + final var ip = IpAddrTemplates.local(); + + final var original = ResolvConfTemplates.defaultSystemResolvedConf(); + Files.writeString(resolvFile, original); + + // act + ResolvconfConfiguratorV2.process(resolvFile, ip, true); + ResolvconfConfiguratorV2.restore(resolvFile); + + // assert + assertEquals(original, Files.readString(resolvFile)); + } + + @Test + void mustPreserveOriginalSettingsAndComments( + @TempDir Path tmpDir + ) throws Exception { + + // arrange + final var resolvFile = tmpDir.resolve("resolv.conf"); + final var ip = IpAddrTemplates.local(); + + final var original = ResolvConfTemplates.defaultSystemResolvedConf(); + Files.writeString(resolvFile, original); + + // act + final var overrideNameServers = true; + ResolvconfConfiguratorV2.process(resolvFile, ip, overrideNameServers); + ResolvconfConfiguratorV2.process(resolvFile, ip, overrideNameServers); + + // assert + assertEquals( + ResolvConfTemplates.defaultSystemResolvedConfDpsChanged(), + Files.readString(resolvFile) + ); + } + + @Test + void mustPreserveOriginalSettingsAndCommentsWhenContainsDpsV1( + @TempDir Path tmpDir + ) throws Exception { + + // arrange + final var resolvFile = tmpDir.resolve("resolv.conf"); + final var ip = IpAddrTemplates.local(); + + final var original = ResolvConfTemplates.defaultSystemResolvedConfWithDpsV1(); + Files.writeString(resolvFile, original); + + // act + final var overrideNameServers = true; + ResolvconfConfiguratorV2.process(resolvFile, ip, overrideNameServers); + + // assert + assertEquals( + ResolvConfTemplates.defaultSystemResolvedConfDpsChanged(), + Files.readString(resolvFile) + ); + } }