hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-22 23:35:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #16650 from alexrford/rb/routing-improvements

Browse Source 
Ruby: ActionDispatch - support `path => target` route format
This commit is contained in:
Alex Ford
2024-06-18 11:17:05 +01:00
committed by GitHub
parent 33704779ea 1100b75a3c
commit 6c3d90e8a0
4 changed files with 138 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

180
ruby/ql/lib/codeql/ruby/frameworks/actiondispatch/internal/Routing.qll
View File

@@ -86,13 +86,13 @@ module Routing {
* ```
*/
private class TopLevelRouteBlock extends RouteBlock, TTopLevelRouteBlock {
MethodCall call;
MethodCall methodCall;
// Routing blocks create scopes which define the namespace for controllers and paths,
// though they can be overridden in various ways.
// The namespaces can differ, so we track them separately.
Block block;
TopLevelRouteBlock() { this = TTopLevelRouteBlock(_, call, block) }
TopLevelRouteBlock() { this = TTopLevelRouteBlock(_, methodCall, block) }
override string getAPrimaryQlClass() { result = "TopLevelRouteBlock" }
@@ -102,9 +102,9 @@ module Routing {
override RouteBlock getParent() { none() }
override string toString() { result = call.toString() }
override string toString() { result = methodCall.toString() }
override Location getLocation() { result = call.getLocation() }
override Location getLocation() { result = methodCall.getLocation() }
override string getPathComponent() { none() }
@@ -122,9 +122,9 @@ module Routing {
*/
private class ConstraintsRouteBlock extends NestedRouteBlock, TConstraintsRouteBlock {
private Block block;
private MethodCall call;
private MethodCall methodCall;
ConstraintsRouteBlock() { this = TConstraintsRouteBlock(parent, call, block) }
ConstraintsRouteBlock() { this = TConstraintsRouteBlock(parent, methodCall, block) }
override string getAPrimaryQlClass() { result = "ConstraintsRouteBlock" }
@@ -134,9 +134,9 @@ module Routing {
override string getControllerComponent() { result = "" }
override string toString() { result = call.toString() }
override string toString() { result = methodCall.toString() }
override Location getLocation() { result = call.getLocation() }
override Location getLocation() { result = methodCall.getLocation() }
}
/**
@@ -149,31 +149,56 @@ module Routing {
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Scoping.html#method-i-scope
*/
private class ScopeRouteBlock extends NestedRouteBlock, TScopeRouteBlock {
private MethodCall call;
private MethodCall methodCall;
private Block block;
ScopeRouteBlock() { this = TScopeRouteBlock(parent, call, block) }
ScopeRouteBlock() { this = TScopeRouteBlock(parent, methodCall, block) }
override string getAPrimaryQlClass() { result = "ScopeRouteBlock" }
override Stmt getAStmt() { result = block.getAStmt() }
override string toString() { result = call.toString() }
override string toString() { result = methodCall.toString() }
override Location getLocation() { result = call.getLocation() }
override Location getLocation() { result = methodCall.getLocation() }
override string getPathComponent() {
call.getKeywordArgument("path").getConstantValue().isStringlikeValue(result)
methodCall.getKeywordArgument("path").getConstantValue().isStringlikeValue(result)
or
not exists(call.getKeywordArgument("path")) and
call.getArgument(0).getConstantValue().isStringlikeValue(result)
not exists(methodCall.getKeywordArgument("path")) and
methodCall.getArgument(0).getConstantValue().isStringlikeValue(result)
}
override string getControllerComponent() {
call.getKeywordArgument(["controller", "module"]).getConstantValue().isStringlikeValue(result)
methodCall
.getKeywordArgument(["controller", "module"])
.getConstantValue()
.isStringlikeValue(result)
}
}
private Expr getActionFromMethodCall(MethodCall methodCall) {
result =
[
// e.g. `get "/comments", to: "comments#index"
methodCall.getKeywordArgument("to"),
// e.g. `get "/comments" => "comments#index"
methodCall.getArgument(0).(Pair).getValue()
]
}
/**
* Gets a string representation of the controller-action pair that is routed
* to by this method call.
*/
private string getActionStringFromMethodCall(MethodCall methodCall) {
getActionFromMethodCall(methodCall).getConstantValue().isStringlikeValue(result)
or
// TODO: use the redirect call argument to resolve the redirect target
getActionFromMethodCall(methodCall).(MethodCall).getMethodName() = "redirect" and
result = "<redirect>#<redirect>"
}
/**
* A route block defined by a call to `resources`.
* ```rb
@@ -184,10 +209,10 @@ module Routing {
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Resources.html#method-i-resources
*/
private class ResourcesRouteBlock extends NestedRouteBlock, TResourcesRouteBlock {
private MethodCall call;
private MethodCall methodCall;
private Block block;
ResourcesRouteBlock() { this = TResourcesRouteBlock(parent, call, block) }
ResourcesRouteBlock() { this = TResourcesRouteBlock(parent, methodCall, block) }
override string getAPrimaryQlClass() { result = "ResourcesRouteBlock" }
@@ -196,19 +221,21 @@ module Routing {
/**
* Gets the `resources` call that gives rise to this route block.
*/
MethodCall getDefiningMethodCall() { result = call }
MethodCall getDefiningMethodCall() { result = methodCall }
override string getPathComponent() {
exists(string resource | call.getArgument(0).getConstantValue().isStringlikeValue(resource) |
exists(string resource |
methodCall.getArgument(0).getConstantValue().isStringlikeValue(resource)
|
result = resource + "/:" + singularize(resource) + "_id"
)
}
override string getControllerComponent() { result = "" }
override string toString() { result = call.toString() }
override string toString() { result = methodCall.toString() }
override Location getLocation() { result = call.getLocation() }
override Location getLocation() { result = methodCall.getLocation() }
}
/**
@@ -250,10 +277,10 @@ module Routing {
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Scoping.html#method-i-namespace
*/
private class NamespaceRouteBlock extends NestedRouteBlock, TNamespaceRouteBlock {
private MethodCall call;
private MethodCall methodCall;
private Block block;
NamespaceRouteBlock() { this = TNamespaceRouteBlock(parent, call, block) }
NamespaceRouteBlock() { this = TNamespaceRouteBlock(parent, methodCall, block) }
override Stmt getAStmt() { result = block.getAStmt() }
@@ -262,12 +289,12 @@ module Routing {
override string getControllerComponent() { result = this.getNamespace() }
private string getNamespace() {
call.getArgument(0).getConstantValue().isStringlikeValue(result)
methodCall.getArgument(0).getConstantValue().isStringlikeValue(result)
}
override string toString() { result = call.toString() }
override string toString() { result = methodCall.toString() }
override Location getLocation() { result = call.getLocation() }
override Location getLocation() { result = methodCall.getLocation() }
}
/**
@@ -340,20 +367,20 @@ module Routing {
*/
string getAPrimaryQlClass() { result = "RouteImpl" }
MethodCall method;
MethodCall methodCall;
/** Gets a string representation of this route. */
string toString() { result = method.toString() }
string toString() { result = methodCall.toString() }
/**
* Gets the location of the method call that defines this route.
*/
Location getLocation() { result = method.getLocation() }
Location getLocation() { result = methodCall.getLocation() }
/**
* Gets the method call that defines this route.
*/
MethodCall getDefiningMethodCall() { result = method }
MethodCall getDefiningMethodCall() { result = methodCall }
/**
* Get the last component of the path. For example, in
@@ -473,20 +500,20 @@ module Routing {
private class ExplicitRoute extends RouteImpl, TExplicitRoute {
RouteBlock parentBlock;
ExplicitRoute() { this = TExplicitRoute(parentBlock, method) }
ExplicitRoute() { this = TExplicitRoute(parentBlock, methodCall) }
override string getAPrimaryQlClass() { result = "ExplicitRoute" }
override RouteBlock getParentBlock() { result = parentBlock }
override string getLastPathComponent() {
method.getArgument(0).getConstantValue().isStringlikeValue(result)
methodCall.getArgument(0).getConstantValue().isStringlikeValue(result)
}
override string getLastControllerComponent() {
method.getKeywordArgument("controller").getConstantValue().isStringlikeValue(result)
methodCall.getKeywordArgument("controller").getConstantValue().isStringlikeValue(result)
or
not exists(method.getKeywordArgument("controller")) and
not exists(methodCall.getKeywordArgument("controller")) and
(
result = extractController(this.getActionString())
or
@@ -507,18 +534,13 @@ module Routing {
)
}
private string getActionString() {
method.getKeywordArgument("to").getConstantValue().isStringlikeValue(result)
or
method.getKeywordArgument("to").(MethodCall).getMethodName() = "redirect" and
result = "<redirect>#<redirect>"
}
private string getActionString() { result = getActionStringFromMethodCall(methodCall) }
override string getAction() {
// get "/photos", action: "index"
method.getKeywordArgument("action").getConstantValue().isStringlikeValue(result)
methodCall.getKeywordArgument("action").getConstantValue().isStringlikeValue(result)
or
not exists(method.getKeywordArgument("action")) and
not exists(methodCall.getKeywordArgument("action")) and
(
// get "/photos", to: "photos#index"
// get "/photos", to: redirect("some_url")
@@ -531,11 +553,11 @@ module Routing {
or
// get :some_action
not exists(this.getActionString()) and
method.getArgument(0).getConstantValue().isStringlikeValue(result)
methodCall.getArgument(0).getConstantValue().isStringlikeValue(result)
)
}
override string getHttpMethod() { result = method.getMethodName().toString() }
override string getHttpMethod() { result = methodCall.getMethodName().toString() }
}
/**
@@ -577,8 +599,8 @@ module Routing {
ResourcesRoute() {
exists(string resource |
this = TResourcesRoute(parent, method, action) and
method.getArgument(0).getConstantValue().isStringlikeValue(resource) and
this = TResourcesRoute(parent, methodCall, action) and
methodCall.getArgument(0).getConstantValue().isStringlikeValue(resource) and
isDefaultResourceRoute(resource, httpMethod, pathComponent, action)
)
}
@@ -590,7 +612,7 @@ module Routing {
override string getLastPathComponent() { result = pathComponent }
override string getLastControllerComponent() {
method.getArgument(0).getConstantValue().isStringlikeValue(result)
methodCall.getArgument(0).getConstantValue().isStringlikeValue(result)
}
override string getAction() { result = action }
@@ -615,8 +637,8 @@ module Routing {
SingularResourceRoute() {
exists(string resource |
this = TResourceRoute(parent, method, action) and
method.getArgument(0).getConstantValue().isStringlikeValue(resource) and
this = TResourceRoute(parent, methodCall, action) and
methodCall.getArgument(0).getConstantValue().isStringlikeValue(resource) and
isDefaultSingularResourceRoute(resource, httpMethod, pathComponent, action)
)
}
@@ -628,7 +650,7 @@ module Routing {
override string getLastPathComponent() { result = pathComponent }
override string getLastControllerComponent() {
method.getArgument(0).getConstantValue().isStringlikeValue(result)
methodCall.getArgument(0).getConstantValue().isStringlikeValue(result)
}
override string getAction() { result = action }
@@ -652,24 +674,23 @@ module Routing {
private class MatchRoute extends RouteImpl, TMatchRoute {
private RouteBlock parent;
MatchRoute() { this = TMatchRoute(parent, method) }
MatchRoute() { this = TMatchRoute(parent, methodCall) }
override string getAPrimaryQlClass() { result = "MatchRoute" }
override RouteBlock getParentBlock() { result = parent }
override string getLastPathComponent() {
[method.getArgument(0), method.getArgument(0).(Pair).getKey()]
[methodCall.getArgument(0), methodCall.getArgument(0).(Pair).getKey()]
.getConstantValue()
.isStringlikeValue(result)
}
override string getLastControllerComponent() {
result = extractController(getActionStringFromMethodCall(methodCall)) or
methodCall.getKeywordArgument("controller").getConstantValue().isStringlikeValue(result) or
result =
extractController(method.getKeywordArgument("to").getConstantValue().getStringlikeValue()) or
method.getKeywordArgument("controller").getConstantValue().isStringlikeValue(result) or
result =
extractController(method
extractController(methodCall
.getArgument(0)
.(Pair)
.getValue()
@@ -679,7 +700,7 @@ module Routing {
override string getHttpMethod() {
exists(string via |
method.getKeywordArgument("via").getConstantValue().isStringlikeValue(via)
methodCall.getKeywordArgument("via").getConstantValue().isStringlikeValue(via)
|
via = "all" and result = anyHttpMethod()
or
@@ -687,7 +708,7 @@ module Routing {
)
or
result =
method
methodCall
.getKeywordArgument("via")
.(ArrayLiteral)
.getElement(_)
@@ -696,11 +717,10 @@ module Routing {
}
override string getAction() {
result = extractAction(getActionStringFromMethodCall(methodCall)) or
methodCall.getKeywordArgument("action").getConstantValue().isStringlikeValue(result) or
result =
extractAction(method.getKeywordArgument("to").getConstantValue().getStringlikeValue()) or
method.getKeywordArgument("action").getConstantValue().isStringlikeValue(result) or
result =
extractAction(method
extractAction(methodCall
.getArgument(0)
.(Pair)
.getValue()
@@ -821,58 +841,58 @@ module Routing {
}
/**
* Holds if the (resource, method, path, action) combination would be generated by a call to `resources :<resource>`.
* Holds if the (resource, httpMethod, path, action) combination would be generated by a call to `resources :<resource>`.
*/
bindingset[resource]
private predicate isDefaultResourceRoute(
string resource, string method, string path, string action
string resource, string httpMethod, string path, string action
) {
action = "create" and
(method = "post" and path = "/" + resource)
(httpMethod = "post" and path = "/" + resource)
or
action = "index" and
(method = "get" and path = "/" + resource)
(httpMethod = "get" and path = "/" + resource)
or
action = "new" and
(method = "get" and path = "/" + resource + "/new")
(httpMethod = "get" and path = "/" + resource + "/new")
or
action = "edit" and
(method = "get" and path = "/" + resource + ":id/edit")
(httpMethod = "get" and path = "/" + resource + ":id/edit")
or
action = "show" and
(method = "get" and path = "/" + resource + "/:id")
(httpMethod = "get" and path = "/" + resource + "/:id")
or
action = "update" and
(method in ["put", "patch"] and path = "/" + resource + "/:id")
(httpMethod in ["put", "patch"] and path = "/" + resource + "/:id")
or
action = "destroy" and
(method = "delete" and path = "/" + resource + "/:id")
(httpMethod = "delete" and path = "/" + resource + "/:id")
}
/**
* Holds if the (resource, method, path, action) combination would be generated by a call to `resource :<resource>`.
* Holds if the (resource, httpMethod, path, action) combination would be generated by a call to `resource :<resource>`.
*/
bindingset[resource]
private predicate isDefaultSingularResourceRoute(
string resource, string method, string path, string action
string resource, string httpMethod, string path, string action
) {
action = "create" and
(method = "post" and path = "/" + resource)
(httpMethod = "post" and path = "/" + resource)
or
action = "new" and
(method = "get" and path = "/" + resource + "/new")
(httpMethod = "get" and path = "/" + resource + "/new")
or
action = "edit" and
(method = "get" and path = "/" + resource + "/edit")
(httpMethod = "get" and path = "/" + resource + "/edit")
or
action = "show" and
(method = "get" and path = "/" + resource)
(httpMethod = "get" and path = "/" + resource)
or
action = "update" and
(method in ["put", "patch"] and path = "/" + resource)
(httpMethod in ["put", "patch"] and path = "/" + resource)
or
action = "destroy" and
(method = "delete" and path = "/" + resource)
(httpMethod = "delete" and path = "/" + resource)
}
/**

62
ruby/ql/test/library-tests/frameworks/action_dispatch/ActionDispatch.expected
View File

@@ -1,6 +1,6 @@
actionDispatchRoutes
| app/config/routes.rb:2:3:8:5 | call to resources | get | posts | posts | index |
| app/config/routes.rb:2:3:8:5 | call to resources | get | posts/:id | posts | show |
| app/config/routes.rb:2:3:9:5 | call to resources | get | posts | posts | index |
| app/config/routes.rb:2:3:9:5 | call to resources | get | posts/:id | posts | show |
| app/config/routes.rb:3:5:6:7 | call to resources | delete | posts/:post_id/comments/:id | comments | destroy |
| app/config/routes.rb:3:5:6:7 | call to resources | get | posts/:post_id/comments | comments | index |
| app/config/routes.rb:3:5:6:7 | call to resources | get | posts/:post_id/comments/:id | comments | show |
@@ -12,38 +12,40 @@ actionDispatchRoutes
| app/config/routes.rb:4:7:4:41 | call to resources | post | posts/:post_id/comments/:comment_id/replies | replies | create |
| app/config/routes.rb:5:7:5:28 | call to post | post | posts/:post_id/comments/:comment_id/flag | comments | flag |
| app/config/routes.rb:7:5:7:37 | call to post | post | posts/:post_id/upvote | posts | upvote |
| app/config/routes.rb:11:5:11:54 | call to post | post | destroy_all_posts | posts | destroy_alll |
| app/config/routes.rb:15:5:15:46 | call to get | get | numbers/:number | numbers | show |
| app/config/routes.rb:19:5:19:44 | call to get | get | admin/jobs | background_jobs | index |
| app/config/routes.rb:23:5:23:64 | call to get | get | admin/secrets | secrets | view_secrets |
| app/config/routes.rb:24:5:24:42 | call to delete | delete | admin/:user_id | users | destroy |
| app/config/routes.rb:27:3:27:48 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:28:3:28:50 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:29:3:29:69 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | delete | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | patch | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | post | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | put | photos/:id | photos | show |
| app/config/routes.rb:33:5:33:43 | call to post | post | upgrade | users | start_upgrade |
| app/config/routes.rb:37:5:37:31 | call to get | get | current_billing_cycle | billing/enterprise | current_billing_cycle |
| app/config/routes.rb:40:3:40:40 | call to resource | get | global_config | global_config | show |
| app/config/routes.rb:43:5:45:7 | call to resources | get | foo/bar | foo/bar | index |
| app/config/routes.rb:43:5:45:7 | call to resources | get | foo/bar/:id | foo/bar | show |
| app/config/routes.rb:44:7:44:39 | call to get | get | foo/bar/:bar_id/show_debug | foo/bar | show_debug |
| app/config/routes.rb:49:5:49:95 | call to delete | delete | users/:user/notifications | users/notifications | destroy |
| app/config/routes.rb:50:5:50:94 | call to post | post | users/:user/notifications/:notification_id/mark_as_read | users/notifications | mark_as_read |
| app/config/routes.rb:8:5:8:39 | call to post | post | posts/:post_id | posts | downvote |
| app/config/routes.rb:12:5:12:54 | call to post | post | destroy_all_posts | posts | destroy_alll |
| app/config/routes.rb:16:5:16:46 | call to get | get | numbers/:number | numbers | show |
| app/config/routes.rb:20:5:20:44 | call to get | get | admin/jobs | background_jobs | index |
| app/config/routes.rb:24:5:24:64 | call to get | get | admin/secrets | secrets | view_secrets |
| app/config/routes.rb:25:5:25:42 | call to delete | delete | admin/:user_id | users | destroy |
| app/config/routes.rb:28:3:28:48 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:29:3:29:50 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:69 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:31:3:31:50 | call to match | delete | photos/:id | photos | show |
| app/config/routes.rb:31:3:31:50 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:31:3:31:50 | call to match | patch | photos/:id | photos | show |
| app/config/routes.rb:31:3:31:50 | call to match | post | photos/:id | photos | show |
| app/config/routes.rb:31:3:31:50 | call to match | put | photos/:id | photos | show |
| app/config/routes.rb:34:5:34:43 | call to post | post | upgrade | users | start_upgrade |
| app/config/routes.rb:38:5:38:31 | call to get | get | current_billing_cycle | billing/enterprise | current_billing_cycle |
| app/config/routes.rb:41:3:41:40 | call to resource | get | global_config | global_config | show |
| app/config/routes.rb:44:5:46:7 | call to resources | get | foo/bar | foo/bar | index |
| app/config/routes.rb:44:5:46:7 | call to resources | get | foo/bar/:id | foo/bar | show |
| app/config/routes.rb:45:7:45:39 | call to get | get | foo/bar/:bar_id/show_debug | foo/bar | show_debug |
| app/config/routes.rb:50:5:50:95 | call to delete | delete | users/:user/notifications | users/notifications | destroy |
| app/config/routes.rb:51:5:51:94 | call to post | post | users/:user/notifications/:notification_id/mark_as_read | users/notifications | mark_as_read |
actionDispatchControllerMethods
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:2:3:3:5 | index |
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:5:3:6:5 | show |
| app/config/routes.rb:2:3:9:5 | call to resources | app/controllers/posts_controller.rb:2:3:3:5 | index |
| app/config/routes.rb:2:3:9:5 | call to resources | app/controllers/posts_controller.rb:5:3:6:5 | show |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:2:3:39:5 | index |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:41:3:42:5 | show |
| app/config/routes.rb:7:5:7:37 | call to post | app/controllers/posts_controller.rb:8:3:9:5 | upvote |
| app/config/routes.rb:27:3:27:48 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:28:3:28:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:29:3:29:69 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:30:3:30:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:50:5:50:94 | call to post | app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
| app/config/routes.rb:8:5:8:39 | call to post | app/controllers/posts_controller.rb:11:3:12:5 | downvote |
| app/config/routes.rb:28:3:28:48 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:29:3:29:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:30:3:30:69 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:31:3:31:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:51:5:51:94 | call to post | app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
underscore
| Foo | foo |
| Foo::Bar | foo/bar |

1
ruby/ql/test/library-tests/frameworks/action_dispatch/app/config/routes.rb
View File

@@ -5,6 +5,7 @@ Rails.application.routes.draw do
post "flag", to: :flag
end
post "upvote", to: "posts#upvote"
post "downvote" => "posts#downvote"
end
if Rails.env.test?

7
ruby/ql/test/library-tests/frameworks/action_dispatch/app/controllers/posts_controller.rb
View File

@@ -4,7 +4,10 @@ class PostsController < ApplicationController
def show
end
def upvote
end
end
def downvote
end
end