Ruby: rack - replace RackApplication with just the rack RequestHandler

Author: alexrford

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll

@@ -1010,23 +1010,6 @@ class ModuleNode instanceof Module {
    */
   MethodNode getAnInstanceMethod() { result = this.getInstanceMethod(_) }
 
-  /**
-   * Gets the singleton method named `name` available in this module, including methods inherited from ancestors.
-   *
-   * Overridden methods are not included.
-   */
-  MethodNode getSingletonMethod(string name) {
-    result.asCallableAstNode() = super.getAnOwnSingletonMethod() and
-    result.getMethodName() = name
-  }
-
-  /**
-   * Gets a singleton method available in this module, including methods inherited from ancestors.
-   *
-   * Overridden methods are not included.
-   */
-  MethodNode getASingletonMethod() { result = this.getSingletonMethod(_) }
-
   /**
    * Gets the enclosing module, as it appears in the qualified name of this module.
    *

ruby/ql/lib/codeql/ruby/frameworks/actiondispatch/internal/Request.qll

@@ -128,7 +128,7 @@ module Request {
     private import codeql.ruby.frameworks.Rack
 
     private class RackEnv extends Env {
-      RackEnv() { this = any(Rack::App::RackApplication app).getEnv().getALocalUse() }
+      RackEnv() { this = any(Rack::App::RequestHandler handler).getEnv().getALocalUse() }
     }
 
     /**

ruby/ql/lib/codeql/ruby/frameworks/rack/internal/App.qll

@@ -12,8 +12,8 @@ private import Response::Private as RP
  * A callable node that takes a single argument and, if it has a method name,
  * is called "call".
  */
-private class PotentialCallNode extends DataFlow::CallableNode {
-  PotentialCallNode() {
+private class PotentialRequestHandler extends DataFlow::CallableNode {
+  PotentialRequestHandler() {
     this.getNumberOfParameters() = 1 and
     (
       this.(DataFlow::MethodNode).getMethodName() = "call"
@@ -32,26 +32,14 @@ private class PotentialCallNode extends DataFlow::CallableNode {
   }
 }
 
-/**
- * A callable node that looks like it implements the rack specification.
- */
-private class CallNode extends PotentialCallNode {
-  private RP::PotentialResponseNode resp;
-
-  CallNode() { resp = trackRackResponse(this) }
-
-  /** Gets a response returned from a request to this application. */
-  RP::PotentialResponseNode getAResponse() { result = resp }
-}
-
-private DataFlow::LocalSourceNode trackRackResponse(TypeBackTracker t, PotentialCallNode call) {
+private DataFlow::LocalSourceNode trackRackResponse(TypeBackTracker t, PotentialRequestHandler call) {
   t.start() and
   result = call.getAReturnNode().getALocalSource()
   or
   exists(TypeBackTracker t2 | result = trackRackResponse(t2, call).backtrack(t2, t))
 }
 
-private RP::PotentialResponseNode trackRackResponse(PotentialCallNode call) {
+private RP::PotentialResponseNode trackRackResponse(PotentialRequestHandler call) {
   result = trackRackResponse(TypeBackTracker::end(), call)
 }
 
@@ -66,7 +54,7 @@ module App {
    * (traditionally called `env`) and returns a rack-compatible response.
    */
   deprecated class AppCandidate extends DataFlow::ClassNode {
-    private CallNode call;
+    private RequestHandler call;
     private RP::PotentialResponseNode resp;
 
     AppCandidate() {
@@ -84,64 +72,18 @@ module App {
     RP::PotentialResponseNode getResponse() { result = resp }
   }
 
-  private newtype TApp =
-    TClassApp(DataFlow::ClassNode cn, CallNode call) or
-    TAnonymousApp(CallNode call)
-
   /**
-   * A rack application. This is either some object that responds to `call`
-   * taking a single argument and returns a rack response, or a lambda or
-   * proc that takes a single `env` argument and returns a rack response.
+   * A callable node that looks like it implements the rack specification.
    */
-  abstract class RackApplication extends TApp {
-    /** Gets a textual representation of this element. */
-    string toString() { result = "Rack application" }
-
-    /** Gets the `DataFlow::CallableNode` that will handle requests to this app. */
-    abstract CallNode getCall();
-
-    /** Gets a response returned from a request to this app. */
-    RP::PotentialResponseNode getAResponse() { result = this.getCall().getAResponse() }
-
-    /** Gets the `env` parameter passed to this app when it handles a request. */
-    DataFlow::ParameterNode getEnv() { result = this.getCall().getParameter(0) }
-  }
-
-  /**
-   * A rack application using a `DataFlow::ClassNode`. The class has either
-   * an instance method or a singleton method named "call" which takes a
-   * single `env` argument and returns a rack response.
-   */
-  private class ClassRackApplication extends TApp, RackApplication {
-    private DataFlow::ClassNode cn;
-    private CallNode call;
-
-    ClassRackApplication() {
-      this = TClassApp(cn, call) and
-      call = [cn.getInstanceMethod("call"), cn.getSingletonMethod("call")]
-    }
-
-    override string toString() { result = "Rack application: " + cn.toString() }
-
-    override CallNode getCall() { result = call }
-  }
-
-  /**
-   * A rack application that is either a lambda or a proc, which takes a
-   * single `env` argument and returns a rack response.
-   */
-  private class AnonymousRackApplication extends TApp, RackApplication {
-    private CallNode call;
+  class RequestHandler extends PotentialRequestHandler {
+    private RP::PotentialResponseNode resp;
 
-    AnonymousRackApplication() {
-      this = TAnonymousApp(call) and
-      not exists(DataFlow::ClassNode cn |
-        call = [cn.getInstanceMethod(_), cn.getSingletonMethod(_)]
-      )
-    }
+    RequestHandler() { resp = trackRackResponse(this) }
 
-    override string toString() { result = "Rack application: " + call.toString() }
+    /** Gets the `env` parameter passed to this request handler. */
+    DataFlow::ParameterNode getEnv() { result = this.getParameter(0) }
 
-    override CallNode getCall() { result = call }
+    /** Gets a response returned from this request handler. */
+    RP::PotentialResponseNode getAResponse() { result = resp }
   }
 }

ruby/ql/lib/codeql/ruby/frameworks/rack/internal/Response.qll

@@ -91,7 +91,7 @@ module Public {
   /** A `DataFlow::Node` returned from a rack request. */
   class ResponseNode extends Http::Server::HttpResponse::Range instanceof Private::PotentialResponseNode
   {
-    ResponseNode() { this = any(A::App::RackApplication app).getAResponse() }
+    ResponseNode() { this = any(A::App::RequestHandler handler).getAResponse() }
 
     override DataFlow::Node getBody() { result = this.(Private::PotentialResponseNode).getBody() }
 

ruby/ql/test/library-tests/frameworks/rack/Rack.expected

@@ -1,15 +1,15 @@
-rackApps
-| Rack application: -> { ... } | rack_apps.rb:21:17:21:19 | env | rack_apps.rb:21:24:21:48 | call to [] |
-| Rack application: Baz | rack.rb:60:12:60:14 | env | rack.rb:66:7:66:22 | call to [] |
-| Rack application: Baz | rack.rb:60:12:60:14 | env | rack.rb:73:5:73:21 | call to [] |
-| Rack application: ClassApp | rack_apps.rb:16:17:16:19 | env | rack_apps.rb:17:5:17:28 | call to [] |
-| Rack application: HelloWorld | rack.rb:2:12:2:14 | env | rack.rb:8:5:8:38 | call to [] |
-| Rack application: InstanceApp | rack_apps.rb:6:12:6:14 | env | rack_apps.rb:10:12:10:34 | call to [] |
-| Rack application: Logger | rack.rb:30:12:30:14 | env | rack.rb:35:5:35:26 | call to [] |
-| Rack application: Proxy | rack.rb:17:12:17:18 | the_env | rack.rb:20:5:20:27 | call to [] |
-| Rack application: Qux | rack.rb:79:17:79:19 | env | rack.rb:93:5:93:78 | call to finish |
-| Rack application: Redirector | rack.rb:40:12:40:14 | env | rack.rb:43:5:43:45 | call to [] |
-| Rack application: { ... } | rack_apps.rb:23:24:23:26 | env | rack_apps.rb:23:29:23:51 | call to [] |
+rackRequestHandlers
+| rack.rb:2:3:9:5 | call | rack.rb:2:12:2:14 | env | rack.rb:8:5:8:38 | call to [] |
+| rack.rb:17:3:21:5 | call | rack.rb:17:12:17:18 | the_env | rack.rb:20:5:20:27 | call to [] |
+| rack.rb:30:3:36:5 | call | rack.rb:30:12:30:14 | env | rack.rb:35:5:35:26 | call to [] |
+| rack.rb:40:3:44:5 | call | rack.rb:40:12:40:14 | env | rack.rb:43:5:43:45 | call to [] |
+| rack.rb:60:3:62:5 | call | rack.rb:60:12:60:14 | env | rack.rb:66:7:66:22 | call to [] |
+| rack.rb:60:3:62:5 | call | rack.rb:60:12:60:14 | env | rack.rb:73:5:73:21 | call to [] |
+| rack.rb:79:3:81:5 | call | rack.rb:79:17:79:19 | env | rack.rb:93:5:93:78 | call to finish |
+| rack_apps.rb:6:3:12:5 | call | rack_apps.rb:6:12:6:14 | env | rack_apps.rb:10:12:10:34 | call to [] |
+| rack_apps.rb:16:3:18:5 | call | rack_apps.rb:16:17:16:19 | env | rack_apps.rb:17:5:17:28 | call to [] |
+| rack_apps.rb:21:14:21:50 | -> { ... } | rack_apps.rb:21:17:21:19 | env | rack_apps.rb:21:24:21:48 | call to [] |
+| rack_apps.rb:23:21:23:53 | { ... } | rack_apps.rb:23:24:23:26 | env | rack_apps.rb:23:29:23:51 | call to [] |
 rackResponseContentTypes
 | rack.rb:8:5:8:38 | call to [] | rack.rb:7:34:7:45 | "text/plain" |
 | rack.rb:20:5:20:27 | call to [] | rack.rb:19:28:19:38 | "text/html" |

ruby/ql/test/library-tests/frameworks/rack/Rack.ql

@@ -2,10 +2,10 @@ private import codeql.ruby.AST
 private import codeql.ruby.frameworks.Rack
 private import codeql.ruby.DataFlow
 
-query predicate rackApps(
-  Rack::App::RackApplication app, DataFlow::ParameterNode env, Rack::Response::ResponseNode resp
+query predicate rackRequestHandlers(
+  Rack::App::RequestHandler handler, DataFlow::ParameterNode env, Rack::Response::ResponseNode resp
 ) {
-  env = app.getEnv() and resp = app.getAResponse()
+  env = handler.getEnv() and resp = handler.getAResponse()
 }
 
 query predicate rackResponseContentTypes(