Skip to content

Commit 47823b5

Browse files
committed
Handle via: :all in Rails routes
ActionDispatch modelling now understands that match "/foo", to: "foo#bar", via: :all is equivalent to match "/foo", to: "foo#bar", via: [:get, :post, :put, :patch, :delete]
1 parent 8bdc05d commit 47823b5

File tree

3 files changed

+36
-16
lines changed

3 files changed

+36
-16
lines changed

ruby/ql/lib/codeql/ruby/frameworks/ActionDispatch.qll

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ module ActionDispatch {
325325
* See `ExplicitRoute`
326326
*/
327327
TExplicitRoute(RouteBlock b, MethodCall m) {
328-
b.getAStmt() = m and m.getMethodName() in ["get", "post", "put", "patch", "delete"]
328+
b.getAStmt() = m and m.getMethodName() = anyHttpMethod()
329329
} or
330330
/**
331331
* See `ResourcesRoute`
@@ -518,7 +518,7 @@ module ActionDispatch {
518518
(
519519
result = extractController(this.getActionString())
520520
or
521-
// If not controller is specified, and we're in a `resources` route block, use the controller of that route.
521+
// If controller is not specified, and we're in a `resources` route block, use the controller of that route.
522522
// For example, in
523523
//
524524
// resources :posts do
@@ -671,6 +671,7 @@ module ActionDispatch {
671671
* ```ruby
672672
* match 'photos/:id' => 'photos#show', via: :get
673673
* match 'photos/:id', to: 'photos#show', via: :get
674+
* match 'photos/:id', to 'photos#show', via: [:get, :post]
674675
* match 'photos/:id', controller: 'photos', action: 'show', via: :get
675676
* ```
676677
*/
@@ -702,7 +703,14 @@ module ActionDispatch {
702703
}
703704

704705
override string getHTTPMethod() {
705-
result = method.getKeywordArgument("via").getConstantValue().getStringOrSymbol() or
706+
exists(string via |
707+
via = method.getKeywordArgument("via").getConstantValue().getStringOrSymbol()
708+
|
709+
via = "all" and result = anyHttpMethod()
710+
or
711+
via != "all" and result = "via"
712+
)
713+
or
706714
result =
707715
method
708716
.getKeywordArgument("via")
@@ -816,11 +824,16 @@ module ActionDispatch {
816824
/**
817825
* Extract the action from a Rails routing string
818826
* ```
819-
* extractController("posts#show") = "show"
827+
* extractAction("posts#show") = "show"
820828
*/
821829
bindingset[input]
822830
private string extractAction(string input) { result = input.regexpCapture("[^#]+#(.+)", 1) }
823831

832+
/**
833+
* Returns the lowercase name of every HTTP method we support.
834+
*/
835+
private string anyHttpMethod() { result = ["get", "post", "put", "patch", "delete"] }
836+
824837
/**
825838
* The inverse of `pluralize`
826839
* photos => photo

ruby/ql/test/library-tests/frameworks/ActionDispatch.expected

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,22 @@ actionDispatchRoutes
1717
| app/config/routes.rb:19:5:19:44 | call to get | get | admin/jobs | background_jobs | index |
1818
| app/config/routes.rb:23:5:23:64 | call to get | get | admin/secrets | secrets | view_secrets |
1919
| app/config/routes.rb:24:5:24:42 | call to delete | delete | admin/:user_id | users | destroy |
20-
| app/config/routes.rb:27:3:27:48 | call to match | get | photos/:id | photos | show |
21-
| app/config/routes.rb:28:3:28:50 | call to match | get | photos/:id | photos | show |
22-
| app/config/routes.rb:29:3:29:69 | call to match | get | photos/:id | photos | show |
23-
| app/config/routes.rb:32:5:32:43 | call to post | post | upgrade | users | start_upgrade |
24-
| app/config/routes.rb:36:5:36:31 | call to get | get | current_billing_cycle | billing/enterprise | current_billing_cycle |
25-
| app/config/routes.rb:39:3:39:40 | call to resource | get | global_config | global_config | show |
26-
| app/config/routes.rb:42:5:44:7 | call to resources | get | foo/bar | foo/bar | index |
27-
| app/config/routes.rb:42:5:44:7 | call to resources | get | foo/bar/:id | foo/bar | show |
28-
| app/config/routes.rb:43:7:43:39 | call to get | get | foo/bar/:bar_id/show_debug | foo/bar | show_debug |
29-
| app/config/routes.rb:48:5:48:95 | call to delete | delete | users/:user/notifications | users/notifications | destroy |
30-
| app/config/routes.rb:49:5:49:94 | call to post | post | users/:user/notifications/:notification_id/mark_as_read | users/notifications | mark_as_read |
20+
| app/config/routes.rb:27:3:27:48 | call to match | via | photos/:id | photos | show |
21+
| app/config/routes.rb:28:3:28:50 | call to match | via | photos/:id | photos | show |
22+
| app/config/routes.rb:29:3:29:69 | call to match | via | photos/:id | photos | show |
23+
| app/config/routes.rb:30:3:30:50 | call to match | delete | photos/:id | photos | show |
24+
| app/config/routes.rb:30:3:30:50 | call to match | get | photos/:id | photos | show |
25+
| app/config/routes.rb:30:3:30:50 | call to match | patch | photos/:id | photos | show |
26+
| app/config/routes.rb:30:3:30:50 | call to match | post | photos/:id | photos | show |
27+
| app/config/routes.rb:30:3:30:50 | call to match | put | photos/:id | photos | show |
28+
| app/config/routes.rb:33:5:33:43 | call to post | post | upgrade | users | start_upgrade |
29+
| app/config/routes.rb:37:5:37:31 | call to get | get | current_billing_cycle | billing/enterprise | current_billing_cycle |
30+
| app/config/routes.rb:40:3:40:40 | call to resource | get | global_config | global_config | show |
31+
| app/config/routes.rb:43:5:45:7 | call to resources | get | foo/bar | foo/bar | index |
32+
| app/config/routes.rb:43:5:45:7 | call to resources | get | foo/bar/:id | foo/bar | show |
33+
| app/config/routes.rb:44:7:44:39 | call to get | get | foo/bar/:bar_id/show_debug | foo/bar | show_debug |
34+
| app/config/routes.rb:49:5:49:95 | call to delete | delete | users/:user/notifications | users/notifications | destroy |
35+
| 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 |
3136
actionDispatchControllerMethods
3237
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:2:3:3:5 | index |
3338
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:5:3:6:5 | show |
@@ -37,7 +42,8 @@ actionDispatchControllerMethods
3742
| app/config/routes.rb:27:3:27:48 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
3843
| app/config/routes.rb:28:3:28:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
3944
| app/config/routes.rb:29:3:29:69 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
40-
| app/config/routes.rb:49:5:49:94 | call to post | app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
45+
| app/config/routes.rb:30:3:30:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
46+
| app/config/routes.rb:50:5:50:94 | call to post | app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
4147
underscore
4248
| Foo | foo |
4349
| Foo::Bar | foo/bar |

ruby/ql/test/library-tests/frameworks/app/config/routes.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
match "photos/:id" => "photos#show", via: :get
2828
match "photos/:id", to: "photos#show", via: :get
2929
match "photos/:id", controller: "photos", action: "show", via: :get
30+
match "photos/:id", to: "photos#show", via: :all
3031

3132
scope controller: "users" do
3233
post "upgrade", action: "start_upgrade"

0 commit comments

Comments
 (0)