Skip to content

Commit 6de38b1

Browse files
authored
Merge pull request #19300 from Napalys/js/fastify
JS: Added support for `fastify.addHook`
2 parents abbf753 + 8b53f8f commit 6de38b1

File tree

6 files changed

+375
-5
lines changed

6 files changed

+375
-5
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added support for the `fastify` `addHook` method.

javascript/ql/lib/semmle/javascript/Routing.qll

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ module Routing {
139139
predicate mayResumeDispatch() {
140140
this.getLastChild().mayResumeDispatch()
141141
or
142+
isInMiddlewareSetup(this)
143+
or
142144
exists(this.(RouteHandler).getAContinuationInvocation())
143145
or
144146
// Leaf nodes that aren't functions are assumed to invoke their continuation
@@ -155,6 +157,8 @@ module Routing {
155157
predicate definitelyResumesDispatch() {
156158
this.getLastChild().definitelyResumesDispatch()
157159
or
160+
isInMiddlewareSetup(this)
161+
or
158162
exists(this.(RouteHandler).getAContinuationInvocation())
159163
or
160164
this instanceof MkRouter
@@ -325,6 +329,19 @@ module Routing {
325329
DataFlow::Node getValueImplicitlyStoredInAccessPath(int n, string path) { none() }
326330
}
327331

332+
/**
333+
* Holds if `node` is installed at a route handler that is declared to be a middleware setup,
334+
* and is therefore assume to resume dispatch.
335+
*/
336+
private predicate isInMiddlewareSetup(Node node) {
337+
exists(RouteSetup::Range range |
338+
node = getRouteSetupNode(range) and
339+
range.isMiddlewareSetup()
340+
)
341+
or
342+
isInMiddlewareSetup(node.getParent())
343+
}
344+
328345
/** Holds if `pred` and `succ` are adjacent siblings and `succ` is installed after `pred`. */
329346
private predicate areSiblings(Node pred, Node succ) {
330347
exists(ValueNode::Range base, int n |
@@ -612,6 +629,20 @@ module Routing {
612629
* Holds if this route setup targets `router` and occurs at the given `cfgNode`.
613630
*/
614631
abstract predicate isInstalledAt(Router::Range router, ControlFlowNode cfgNode);
632+
633+
/**
634+
* Holds if this is a middleware setup, meaning dispatch will resume after the
635+
* route handlers in this route setup have completed (usually meaning that they have returned a promise, which has resolved).
636+
*
637+
* This should only be overridden when the route setup itself determines whether subsequent
638+
* route handlers are invoked afterwards.
639+
* - For Express-like libraries, the route _handler_ determines whether to resume dispatch,
640+
* based on whether the `next` callback is invoked. For such libraries, do not override `isMiddlewareSetup`.
641+
* - For Fastify-like libraries, the route _setup_ determines whether to resume dispatch.
642+
* For example, `.addHook()` will resume dispatch whereas `.get()` will not. `isMiddlewareSetup()` should thus
643+
* hold for `.addHook()` but not for `.get()` calls.
644+
*/
645+
predicate isMiddlewareSetup() { none() }
615646
}
616647

617648
/**
@@ -892,10 +923,14 @@ module Routing {
892923
* based on `Node::Range::getValueAtAccessPath`.
893924
*/
894925
private DataFlow::Node getAnAccessPathRhs(Node base, int n, string path) {
895-
// Assigned in the body of a route handler function, whi
926+
// Assigned in the body of a route handler function, which is a middleware
896927
exists(RouteHandler handler | base = handler |
897928
result = AccessPath::getAnAssignmentTo(handler.getParameter(n).ref(), path) and
898-
exists(handler.getAContinuationInvocation())
929+
(
930+
exists(handler.getAContinuationInvocation())
931+
or
932+
isInMiddlewareSetup(handler)
933+
)
899934
)
900935
or
901936
// Implicit assignment contributed by framework model

javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ module Fastify {
138138

139139
RouteSetup() {
140140
this = server(server).getAMethodCall(methodName) and
141-
methodName = ["route", "get", "head", "post", "put", "delete", "options", "patch"]
141+
methodName = ["route", "get", "head", "post", "put", "delete", "options", "patch", "addHook"]
142142
}
143143

144144
override DataFlow::SourceNode getARouteHandler() {
@@ -164,13 +164,19 @@ module Fastify {
164164

165165
private class ShorthandRoutingTreeSetup extends Routing::RouteSetup::MethodCall instanceof RouteSetup
166166
{
167-
ShorthandRoutingTreeSetup() { not this.getMethodName() = "route" }
167+
ShorthandRoutingTreeSetup() { not this.getMethodName() = ["route", "addHook"] }
168168

169169
override string getRelativePath() { result = this.getArgument(0).getStringValue() }
170170

171171
override Http::RequestMethodName getHttpMethod() { result = this.getMethodName().toUpperCase() }
172172
}
173173

174+
private class AddHookRouteSetup extends Routing::RouteSetup::MethodCall instanceof RouteSetup {
175+
AddHookRouteSetup() { this.getMethodName() = "addHook" }
176+
177+
override predicate isMiddlewareSetup() { any() }
178+
}
179+
174180
/** Gets the name of the `n`th handler function that can be installed a route setup, in order of execution. */
175181
private string getNthHandlerName(int n) {
176182
result =
@@ -322,7 +328,11 @@ module Fastify {
322328
ResponseSendArgument() {
323329
this = rh.getAResponseSource().ref().getAMethodCall("send").getArgument(0)
324330
or
325-
this = rh.(DataFlow::FunctionNode).getAReturn()
331+
exists(RouteSetup setup |
332+
rh = setup.getARouteHandler() and
333+
this = rh.(DataFlow::FunctionNode).getAReturn() and
334+
setup.getMethodName() != "addHook"
335+
)
326336
}
327337

328338
override RouteHandler getRouteHandler() { result = rh }

0 commit comments

Comments
 (0)