Skip to content

Commit dead7a8

Browse files
committed
Ruby: Make most of ActionDispatch private
Any classes/predicates not used externally or in tests are now private. Also fix some typos.
1 parent fa28e55 commit dead7a8

File tree

2 files changed

+23
-18
lines changed

2 files changed

+23
-18
lines changed

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ module ActionDispatch {
6464
* the route defined by the call to `get` has the full path `/admin/dashboard`.
6565
* We track these contributions via `getPathComponent` and `getControllerComponent`.
6666
*/
67-
abstract class RouteBlock extends TRouteBlock {
67+
abstract private class RouteBlock extends TRouteBlock {
6868
/**
6969
* Gets the name of a primary CodeQL class to which this route block belongs.
7070
*/
@@ -228,7 +228,7 @@ module ActionDispatch {
228228
* ```
229229
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Resources.html#method-i-resources
230230
*/
231-
class ResourcesRouteBlock extends NestedRouteBlock, TResourcesRouteBlock {
231+
private class ResourcesRouteBlock extends NestedRouteBlock, TResourcesRouteBlock {
232232
private MethodCall call;
233233
private Block block;
234234

@@ -267,7 +267,7 @@ module ActionDispatch {
267267
* We ignore the condition and analyze both branches to obtain as
268268
* much routing information as possible.
269269
*/
270-
class ConditionalRouteBlock extends NestedRouteBlock, TConditionalRouteBlock {
270+
private class ConditionalRouteBlock extends NestedRouteBlock, TConditionalRouteBlock {
271271
private ConditionalExpr e;
272272

273273
ConditionalRouteBlock() { this = TConditionalRouteBlock(parent, e) }
@@ -294,7 +294,7 @@ module ActionDispatch {
294294
* ```
295295
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Scoping.html#method-i-namespace
296296
*/
297-
class NamespaceRouteBlock extends NestedRouteBlock, TNamespaceRouteBlock {
297+
private class NamespaceRouteBlock extends NestedRouteBlock, TNamespaceRouteBlock {
298298
private MethodCall call;
299299
private Block block;
300300

@@ -494,7 +494,7 @@ module ActionDispatch {
494494
* put "/photos/:id", to: "photos#update"
495495
* ```
496496
*/
497-
class ExplicitRoute extends Route, TExplicitRoute {
497+
private class ExplicitRoute extends Route, TExplicitRoute {
498498
RouteBlock parentBlock;
499499

500500
ExplicitRoute() { this = TExplicitRoute(parentBlock, method) }
@@ -593,7 +593,7 @@ module ActionDispatch {
593593
* get "/photos/:photo_id/foo", to: "photos#foo"
594594
* ```
595595
*/
596-
class ResourcesRoute extends Route, TResourcesRoute {
596+
private class ResourcesRoute extends Route, TResourcesRoute {
597597
RouteBlock parent;
598598
string resource;
599599
string action;
@@ -628,7 +628,7 @@ module ActionDispatch {
628628
* resource :account
629629
* ```
630630
*/
631-
class SingularResourceRoute extends Route, TResourceRoute {
631+
private class SingularResourceRoute extends Route, TResourceRoute {
632632
RouteBlock parent;
633633
string resource;
634634
string action;
@@ -666,7 +666,7 @@ module ActionDispatch {
666666
* match 'photos/:id', controller: 'photos', action: 'show', via: :get
667667
* ```
668668
*/
669-
class MatchRoute extends Route, TMatchRoute {
669+
private class MatchRoute extends Route, TMatchRoute {
670670
private RouteBlock parent;
671671

672672
MatchRoute() { this = TMatchRoute(parent, method) }
@@ -704,7 +704,7 @@ module ActionDispatch {
704704
* - `except:` removes the given actions from the set.
705705
*/
706706
bindingset[action]
707-
predicate applyActionFilters(MethodCall m, string action) {
707+
private predicate applyActionFilters(MethodCall m, string action) {
708708
// Respect the `only` keyword argument, which restricts the set of actions.
709709
(
710710
not exists(m.getKeywordArgument("only"))
@@ -727,7 +727,9 @@ module ActionDispatch {
727727
* Holds if the (resource, method, path, action) combination would be generated by a call to `resources :<resource>`.
728728
*/
729729
bindingset[resource]
730-
predicate isDefaultResourceRoute(string resource, string method, string path, string action) {
730+
private predicate isDefaultResourceRoute(
731+
string resource, string method, string path, string action
732+
) {
731733
action = "create" and
732734
(method = "post" and path = "/" + resource)
733735
or
@@ -754,7 +756,7 @@ module ActionDispatch {
754756
* Holds if the (resource, method, path, action) combination would be generated by a call to `resource :<resource>`.
755757
*/
756758
bindingset[resource]
757-
predicate isDefaultSingularResourceRoute(
759+
private predicate isDefaultSingularResourceRoute(
758760
string resource, string method, string path, string action
759761
) {
760762
action = "create" and
@@ -780,17 +782,18 @@ module ActionDispatch {
780782
* Extract the controller from a Rails routing string
781783
* ```
782784
* extractController("posts#show") = "posts"
785+
* ```
783786
*/
784787
bindingset[input]
785-
string extractController(string input) { result = input.regexpCapture("([^#]+)#.+", 1) }
788+
private string extractController(string input) { result = input.regexpCapture("([^#]+)#.+", 1) }
786789

787790
/**
788791
* Extract the action from a Rails routing string
789792
* ```
790793
* extractController("posts#show") = "show"
791794
*/
792795
bindingset[input]
793-
string extractAction(string input) { result = input.regexpCapture("[^#]+#(.+)", 1) }
796+
private string extractAction(string input) { result = input.regexpCapture("[^#]+#(.+)", 1) }
794797

795798
/**
796799
* A basic pluralizer for English strings.
@@ -799,7 +802,7 @@ module ActionDispatch {
799802
* TODO: remove?
800803
*/
801804
bindingset[input]
802-
string pluralize(string input) {
805+
private string pluralize(string input) {
803806
exists(string prefix | prefix = input.regexpCapture("(.*)y", 1) | result = prefix + "ies")
804807
or
805808
not input.regexpMatch(".*y") and
@@ -813,7 +816,7 @@ module ActionDispatch {
813816
* not_plural => not_plural
814817
*/
815818
bindingset[input]
816-
string singularize(string input) {
819+
private string singularize(string input) {
817820
exists(string prefix | prefix = input.regexpCapture("(.*)ies", 1) | result = prefix + "y")
818821
or
819822
not input.regexpMatch(".*ies") and
@@ -890,13 +893,15 @@ module ActionDispatch {
890893
* Convert the first character of the string to lowercase.
891894
*/
892895
bindingset[input]
893-
string decapitalize(string input) { result = input.charAt(0).toLowerCase() + input.suffix(1) }
896+
private string decapitalize(string input) {
897+
result = input.charAt(0).toLowerCase() + input.suffix(1)
898+
}
894899

895900
/**
896901
* Strip leading and trailing forward slashes from the string.
897902
*/
898903
bindingset[input]
899-
string stripSlashes(string input) {
904+
private string stripSlashes(string input) {
900905
result = input.regexpReplaceAll("^/+(.+)$", "$1").regexpReplaceAll("^(.*[^/])/+$", "$1")
901906
}
902907
}

ruby/ql/lib/codeql/ruby/security/UrlRedirectCustomizations.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ module UrlRedirect {
6262
// redirection as browsers will not initiate them from clicking a link.
6363
method = this.asExpr().getExpr().getEnclosingMethod() and
6464
(
65-
// If there's a Rails GET route to this handler, we can be certain that it is a candiate.
65+
// If there's a Rails GET route to this handler, we can be certain that it is a candidate.
6666
method.(ActionControllerActionMethod).getARoute().getHTTPMethod() = "get"
6767
or
6868
// Otherwise, we have to rely on a heuristic to filter out invulnerable handlers.

0 commit comments

Comments
 (0)