#129 protection vs null pointer (#134)

Tyreviel · web-flow · commit e46bb6bc66c6 · 2026-02-26T11:52:53.000+01:00
* Added LogContext and ServerLogFormatter, amended ServerLogging to use new formatter. * Minor change to ConnectionHandler to generate and set connection ID, and LoggingFilter to use the logger, ensuring connection ID is included in the logs * created a test, ConnectionIdLoggingTest, to verify that the connection ID is correctly included in the logs * Updated LoggingFilterTest to check the java.util.logging.Logger instead of System.out, reflecting recent changes to LoggingFilter * Updated LoggingFilterTest to check the java.util.logging.Logger instead of System.out, reflecting recent changes to LoggingFilter (#95) * Apply Spotless formatting to resolve CI failure (#95) * Include stack traces in log output as suggested by code review (#95) * build: add JSpecify, Error Prone and NullAway for null safety * feat: add NullMarked to all packages * fix: resolve nullability issues identified by NullAway * build: update Error Prone/NullAway versions and fix compiler args * fix: additional nullability issues in StaticFileHandler and ConfigLoader * fix: resolve remaining nullability issues in tests and supporting classes * style: apply spotless formatting fixes * fix: mark LogContext.getConnectionId as @nullable * style: fix formatting in LogContext.java * test: pin logger level and use SimpleFormatter in LoggingFilterTest * refactor: update log formatter to use record instant and add null protection to ConnectionHandler * fix: resolve NullAway errors in HttpResponse, CorsFilter and CorsFilterTest
diff --git a/pom.xml b/pom.xml
@@ -21,6 +21,8 @@
         <junit.jupiter.version>6.0.2</junit.jupiter.version>
         <assertj.core.version>3.27.7</assertj.core.version>
         <mockito.version>5.21.0</mockito.version>
+        <errorprone.version>2.45.0</errorprone.version>
+        <nullaway.version>0.13.1</nullaway.version>
         <build.time>${maven.build.timestamp}</build.time>
         <argLine/>
     </properties>
@@ -72,6 +74,11 @@
             <version>1.21.4</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.jspecify</groupId>
+            <artifactId>jspecify</artifactId>
+            <version>1.0.0</version>
+        </dependency>
     </dependencies>
     <build>
         <plugins>
@@ -84,6 +91,36 @@
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.15.0</version>
+                <configuration>
+                    <fork>true</fork>
+                    <compilerArgs>
+                        <arg>-XDcompilePolicy=simple</arg>
+                        <arg>--should-stop=ifError=FLOW</arg>
+                        <arg>-Xplugin:ErrorProne -Xep:NullAway:ERROR -XepOpt:NullAway:AnnotatedPackages=org.juv25d</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
+                        <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
+                        <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
+                        <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
+                    </compilerArgs>
+                    <annotationProcessorPaths>
+                        <path>
+                            <groupId>com.google.errorprone</groupId>
+                            <artifactId>error_prone_core</artifactId>
+                            <version>${errorprone.version}</version>
+                        </path>
+                        <path>
+                            <groupId>com.uber.nullaway</groupId>
+                            <artifactId>nullaway</artifactId>
+                            <version>${nullaway.version}</version>
+                        </path>
+                    </annotationProcessorPaths>
+                </configuration>
             </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
diff --git a/src/main/java/org/juv25d/ConnectionHandler.java b/src/main/java/org/juv25d/ConnectionHandler.java
@@ -32,7 +32,8 @@ public void run() {
             var out = socket.getOutputStream();
 
             HttpRequest parsed = httpParser.parse(in);
-            String remoteIp = socket.getInetAddress().getHostAddress();
+            java.net.InetAddress inetAddress = socket.getInetAddress();
+            String remoteIp = (inetAddress != null) ? inetAddress.getHostAddress() : "UNKNOWN";
 
             HttpRequest request = new HttpRequest(
                 parsed.method(),
diff --git a/src/main/java/org/juv25d/config/FilterConfig.java b/src/main/java/org/juv25d/config/FilterConfig.java
@@ -1,6 +1,7 @@
 package org.juv25d.config;
 
 import java.util.Map;
+import org.jspecify.annotations.Nullable;
 
 public class FilterConfig {
 
@@ -10,7 +11,7 @@ public FilterConfig(Map<String, String> params) {
         this.params = params;
     }
 
-    public String get(String key) {
+    @Nullable public String get(String key) {
         return params.get(key);
     }
 }
diff --git a/src/main/java/org/juv25d/config/package-info.java b/src/main/java/org/juv25d/config/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.config;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/di/package-info.java b/src/main/java/org/juv25d/di/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.di;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/filter/CorsFilter.java b/src/main/java/org/juv25d/filter/CorsFilter.java
@@ -63,7 +63,8 @@ public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throw
         chain.doFilter(req, res);
     }
 
-    private String header(Map<String, String> headers, String key) {
+    @org.jspecify.annotations.Nullable private String header(Map<String, String> headers, @org.jspecify.annotations.Nullable String key) {
+        if (key == null) return null;
         for (var entry : headers.entrySet()) {
             if (entry.getKey() != null && entry.getKey().equalsIgnoreCase(key)) {
                 return entry.getValue();
diff --git a/src/main/java/org/juv25d/filter/FilterRegistration.java b/src/main/java/org/juv25d/filter/FilterRegistration.java
@@ -1,10 +1,12 @@
 package org.juv25d.filter;
 
-public record FilterRegistration(Filter filter, int order, String pattern)
+import org.jspecify.annotations.Nullable;
+
+public record FilterRegistration(Filter filter, int order, @Nullable String pattern)
     implements Comparable<FilterRegistration> {
 
     @Override
     public int compareTo(FilterRegistration o) {
         return Integer.compare(this.order, o.order);
     }
-}
+}
diff --git a/src/main/java/org/juv25d/filter/IpFilter.java b/src/main/java/org/juv25d/filter/IpFilter.java
@@ -5,6 +5,8 @@
 import org.juv25d.http.HttpRequest;
 import org.juv25d.http.HttpResponse;
 
+import org.jspecify.annotations.Nullable;
+
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.HashSet;
@@ -17,7 +19,7 @@ public class IpFilter implements Filter {
 
     private final boolean allowByDefault;
 
-    public IpFilter(Set<String> whitelist, Set<String> blacklist,  boolean allowByDefault) {
+    public IpFilter(@Nullable Set<String> whitelist, @Nullable Set<String> blacklist, boolean allowByDefault) {
         if (whitelist != null) {
             this.whitelist.addAll(whitelist);
         }
diff --git a/src/main/java/org/juv25d/filter/annotation/package-info.java b/src/main/java/org/juv25d/filter/annotation/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.filter.annotation;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/filter/package-info.java b/src/main/java/org/juv25d/filter/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.filter;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/handler/MimeTypeResolver.java b/src/main/java/org/juv25d/handler/MimeTypeResolver.java
@@ -2,6 +2,7 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Resolves MIME types based on file extensions.
@@ -52,7 +53,7 @@ private MimeTypeResolver() {
      * @return the MIME type (e.g., "text/html", "text/css")
      *         or "application/octet-stream" if unknown
      */
-    public static String getMimeType(String filename) {
+    public static String getMimeType(@Nullable String filename) {
         if (filename == null || filename.isEmpty()) {
             return "application/octet-stream";
         }
diff --git a/src/main/java/org/juv25d/handler/StaticFileHandler.java b/src/main/java/org/juv25d/handler/StaticFileHandler.java
@@ -4,6 +4,8 @@
 import org.juv25d.http.HttpResponse;
 import org.juv25d.logging.ServerLogging;
 
+import org.jspecify.annotations.Nullable;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
@@ -164,7 +166,7 @@ private static byte[] loadResource(String resourcePath) throws IOException {
         }
     }
 
-    private static String getHeaderIgnoreCase(Map<String, String> headers, String name) {
+    @Nullable private static String getHeaderIgnoreCase(Map<String, String> headers, String name) {
        if (headers == null || headers.isEmpty() || name == null) {
            return null;
        }
@@ -197,13 +199,13 @@ private static String toHex(byte[] bytes) {
         return new String(out);
     }
 
-    private static String opaqueTag(String etag) {
+    @Nullable private static String opaqueTag(@Nullable String etag) {
         if (etag == null) return null;
         String e = etag.trim();
         return e.startsWith("W/") ? e.substring(2) : e;
     }
 
-    private static boolean etagMatches(String ifNoneMatchHeader, String currentEtag) {
+    private static boolean etagMatches(@Nullable String ifNoneMatchHeader, String currentEtag) {
         if (ifNoneMatchHeader == null || ifNoneMatchHeader.isBlank()) {
             return false;
         }
@@ -214,7 +216,8 @@ private static boolean etagMatches(String ifNoneMatchHeader, String currentEtag)
 
         String[] parts = value.split(",");
         for (String part : parts) {
-            if (part != null && opaqueTag(part) != null && opaqueTag(part).equals(opaqueTag(currentEtag))) {
+            String tag = opaqueTag(part);
+            if (part != null && tag != null && tag.equals(opaqueTag(currentEtag))) {
                 return true;
             }
         }
diff --git a/src/main/java/org/juv25d/handler/package-info.java b/src/main/java/org/juv25d/handler/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.handler;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/http/HttpParser.java b/src/main/java/org/juv25d/http/HttpParser.java
@@ -4,6 +4,7 @@
 import java.io.InputStream;
 import java.util.Map;
 import java.util.TreeMap;
+import org.jspecify.annotations.Nullable;
 
 public class HttpParser {
 
@@ -63,7 +64,7 @@ public HttpRequest parse(InputStream in) throws IOException {
         return new HttpRequest(method, path, query, version, headers, body, "UNKNOWN");
     }
 
-    private String readLine(InputStream in) throws IOException {
+    @Nullable private String readLine(InputStream in) throws IOException {
         StringBuilder sb = new StringBuilder();
         int b;
         while ((b = in.read()) != -1) {
diff --git a/src/main/java/org/juv25d/http/HttpRequest.java b/src/main/java/org/juv25d/http/HttpRequest.java
@@ -1,18 +1,19 @@
 package org.juv25d.http;
 
 import java.util.Map;
+import org.jspecify.annotations.Nullable;
 
 public record HttpRequest(
         String method,
         String path,
-        String queryString,
+        @Nullable String queryString,
         String httpVersion,
         Map<String, String> headers,
         byte[] body,
         String remoteIp,
         long creationTimeNanos
 ) {
-    public HttpRequest(String method, String path, String queryString, String httpVersion, Map<String, String> headers, byte[] body, String remoteIp) {
+    public HttpRequest(String method, String path, @Nullable String queryString, String httpVersion, Map<String, String> headers, byte[] body, String remoteIp) {
         this(method, path, queryString, httpVersion, headers, body, remoteIp, System.nanoTime());
     }
 }
diff --git a/src/main/java/org/juv25d/http/HttpResponse.java b/src/main/java/org/juv25d/http/HttpResponse.java
@@ -49,7 +49,7 @@ public Map<String, String> headers() {
         return headers;
     }
 
-    public String getHeader(String name) {
+    @org.jspecify.annotations.Nullable public String getHeader(@org.jspecify.annotations.Nullable String name) {
         if (name == null) {
             return null;
         }
diff --git a/src/main/java/org/juv25d/http/package-info.java b/src/main/java/org/juv25d/http/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.http;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/logging/LogContext.java b/src/main/java/org/juv25d/logging/LogContext.java
@@ -1,13 +1,15 @@
 package org.juv25d.logging;
 
+import org.jspecify.annotations.Nullable;
+
 public class LogContext {
     private static final ThreadLocal<String> connectionId = new ThreadLocal<>();
 
     public static void setConnectionId(String id) {
         connectionId.set(id);
     }
 
-    public static String getConnectionId() {
+    @Nullable public static String getConnectionId() {
         return connectionId.get();
     }
 
diff --git a/src/main/java/org/juv25d/logging/ServerLogFormatter.java b/src/main/java/org/juv25d/logging/ServerLogFormatter.java
@@ -18,7 +18,7 @@ public String format(LogRecord record) {
 
         StringBuilder sb = new StringBuilder(
                 String.format("%s %s%s: %s%n",
-                        ZonedDateTime.now(ZoneId.systemDefault()).format(dtf),
+                        ZonedDateTime.ofInstant(record.getInstant(), ZoneId.systemDefault()).format(dtf),
                         record.getLevel(),
                         idPart,
                         formatMessage(record)));
diff --git a/src/main/java/org/juv25d/logging/package-info.java b/src/main/java/org/juv25d/logging/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.logging;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/package-info.java b/src/main/java/org/juv25d/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/plugin/package-info.java b/src/main/java/org/juv25d/plugin/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.plugin;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/proxy/ProxyRoute.java b/src/main/java/org/juv25d/proxy/ProxyRoute.java
@@ -1,5 +1,7 @@
 package org.juv25d.proxy;
 
+import org.jspecify.annotations.Nullable;
+
 /*
  * Maps a baseRoute with its respective proxy upstream target URL.
  * The mappings are initialized in ConfigLoader.java when reading them from
@@ -18,7 +20,7 @@ public ProxyRoute(String baseRoute, String upstreamUrl) {
         this.upstreamUrl = upstreamUrl;
     }
 
-    public String buildUrl(String targetPath, String query) {
+    public String buildUrl(String targetPath, @Nullable String query) {
         String url = upstreamUrl + targetPath;
         if (query != null && !query.isEmpty()) url += "?" + query;
 
diff --git a/src/main/java/org/juv25d/proxy/package-info.java b/src/main/java/org/juv25d/proxy/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.proxy;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/router/package-info.java b/src/main/java/org/juv25d/router/package-info.java
@@ -0,0 +1,4 @@
+@NullMarked
+package org.juv25d.router;
+
+import org.jspecify.annotations.NullMarked;
diff --git a/src/main/java/org/juv25d/util/ConfigLoader.java b/src/main/java/org/juv25d/util/ConfigLoader.java
diff --git a/src/main/java/org/juv25d/util/package-info.java b/src/main/java/org/juv25d/util/package-info.java
diff --git a/src/test/java/org/juv25d/filter/CorsFilterTest.java b/src/test/java/org/juv25d/filter/CorsFilterTest.java
diff --git a/src/test/java/org/juv25d/filter/LoggingFilterTest.java b/src/test/java/org/juv25d/filter/LoggingFilterTest.java
diff --git a/src/test/java/org/juv25d/proxy/ProxyPluginTest.java b/src/test/java/org/juv25d/proxy/ProxyPluginTest.java
diff --git a/src/test/java/org/juv25d/util/ConfigLoaderTest.java b/src/test/java/org/juv25d/util/ConfigLoaderTest.java

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`package org.juv25d.config;`
`2`	`2`
`3`	`3`	`import java.util.Map;`
	`4`	`+import org.jspecify.annotations.Nullable;`
`4`	`5`
`5`	`6`	`public class FilterConfig {`
`6`	`7`
`@@ -10,7 +11,7 @@ public FilterConfig(Map<String, String> params) {`
`10`	`11`	`this.params = params;`
`11`	`12`	`}`
`12`	`13`
`13`		`- public String get(String key) {`
	`14`	`+ @Nullable public String get(String key) {`
`14`	`15`	`return params.get(key);`
`15`	`16`	`}`
`16`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,8 @@ public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throw`
`63`	`63`	`chain.doFilter(req, res);`
`64`	`64`	`}`
`65`	`65`
`66`		`- private String header(Map<String, String> headers, String key) {`
	`66`	`+ @org.jspecify.annotations.Nullable private String header(Map<String, String> headers, @org.jspecify.annotations.Nullable String key) {`
	`67`	`+ if (key == null) return null;`
`67`	`68`	`for (var entry : headers.entrySet()) {`
`68`	`69`	`if (entry.getKey() != null && entry.getKey().equalsIgnoreCase(key)) {`
`69`	`70`	`return entry.getValue();`