Merge pull request #16628 from geoand/#16620

Avoid potential NPE in Metrics filter
quarkusio · Apr 19, 2021 · 8d6d08c · 8d6d08c
2 parents 61ef185 + 7cf6652
commit 8d6d08c
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 9 deletions.
diff --git a/...t/src/main/java/io/quarkus/resteasy/reactive/server/deployment/CustomFilterGenerator.java b/...t/src/main/java/io/quarkus/resteasy/reactive/server/deployment/CustomFilterGenerator.java
@@ -381,7 +381,8 @@ private static AssignableResultHandle getResourceInfoHandle(MethodCreator filter
         ResultHandle runtimeResourceHandle = GeneratorUtils.runtimeResourceHandle(filterMethod, rrReqCtxHandle);
         AssignableResultHandle resourceInfo = filterMethod.createVariable(ResteasyReactiveResourceInfo.class);
         BranchResult ifNullBranch = filterMethod.ifNull(runtimeResourceHandle);
-        ifNullBranch.trueBranch().assign(resourceInfo, ifNullBranch.trueBranch().loadNull());
+        ifNullBranch.trueBranch().assign(resourceInfo, ifNullBranch.trueBranch().readStaticField(
+                FieldDescriptor.of(SimpleResourceInfo.NullValues.class, "INSTANCE", SimpleResourceInfo.NullValues.class)));
         ifNullBranch.falseBranch().assign(resourceInfo, ifNullBranch.falseBranch().invokeVirtualMethod(
                 ofMethod(RuntimeResource.class, "getLazyMethod", ResteasyReactiveResourceInfo.class),
                 runtimeResourceHandle));
@@ -392,7 +393,8 @@ private static AssignableResultHandle getSimpleResourceInfoHandle(MethodCreator
         ResultHandle runtimeResourceHandle = GeneratorUtils.runtimeResourceHandle(filterMethod, rrReqCtxHandle);
         AssignableResultHandle resourceInfo = filterMethod.createVariable(SimpleResourceInfo.class);
         BranchResult ifNullBranch = filterMethod.ifNull(runtimeResourceHandle);
-        ifNullBranch.trueBranch().assign(resourceInfo, ifNullBranch.trueBranch().loadNull());
+        ifNullBranch.trueBranch().assign(resourceInfo, ifNullBranch.trueBranch().readStaticField(
+                FieldDescriptor.of(SimpleResourceInfo.NullValues.class, "INSTANCE", SimpleResourceInfo.NullValues.class)));
         ifNullBranch.falseBranch().assign(resourceInfo, ifNullBranch.falseBranch().invokeVirtualMethod(
                 ofMethod(RuntimeResource.class, "getSimplifiedResourceInfo", SimpleResourceInfo.class),
                 runtimeResourceHandle));

diff --git a/...n/java/io/quarkus/resteasy/reactive/server/deployment/ServerExceptionMapperGenerator.java b/...n/java/io/quarkus/resteasy/reactive/server/deployment/ServerExceptionMapperGenerator.java
@@ -25,6 +25,7 @@
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import javax.ws.rs.container.ResourceInfo;
 import javax.ws.rs.core.Response;
 
 import org.jboss.jandex.AnnotationValue;
@@ -45,6 +46,8 @@
 import org.jboss.resteasy.reactive.server.spi.ServerRequestContext;
 
 import io.quarkus.arc.Unremovable;
+import io.quarkus.gizmo.AssignableResultHandle;
+import io.quarkus.gizmo.BranchResult;
 import io.quarkus.gizmo.ClassCreator;
 import io.quarkus.gizmo.ClassOutput;
 import io.quarkus.gizmo.FieldDescriptor;
@@ -478,14 +481,24 @@ private static TargetMethodParamsInfo getTargetMethodParamsInfo(MethodInfo targe
                         ofMethod(RoutingContext.class, "request", HttpServerRequest.class), routingContextHandle);
             } else if (RESOURCE_INFO.equals(paramDotName)) {
                 ResultHandle runtimeResourceHandle = runtimeResourceHandle(mc, contextHandle);
-                targetMethodParamHandles[i] = mc.invokeVirtualMethod(
+                AssignableResultHandle resourceInfo = mc.createVariable(ResourceInfo.class);
+                BranchResult ifNullBranch = mc.ifNull(runtimeResourceHandle);
+                ifNullBranch.trueBranch().assign(resourceInfo, ifNullBranch.trueBranch().readStaticField(FieldDescriptor
+                        .of(SimpleResourceInfo.NullValues.class, "INSTANCE", SimpleResourceInfo.NullValues.class)));
+                ifNullBranch.falseBranch().assign(resourceInfo, ifNullBranch.falseBranch().invokeVirtualMethod(
                         ofMethod(RuntimeResource.class, "getLazyMethod", ResteasyReactiveResourceInfo.class),
-                        runtimeResourceHandle);
+                        runtimeResourceHandle));
+                targetMethodParamHandles[i] = resourceInfo;
             } else if (SIMPLIFIED_RESOURCE_INFO.equals(paramDotName)) {
                 ResultHandle runtimeResourceHandle = runtimeResourceHandle(mc, contextHandle);
-                targetMethodParamHandles[i] = mc.invokeVirtualMethod(
+                AssignableResultHandle resourceInfo = mc.createVariable(SimpleResourceInfo.class);
+                BranchResult ifNullBranch = mc.ifNull(runtimeResourceHandle);
+                ifNullBranch.trueBranch().assign(resourceInfo, ifNullBranch.trueBranch().readStaticField(FieldDescriptor
+                        .of(SimpleResourceInfo.NullValues.class, "INSTANCE", SimpleResourceInfo.NullValues.class)));
+                ifNullBranch.falseBranch().assign(resourceInfo, ifNullBranch.falseBranch().invokeVirtualMethod(
                         ofMethod(RuntimeResource.class, "getSimplifiedResourceInfo", SimpleResourceInfo.class),
-                        runtimeResourceHandle);
+                        runtimeResourceHandle));
+                targetMethodParamHandles[i] = resourceInfo;
             } else if (ROUTING_CONTEXT.equals(paramDotName)) {
                 targetMethodParamHandles[i] = routingContextHandler(mc, contextHandle);
             } else {

diff --git a/...test/java/io/quarkus/resteasy/reactive/server/test/customproviders/UniResponseFilter.java b/...test/java/io/quarkus/resteasy/reactive/server/test/customproviders/UniResponseFilter.java
@@ -12,7 +12,7 @@ public class UniResponseFilter {
     @ServerResponseFilter
     Uni<Void> filter(SimpleResourceInfo simplifiedResourceInfo,
             ContainerResponseContext responseContext) {
-        if (simplifiedResourceInfo != null) {
+        if (simplifiedResourceInfo.getMethodName() != null) {
             return Uni.createFrom().deferred(() -> {
                 responseContext.getHeaders().putSingle("java-method", simplifiedResourceInfo.getMethodName());
                 return Uni.createFrom().nullItem();

diff --git a/...s/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java b/...s/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java
@@ -17,8 +17,11 @@ public class QuarkusRestMetricsFilter {
 
     @ServerResponseFilter
     public void filter(ResourceInfo resourceInfo, Throwable throwable) {
-        final Class<?> resourceClass = resourceInfo.getResourceClass();
-        final Method resourceMethod = resourceInfo.getResourceMethod();
+        Class<?> resourceClass = resourceInfo.getResourceClass();
+        Method resourceMethod = resourceInfo.getResourceMethod();
+        if ((resourceClass == null) || (resourceMethod == null)) {
+            return;
+        }
         maybeCreateMetrics(resourceClass, resourceMethod);
         FilterUtil.finishRequest(System.nanoTime(), resourceInfo.getResourceClass(),
                 resourceInfo.getResourceMethod().getName(),

diff --git a/...e/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/SimpleResourceInfo.java b/...e/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/SimpleResourceInfo.java
@@ -1,5 +1,8 @@
 package org.jboss.resteasy.reactive.server;
 
+import java.lang.reflect.Method;
+import javax.ws.rs.container.ResourceInfo;
+
 /**
  * Type that can be injected into places where ResourceInfo can.
  * The idea is that this can be used when a piece of code does not need access to the entire resource method
@@ -21,4 +24,32 @@ public interface SimpleResourceInfo {
      * Get the parameter types of the resource method that is the target of a request
      */
     Class<?>[] parameterTypes();
+
+    class NullValues implements SimpleResourceInfo, ResourceInfo {
+
+        public static final NullValues INSTANCE = new NullValues();
+
+        private NullValues() {
+        }
+
+        @Override
+        public Method getResourceMethod() {
+            return null;
+        }
+
+        @Override
+        public Class<?> getResourceClass() {
+            return null;
+        }
+
+        @Override
+        public String getMethodName() {
+            return null;
+        }
+
+        @Override
+        public Class<?>[] parameterTypes() {
+            return new Class[0];
+        }
+    }
 }