Repository: mageddo/dns-proxy-server
Pull request #406 (2023-04-02 03:27:43)

Adjusting and testing npe scenario


Patch:
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index 5f6aa0fc..2ffc0371 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -8,6 +8,7 @@ for more details
 * Fixed arm64 images stopped being pushed at `3.19`
 * Cache improvement for a concurrent approach synchronized by key
 * Increasing tcp and udp server parallelism from 20 to 50 threads each, threads are created lazilly
+* Fixing solver NPE on unsupported query types
 
 ## 3.14.5
 * Specify minimum required docker api version `1.21` (as DPS 2)
diff --git a/gradle.properties b/gradle.properties
index adde2074..aaee2625 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -1,1 +1,1 @@
-version=3.15.8-snapshot
+version=3.15.9-snapshot
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/Config.java b/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
index 129b4d71..b1c240d4 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
@@ -23,7 +23,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.mageddo.commons.lang.Objects.*;
+import static com.mageddo.commons.lang.Objects.mapOrNull;
 
 /**
  * Domain object which owns the configs.
@@ -176,7 +176,7 @@ public class Config {
       private final int type;
 
       public boolean isNot(Type... types) {
-        return isNot(this.type, types);
+        return Types.isNot(this.type, types);
       }
 
       public static Type of(Integer code) {
@@ -200,17 +200,6 @@ public class Config {
         return contains(of(type));
       }
 
-      public static boolean isNot(Integer code, Type... types) {
-        return !is(of(code), types);
-      }
-
-      public static boolean is(Type current, Type... possible) {
-        return Stream
-          .of(possible)
-          .collect(Collectors.toSet())
-          .contains(current);
-      }
-
       public IP.Version toVersion() {
         return switch (this) {
           case A -> IP.Version.IPV4;
@@ -220,7 +209,7 @@ public class Config {
       }
 
       public boolean isAddressSolving() {
-        return is(this, Config.Entry.Type.A, Config.Entry.Type.AAAA);
+        return Types.is(this, Config.Entry.Type.A, Config.Entry.Type.AAAA);
       }
     }
   }
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/Types.java b/src/main/java/com/mageddo/dnsproxyserver/config/Types.java
new file mode 100644
index 00000000..cde1f98c
--- /dev/null
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/Types.java
@@ -0,0 +1,27 @@
+package com.mageddo.dnsproxyserver.config;
+
+import com.mageddo.dnsproxyserver.config.Config.Entry.Type;
+
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Types {
+
+  public static boolean isNot(Type current, Type... possible) {
+    return !is(current, possible);
+  }
+
+  public static boolean is(Type current, Type... possible) {
+    if (current == null) {
+      return false;
+    }
+    return Stream
+      .of(possible)
+      .collect(Collectors.toSet())
+      .contains(current);
+  }
+
+  public static boolean isNot(Integer code, Type... types) {
+    return !is(Type.of(code), types);
+  }
+}
diff --git a/src/main/java/com/mageddo/dnsproxyserver/server/dns/Messages.java b/src/main/java/com/mageddo/dnsproxyserver/server/dns/Messages.java
index f6405eed..c46c5a90 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/server/dns/Messages.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/server/dns/Messages.java
@@ -107,7 +107,7 @@ public class Messages {
 
   public static Message aAnswer(Message query, String ip, long ttl) {
     final var res = withNoErrorResponse(query.clone());
-    if(StringUtils.isBlank(ip)){
+    if (StringUtils.isBlank(ip)) {
       return res;
     }
     final var answer = new ARecord(res.getQuestion().getName(), DClass.IN, ttl, Ips.toAddress(ip));
@@ -144,6 +144,10 @@ public class Messages {
     return Message.newQuery(query(host, Type.AAAA));
   }
 
+  public static Message soaQuestion(String hostname) {
+    return Message.newQuery(query(hostname, Type.SOA));
+  }
+
   @SneakyThrows
   public static Record query(final String host, final int type) {
     return Record.newRecord(Name.fromString(Hostnames.toAbsoluteName(host)), type, DClass.IN, 0);
diff --git a/src/main/java/com/mageddo/dnsproxyserver/server/dns/RequestHandlerDefault.java b/src/main/java/com/mageddo/dnsproxyserver/server/dns/RequestHandlerDefault.java
index 9952ecbd..b0ce2fb2 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/server/dns/RequestHandlerDefault.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/server/dns/RequestHandlerDefault.java
@@ -88,9 +88,9 @@ public class RequestHandlerDefault implements RequestHandler {
           return res;
         } catch (Exception e) {
           log.warn(
-            "status=solverFailed, currentSolverTime={}, totalTime={}, solver={}, eClass={}, msg={}",
+            "status=solverFailed, currentSolverTime={}, totalTime={}, solver={}, query={}, eClass={}, msg={}",
             stopWatch.getTime() - stopWatch.getSplitTime(), stopWatch.getTime(), solverName,
-            ClassUtils.getSimpleName(e), e.getMessage(), e
+            simplePrint(reqMsg), ClassUtils.getSimpleName(e), e.getMessage(), e
           );
         }
       }
diff --git a/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverDocker.java b/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverDocker.java
index 9b89bda5..258ac675 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverDocker.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverDocker.java
@@ -1,6 +1,7 @@
 package com.mageddo.dnsproxyserver.server.dns.solver;
 
 import com.mageddo.dnsproxyserver.config.Config.Entry.Type;
+import com.mageddo.dnsproxyserver.config.Types;
 import com.mageddo.dnsproxyserver.docker.ContainerSolvingService;
 import com.mageddo.dnsproxyserver.docker.DockerDAO;
 import com.mageddo.dnsproxyserver.server.dns.Messages;
@@ -27,14 +28,14 @@ public class SolverDocker implements Solver {
       return null;
     }
 
-    final var type = Messages.findQuestionTypeCode(query);
-    if (Type.isNot(type, Type.AAAA, Type.A)) {
+    final var type = Messages.findQuestionType(query);
+    if (Types.isNot(type, Type.AAAA, Type.A)) {
       log.trace("status=unsupportedType, type={}", type);
       return null;
     }
 
     final var askedHost = Messages.findQuestionHostname(query);
-    final var version = Type.of(type).toVersion();
+    final var version = type.toVersion();
     return HostnameMatcher.match(askedHost, version, hostname -> {
       final var entry = this.containerSolvingService.findBestMatch(hostname);
       if (!entry.isHostnameMatched()) {
diff --git a/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverLocalDB.java b/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverLocalDB.java
index 92210a68..28d82790 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverLocalDB.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverLocalDB.java
@@ -3,6 +3,7 @@ package com.mageddo.dnsproxyserver.server.dns.solver;
 import com.mageddo.dnsproxyserver.config.Config;
 import com.mageddo.dnsproxyserver.config.Config.Entry.Type;
 import com.mageddo.dnsproxyserver.config.ConfigDAO;
+import com.mageddo.dnsproxyserver.config.Types;
 import com.mageddo.dnsproxyserver.server.dns.Messages;
 import dagger.Lazy;
 import lombok.AllArgsConstructor;
@@ -28,8 +29,8 @@ public class SolverLocalDB implements Solver {
     final var stopWatch = StopWatch.createStarted();
 
     final var type = Messages.findQuestionTypeCode(query);
-    if (Type.isNot(type, Type.A, Type.CNAME, Type.AAAA)) {
-      log.trace("status=typeNotSupported, action=continue, type={}, time={}", type, stopWatch.getTime());
+    if (Types.isNot(type, Type.A, Type.CNAME, Type.AAAA)) {
+      log.debug("status=typeNotSupported, action=continue, type={}, time={}", type, stopWatch.getTime());
       return null;
     }
 
diff --git a/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystem.java b/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystem.java
index 658c1ea0..8fd2be9e 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystem.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystem.java
@@ -3,6 +3,7 @@ package com.mageddo.dnsproxyserver.server.dns.solver;
 import com.mageddo.commons.lang.Objects;
 import com.mageddo.dnsproxyserver.config.Config.Entry.Type;
 import com.mageddo.dnsproxyserver.config.Configs;
+import com.mageddo.dnsproxyserver.config.Types;
 import com.mageddo.dnsproxyserver.server.dns.Messages;
 import com.mageddo.dnsproxyserver.usecase.HostMachineService;
 import com.mageddo.net.IP;
@@ -13,6 +14,8 @@ import org.xbill.DNS.Message;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
+import static com.mageddo.dnsproxyserver.server.dns.Messages.findQuestionTypeCode;
+
 @Slf4j
 @Singleton
 @AllArgsConstructor(onConstructor = @__({@Inject}))
@@ -23,9 +26,10 @@ public class SolverSystem implements Solver {
   @Override
   public Response handle(Message query) {
     final var hostname = Messages.findQuestionHostname(query);
+
     final var questionType = Messages.findQuestionType(query);
-    if (questionType.isNot(Type.A, Type.AAAA)) {
-      log.debug("status=unsupportedType, type={}, query={}", questionType, Messages.simplePrint(query));
+    if (Types.isNot(questionType, Type.A, Type.AAAA)) {
+      log.debug("status=unsupportedType, type={}, query={}", findQuestionTypeCode(query), Messages.simplePrint(query));
       return null;
     }
     final var config = Configs.getInstance();


Test patch:
diff --git a/src/test/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystemTest.java b/src/test/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystemTest.java
new file mode 100644
index 00000000..859f3c7a
--- /dev/null
+++ b/src/test/java/com/mageddo/dnsproxyserver/server/dns/solver/SolverSystemTest.java
@@ -0,0 +1,31 @@
+package com.mageddo.dnsproxyserver.server.dns.solver;
+
+import com.mageddo.dnsproxyserver.templates.MessageTemplates;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Spy;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+@ExtendWith(MockitoExtension.class)
+class SolverSystemTest {
+
+  @Spy
+  @InjectMocks
+  SolverSystem solver;
+
+  @Test
+  void mustReturnNullWhenTypeIsNotSupported() {
+    // arrange
+    final var query = MessageTemplates.acmeSoaQuery();
+
+    // act
+    final var res = this.solver.handle(query);
+
+    // assert
+    assertNull(res);
+  }
+
+}
diff --git a/src/test/java/com/mageddo/dnsproxyserver/templates/MessageTemplates.java b/src/test/java/com/mageddo/dnsproxyserver/templates/MessageTemplates.java
index 35883112..38bc7dc8 100644
--- a/src/test/java/com/mageddo/dnsproxyserver/templates/MessageTemplates.java
+++ b/src/test/java/com/mageddo/dnsproxyserver/templates/MessageTemplates.java
@@ -30,4 +30,8 @@ public class MessageTemplates {
   public static Message acmeNxDomain() {
     return Messages.nxDomain(MessageTemplates.acmeAQuery());
   }
+
+  public static Message acmeSoaQuery() {
+    return Messages.soaQuestion(ACME_HOSTNAME);
+  }
 }