Skip to content

Commit 80835a5

Browse files
committed
Ruby: Don't expose abstract class
Make ActionDispatch::Route into a private class ActionDispatch::RouteImpl, defining a new class Route which exposes the necessary public API from RouteImpl. Also rename getHTTPMethod to getHttpMethod.
1 parent a8a7c15 commit 80835a5

File tree

3 files changed

+64
-12
lines changed

3 files changed

+64
-12
lines changed

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

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,64 @@ module ActionDispatch {
284284
* resources :photos
285285
* ```
286286
*/
287-
abstract class Route extends TRoute {
287+
class Route extends TRoute instanceof RouteImpl {
288288
/**
289289
* Gets the name of a primary CodeQL class to which this route belongs.
290290
*/
291291
string getAPrimaryQlClass() { result = "Route" }
292292

293+
/** Gets a string representation of this route. */
294+
string toString() { result = super.toString() }
295+
296+
/**
297+
* Gets the location of the method call that defines this route.
298+
*/
299+
Location getLocation() { result = super.getLocation() }
300+
301+
/**
302+
* Gets the full controller targeted by this route.
303+
*/
304+
string getController() { result = super.getController() }
305+
306+
/**
307+
* Gets the action targeted by this route.
308+
*/
309+
string getAction() { result = super.getAction() }
310+
311+
/**
312+
* Gets the HTTP method of this route.
313+
* The result is one of [get, post, put, patch, delete].
314+
*/
315+
string getHttpMethod() { result = super.getHttpMethod() }
316+
317+
/**
318+
* Gets the full path of the route.
319+
*/
320+
string getPath() { result = super.getPath() }
321+
322+
/**
323+
* Get a URL capture. This is a wildcard URL segment whose value is placed in `params`.
324+
* For example, in
325+
* ```ruby
326+
* get "/foo/:bar/baz", to: "users#index"
327+
* ```
328+
* the capture is `:bar`.
329+
*/
330+
string getACapture() { result = super.getACapture() }
331+
}
332+
333+
/**
334+
* The implementation of `Route`.
335+
* This class is abstract and is thus kept private so we don't expose it to
336+
* users.
337+
* Extend this class to add new instances of routes.
338+
*/
339+
abstract private class RouteImpl extends TRoute {
340+
/**
341+
* Gets the name of a primary CodeQL class to which this route belongs.
342+
*/
343+
string getAPrimaryQlClass() { result = "RouteImpl" }
344+
293345
MethodCall method;
294346

295347
/** Gets a string representation of this route. */
@@ -319,7 +371,7 @@ module ActionDispatch {
319371
* Gets the HTTP method of this route.
320372
* The result is one of [get, post, put, patch, delete].
321373
*/
322-
abstract string getHTTPMethod();
374+
abstract string getHttpMethod();
323375

324376
/**
325377
* Gets the last controller component.
@@ -420,7 +472,7 @@ module ActionDispatch {
420472
* put "/photos/:id", to: "photos#update"
421473
* ```
422474
*/
423-
private class ExplicitRoute extends Route, TExplicitRoute {
475+
private class ExplicitRoute extends RouteImpl, TExplicitRoute {
424476
RouteBlock parentBlock;
425477

426478
ExplicitRoute() { this = TExplicitRoute(parentBlock, method) }
@@ -485,7 +537,7 @@ module ActionDispatch {
485537
)
486538
}
487539

488-
override string getHTTPMethod() { result = method.getMethodName().toString() }
540+
override string getHttpMethod() { result = method.getMethodName().toString() }
489541
}
490542

491543
/**
@@ -519,7 +571,7 @@ module ActionDispatch {
519571
* get "/photos/:photo_id/foo", to: "photos#foo"
520572
* ```
521573
*/
522-
private class ResourcesRoute extends Route, TResourcesRoute {
574+
private class ResourcesRoute extends RouteImpl, TResourcesRoute {
523575
RouteBlock parent;
524576
string resource;
525577
string action;
@@ -544,7 +596,7 @@ module ActionDispatch {
544596

545597
override string getAction() { result = action }
546598

547-
override string getHTTPMethod() { result = httpMethod }
599+
override string getHttpMethod() { result = httpMethod }
548600
}
549601

550602
/**
@@ -556,7 +608,7 @@ module ActionDispatch {
556608
* resource :account
557609
* ```
558610
*/
559-
private class SingularResourceRoute extends Route, TResourceRoute {
611+
private class SingularResourceRoute extends RouteImpl, TResourceRoute {
560612
RouteBlock parent;
561613
string resource;
562614
string action;
@@ -581,7 +633,7 @@ module ActionDispatch {
581633

582634
override string getAction() { result = action }
583635

584-
override string getHTTPMethod() { result = httpMethod }
636+
override string getHttpMethod() { result = httpMethod }
585637
}
586638

587639
/**
@@ -597,7 +649,7 @@ module ActionDispatch {
597649
* match 'photos/:id', controller: 'photos', action: 'show', via: :get
598650
* ```
599651
*/
600-
private class MatchRoute extends Route, TMatchRoute {
652+
private class MatchRoute extends RouteImpl, TMatchRoute {
601653
private RouteBlock parent;
602654

603655
MatchRoute() { this = TMatchRoute(parent, method) }
@@ -625,7 +677,7 @@ module ActionDispatch {
625677
.getStringOrSymbol())
626678
}
627679

628-
override string getHTTPMethod() {
680+
override string getHttpMethod() {
629681
exists(string via |
630682
method.getKeywordArgument("via").getConstantValue().isStringOrSymbol(via)
631683
|

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ module UrlRedirect {
6363
method = this.asExpr().getExpr().getEnclosingMethod() and
6464
(
6565
// If there's a Rails GET route to this handler, we can be certain that it is a candidate.
66-
method.(ActionControllerActionMethod).getARoute().getHTTPMethod() = "get"
66+
method.(ActionControllerActionMethod).getARoute().getHttpMethod() = "get"
6767
or
6868
// Otherwise, we have to rely on a heuristic to filter out invulnerable handlers.
6969
// We exclude any handlers with names containing create/update/destroy, as these are not likely to handle GET requests.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ private import codeql.ruby.frameworks.ActionController
55
query predicate actionDispatchRoutes(
66
ActionDispatch::Route r, string method, string path, string controller, string action
77
) {
8-
r.getHTTPMethod() = method and
8+
r.getHttpMethod() = method and
99
r.getPath() = path and
1010
r.getController() = controller and
1111
r.getAction() = action

0 commit comments

Comments
 (0)