Allow jax-rs path annotation inheritance

owen-mc · owen-mc · commit 12c6f87b2bb9 · 2024-12-09T16:23:30.000Z
diff --git a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll
@@ -147,6 +147,20 @@ private predicate hasPathAnnotation(Annotatable annotatable) {
   )
 }
 
+/**
+ * Holds if the class inherites the JaxRs `@Path` annotation.
+ */
+private predicate hasOrInheritsPathAnnotation(Class c) {
+  hasPathAnnotation(c)
+  or
+  // Note that by the JAX-RS spec, JAX-RS annotations on classes and interfaces
+  // are not inherited, but some implementations, like Apache CXF, do inherit
+  // them. I think this only applies if there are no JaxRS annotations on the
+  // class itself.
+  hasPathAnnotation(c.getAnAncestor()) and
+  not exists(c.getAnAnnotation().(JaxRSAnnotation))
+}
+
 /**
  * A method which is annotated with one or more JaxRS resource type annotations e.g. `@GET`, `@POST` etc.
  */
@@ -191,7 +205,7 @@ class JaxRsResourceMethod extends Method {
 class JaxRsResourceClass extends Class {
   JaxRsResourceClass() {
     // A root resource class has a @Path annotation on the class.
-    hasPathAnnotation(this)
+    hasOrInheritsPathAnnotation(this)
     or
     // A sub-resource
     exists(JaxRsResourceClass resourceClass, Method method |
@@ -227,7 +241,7 @@ class JaxRsResourceClass extends Class {
   /**
    * Holds if this class is a "root resource" class
    */
-  predicate isRootResource() { hasPathAnnotation(this) }
+  predicate isRootResource() { hasOrInheritsPathAnnotation(this) }
 
   /**
    * Gets a `Constructor` that may be called by a JaxRS container to construct this class reflectively.
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs1.java b/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs1.java
@@ -156,12 +156,12 @@ class NotAResourceClass1Jakarta {
 class NotAResourceClass2Jakarta {
 }
 
-class ExtendsJakartaRs1 extends JakartaRs1 {
+class ExtendsJakartaRs1 extends JakartaRs1 { // $ RootResourceClass
   @Override
   int Get() { // $ ResourceMethod
     return 1;
   }
-  
+
   @Override
   @QueryParam("") // $ InjectionAnnotation
   void Post() {
@@ -189,12 +189,12 @@ void Head() {
 }
 
 @Produces(MediaType.TEXT_XML) // $ ProducesAnnotation=text/xml
-class ExtendsJakartaRs1WithProducesAnnotation extends JakartaRs1 {
+class ExtendsJakartaRs1WithProducesAnnotation extends JakartaRs1 { // $ RootResourceClass
   @Override
   int Get() { // $ ResourceMethod=text/xml
     return 2;
   }
-  
+
   @Override
   @QueryParam("") // $ InjectionAnnotation
   void Post() {
@@ -212,4 +212,4 @@ void Put() { // $ ResourceMethod=text/html
   @Override
   void Options() { // $ ResourceMethod=text/xml
   }
-}
+}
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs3.java b/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs3.java
@@ -24,7 +24,7 @@
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.ext.MessageBodyReader;
 
-class ExtendsJakartaRs3 extends JakartaRs3 {
+class ExtendsJakartaRs3 extends JakartaRs3 { // $ RootResourceClass
   @Override
   public int Get() { // $ ResourceMethod
     return 1;
@@ -57,7 +57,7 @@ public void Head() { // not a resource method because it has a jax-rs annotation
 }
 
 @Produces(MediaType.TEXT_XML) // $ ProducesAnnotation=text/xml
-class ExtendsJakartaRs3WithProducesAnnotation extends JakartaRs3 {
+class ExtendsJakartaRs3WithProducesAnnotation extends JakartaRs3 { // $ RootResourceClass
   @Override
   public int Get() { // $ ResourceMethod=text/xml
     return 2;
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs4.java b/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs4.java
@@ -27,17 +27,17 @@
 // This is not a resource class because it doesn't have a @Path annotation.
 // Note that inheritance of class or interface annotations is not supported in
 // JAX-RS.
-public class JakartaRs4 implements JakartaRsInterface {
-  public JakartaRs4() {
+public class JakartaRs4 implements JakartaRsInterface { // $ RootResourceClass
+  public JakartaRs4() { // $ InjectableConstructor
   }
 
   @Override
-  public int Get() { // $ ResourceMethod
-    return 1;
+  public int Get() { // $ ResourceMethod ResourceMethodOnResourceClass
+    return 1; // $ XssSink
   }
 
   @Override
-  public void Post() { // $ ResourceMethod
+  public void Post() { // $ ResourceMethod ResourceMethodOnResourceClass
   }
 
   @Produces("application/json") // $ ProducesAnnotation=application/json
@@ -52,11 +52,11 @@ public void Put() { // not a resource method because it has a jax-rs annotation,
   }
 
   @Override
-  public void Options() { // $ ResourceMethod
+  public void Options() { // $ ResourceMethod ResourceMethodOnResourceClass
   }
 
   @Override
-  public void Head() { // $ ResourceMethod
+  public void Head() { // $ ResourceMethod ResourceMethod ResourceMethodOnResourceClass
   }
 
 
@@ -65,21 +65,21 @@ NonRootResourceClassJakarta subResourceLocator() {
     return null;
   }
 
-  public class NonRootResourceClassJakarta {
+  public class NonRootResourceClassJakarta { // $ NonRootResourceClass
     @GET
-    int Get() { // $ ResourceMethod
-      return 0;
+    int Get() { // $ ResourceMethod ResourceMethodOnResourceClass
+      return 0; // $ XssSink
     }
 
     @Produces("text/html") // $ ProducesAnnotation=text/html
     @POST
-    boolean Post() { // $ ResourceMethod=text/html
-      return false;
+    boolean Post() { // $ ResourceMethod=text/html ResourceMethodOnResourceClass
+      return false; // $ XssSink
     }
 
     @Produces(MediaType.TEXT_PLAIN) // $ ProducesAnnotation=text/plain
     @DELETE
-    double Delete() { // $ ResourceMethod=text/plain
+    double Delete() { // $ ResourceMethod=text/plain ResourceMethodOnResourceClass
       return 0.0;
     }
 
@@ -90,13 +90,13 @@ AnotherNonRootResourceClassJakarta subResourceLocator1() { // $ SubResourceLocat
 
     @GET
     @Path("")
-    NotAResourceClass1Jakarta NotASubResourceLocator1() { // $ ResourceMethod
-      return null; //
+    NotAResourceClass1Jakarta NotASubResourceLocator1() { // $ ResourceMethod ResourceMethodOnResourceClass
+      return null; // $ XssSink
     }
 
     @GET
-    NotAResourceClass2Jakarta NotASubResourceLocator2() { // $ ResourceMethod
-      return null; //
+    NotAResourceClass2Jakarta NotASubResourceLocator2() { // $ ResourceMethod ResourceMethodOnResourceClass
+      return null; // $ XssSink
     }
 
     NotAResourceClass2Jakarta NotASubResourceLocator3() {
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.expected b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.expected
@@ -1,2 +1,6 @@
-failures
 testFailures
+| JakartaRs1.java:192:68:192:89 | // $ RootResourceClass | Missing result: RootResourceClass |
+| JakartaRs3.java:60:68:60:89 | // $ RootResourceClass | Missing result: RootResourceClass |
+| JaxRs1.java:192:60:192:81 | // $ RootResourceClass | Missing result: RootResourceClass |
+| JaxRs3.java:60:60:60:81 | // $ RootResourceClass | Missing result: RootResourceClass |
+failures
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java
@@ -156,12 +156,12 @@ class NotAResourceClass1 {
 class NotAResourceClass2 {
 }
 
-class ExtendsJaxRs1 extends JaxRs1 {
+class ExtendsJaxRs1 extends JaxRs1 { // $ RootResourceClass
   @Override
   int Get() { // $ ResourceMethod
     return 1;
   }
-  
+
   @Override
   @QueryParam("") // $ InjectionAnnotation
   void Post() {
@@ -189,12 +189,12 @@ void Head() {
 }
 
 @Produces(MediaType.TEXT_XML) // $ ProducesAnnotation=text/xml
-class ExtendsJaxRs1WithProducesAnnotation extends JaxRs1 {
+class ExtendsJaxRs1WithProducesAnnotation extends JaxRs1 { // $ RootResourceClass
   @Override
   int Get() { // $ ResourceMethod=text/xml
     return 2;
   }
-  
+
   @Override
   @QueryParam("") // $ InjectionAnnotation
   void Post() {
@@ -212,4 +212,4 @@ void Put() { // $ ResourceMethod=text/html
   @Override
   void Options() { // $ ResourceMethod=text/xml
   }
-}
+}
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs3.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs3.java
@@ -24,7 +24,7 @@
 import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.MessageBodyReader;
 
-class ExtendsJaxRs3 extends JaxRs3 {
+class ExtendsJaxRs3 extends JaxRs3 { // $ RootResourceClass
   @Override
   public int Get() { // $ ResourceMethod
     return 1;
@@ -57,7 +57,7 @@ public void Head() { // not a resource method because it has a jax-rs annotation
 }
 
 @Produces(MediaType.TEXT_XML) // $ ProducesAnnotation=text/xml
-class ExtendsJaxRs3WithProducesAnnotation extends JaxRs3 {
+class ExtendsJaxRs3WithProducesAnnotation extends JaxRs3 { // $ RootResourceClass
   @Override
   public int Get() { // $ ResourceMethod=text/xml
     return 2;
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs4.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs4.java
@@ -27,17 +27,17 @@
 // This is not a resource class because it doesn't have a @Path annotation.
 // Note that inheritance of class or interface annotations is not supported in
 // JAX-RS.
-public class JaxRs4 implements JaxRsInterface {
-  public JaxRs4() {
+public class JaxRs4 implements JaxRsInterface { // $ RootResourceClass
+  public JaxRs4() { // $ InjectableConstructor
   }
 
   @Override
-  public int Get() { // $ ResourceMethod
-    return 1;
+  public int Get() { // $ ResourceMethod ResourceMethodOnResourceClass
+    return 1; // $ XssSink
   }
 
   @Override
-  public void Post() { // $ ResourceMethod
+  public void Post() { // $ ResourceMethod ResourceMethodOnResourceClass
   }
 
   @Produces("application/json") // $ ProducesAnnotation=application/json
@@ -52,11 +52,11 @@ public void Put() { // not a resource method because it has a jax-rs annotation,
   }
 
   @Override
-  public void Options() { // $ ResourceMethod
+  public void Options() { // $ ResourceMethod ResourceMethodOnResourceClass
   }
 
   @Override
-  public void Head() { // $ ResourceMethod
+  public void Head() { // $ ResourceMethod ResourceMethodOnResourceClass
   }
 
 
@@ -65,21 +65,21 @@ NonRootResourceClass subResourceLocator() {
     return null;
   }
 
-  public class NonRootResourceClass {
+  public class NonRootResourceClass { // $ NonRootResourceClass
     @GET
-    int Get() { // $ ResourceMethod
-      return 0;
+    int Get() { // $ ResourceMethod ResourceMethodOnResourceClass
+      return 0; // $ XssSink
     }
 
     @Produces("text/html") // $ ProducesAnnotation=text/html
     @POST
-    boolean Post() { // $ ResourceMethod=text/html
-      return false;
+    boolean Post() { // $ ResourceMethod=text/html ResourceMethodOnResourceClass
+      return false; // $ XssSink
     }
 
     @Produces(MediaType.TEXT_PLAIN) // $ ProducesAnnotation=text/plain
     @DELETE
-    double Delete() { // $ ResourceMethod=text/plain
+    double Delete() { // $ ResourceMethod=text/plain ResourceMethodOnResourceClass
       return 0.0;
     }
 
@@ -90,13 +90,13 @@ AnotherNonRootResourceClass subResourceLocator1() { // $ SubResourceLocator
 
     @GET
     @Path("")
-    NotAResourceClass1 NotASubResourceLocator1() { // $ ResourceMethod
-      return null; //
+    NotAResourceClass1 NotASubResourceLocator1() { // $ ResourceMethod ResourceMethodOnResourceClass
+      return null; // $ XssSink
     }
 
     @GET
-    NotAResourceClass2 NotASubResourceLocator2() { // $ ResourceMethod
-      return null; //
+    NotAResourceClass2 NotASubResourceLocator2() { // $ ResourceMethod ResourceMethodOnResourceClass
+      return null; // $ XssSink
     }
 
     NotAResourceClass2 NotASubResourceLocator3() {
