Interrupted request leads to NPE using Netty + AccessLog fix jooby-project#2260

jknack · jknack · commit 4a9465bc18bc · 2021-04-20T11:15:19.000-03:00
- Make sure Context.remoteAddress returns a non-null value
- Update access log to get remote address at the beginning of requests (less chance of loosing it)
diff --git a/jooby/src/main/java/io/jooby/AccessLogHandler.java b/jooby/src/main/java/io/jooby/AccessLogHandler.java
@@ -218,9 +218,11 @@ public AccessLogHandler() {
   @Nonnull @Override public Route.Handler apply(@Nonnull Route.Handler next) {
     long timestamp = System.currentTimeMillis();
     return ctx -> {
+      // Take remote address here (less chances of loosing it on interrupted requests).
+      String remoteAddr = ctx.getRemoteAddress();
       ctx.onComplete(context -> {
         StringBuilder sb = new StringBuilder(MESSAGE_SIZE);
-        sb.append(ctx.getRemoteAddress());
+        sb.append(remoteAddr);
         sb.append(SP).append(DASH).append(SP);
         sb.append(userId.apply(ctx));
         sb.append(SP);
diff --git a/jooby/src/main/java/io/jooby/Context.java b/jooby/src/main/java/io/jooby/Context.java
@@ -553,7 +553,8 @@ public interface Context extends Registry {
    * If you run behind a reverse proxy that has been configured to send the X-Forwarded-* header,
    * please consider to set {@link Router#setTrustProxy(boolean)} option.
    *
-   * @return The IP address of the client or last proxy that sent the request.
+   * @return The IP address of the client or last proxy that sent the request or
+   *   <code>empty string</code> for interrupted requests.
    */
   @Nonnull String getRemoteAddress();
 
diff --git a/modules/jooby-jetty/src/main/java/io/jooby/internal/jetty/JettyContext.java b/modules/jooby-jetty/src/main/java/io/jooby/internal/jetty/JettyContext.java
@@ -54,6 +54,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintWriter;
+import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
@@ -67,6 +68,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
 import static org.eclipse.jetty.http.HttpHeader.CONTENT_TYPE;
@@ -262,7 +264,13 @@ public JettyContext(Request request, Router router, int bufferSize, long maxRequ
   }
 
   @Nonnull @Override public String getRemoteAddress() {
-    return remoteAddress == null ? request.getRemoteAddr() : remoteAddress;
+    if (remoteAddress == null) {
+      String remoteAddr = Optional.ofNullable(request.getRemoteAddr())
+          .orElse("")
+          .trim();
+      return remoteAddr;
+    }
+    return remoteAddress;
   }
 
   @Nonnull @Override public Context setRemoteAddress(@Nonnull String remoteAddress) {
diff --git a/modules/jooby-netty/src/main/java/io/jooby/internal/netty/NettyContext.java b/modules/jooby-netty/src/main/java/io/jooby/internal/netty/NettyContext.java
@@ -259,10 +259,13 @@ boolean isHttpGet() {
 
   @Nonnull @Override public String getRemoteAddress() {
     if (this.remoteAddress == null) {
-      InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress();
-      String hostAddress = remoteAddress.getAddress().getHostAddress();
-      int i = hostAddress.lastIndexOf('%');
-      this.remoteAddress = i > 0 ? hostAddress.substring(0, i) : hostAddress;
+      InetSocketAddress inetAddress = (InetSocketAddress) ctx.channel().remoteAddress();
+      if (inetAddress != null) {
+        String hostAddress = inetAddress.getAddress().getHostAddress();
+        int i = hostAddress.lastIndexOf('%');
+        this.remoteAddress = i > 0 ? hostAddress.substring(0, i) : hostAddress;
+      }
+      return "";
     }
     return remoteAddress;
   }
diff --git a/modules/jooby-utow/src/main/java/io/jooby/internal/utow/UtowContext.java b/modules/jooby-utow/src/main/java/io/jooby/internal/utow/UtowContext.java
@@ -16,6 +16,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintWriter;
+import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
@@ -28,6 +29,7 @@
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
 import javax.annotation.Nonnull;
@@ -185,7 +187,14 @@ boolean isHttpGet() {
   }
 
   @Nonnull @Override public String getRemoteAddress() {
-    return remoteAddress == null ? exchange.getSourceAddress().getHostString() : remoteAddress;
+    if (remoteAddress == null) {
+      String remoteAddr = Optional.ofNullable(exchange.getSourceAddress())
+          .map(InetSocketAddress::getHostString)
+          .orElse("")
+          .trim();
+      return remoteAddr;
+    }
+    return remoteAddress;
   }
 
   @Nonnull @Override public Context setRemoteAddress(@Nonnull String remoteAddress) {

Original file line number	Diff line number	Diff line change
`@@ -553,7 +553,8 @@ public interface Context extends Registry {`
`553`	`553`	`* If you run behind a reverse proxy that has been configured to send the X-Forwarded-* header,`
`554`	`554`	`* please consider to set {@link Router#setTrustProxy(boolean)} option.`
`555`	`555`	`*`
`556`		`- * @return The IP address of the client or last proxy that sent the request.`
	`556`	`+ * @return The IP address of the client or last proxy that sent the request or`
	`557`	`+ * <code>empty string</code> for interrupted requests.`
`557`	`558`	`*/`
`558`	`559`	`@Nonnull String getRemoteAddress();`
`559`	`560`