Skip to content

Commit 0260925

Browse files
erik-krogherik-krogh
committed
changes based on review
1 parent 4e880e2 commit 0260925

3 files changed

Lines changed: 73 additions & 47 deletions

File tree

‎javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,18 @@ module SocketIO {
2323
result = DataFlow::moduleImport("socket.io").getAMemberCall("listen")
2424
}
2525

26+
/**
27+
* A common superclass for all socket-like objects on the serverside of SocketIO.
28+
* All of the subclasses can be used to send data to SocketIO clients (see the `SendNode` class).
29+
*/
2630
abstract private class SocketIOObject extends DataFlow::SourceNode, EventEmitter::Range {
31+
/**
32+
* Gets a node that refers to a SocketIOObject object.
33+
*/
2734
abstract DataFlow::SourceNode ref();
35+
36+
/** Gets namespace belonging to this object. */
37+
abstract NamespaceObject getNamespace();
2838
}
2939

3040
/** A socket.io server. */
@@ -35,6 +45,9 @@ module SocketIO {
3545

3646
/** Gets the default namespace of this server. */
3747
NamespaceObject getDefaultNamespace() { result = MkNamespace(this, "/") }
48+
49+
/** Gets the default namespace of this server. */
50+
override NamespaceObject getNamespace() { result = getDefaultNamespace() }
3851

3952
/** Gets the namespace with the given path of this server. */
4053
NamespaceObject getNamespace(string path) { result = MkNamespace(this, path) }
@@ -126,7 +139,7 @@ module SocketIO {
126139
}
127140

128141
/** Gets the namespace to which this object refers. */
129-
NamespaceObject getNamespace() { result = ns }
142+
override NamespaceObject getNamespace() { result = ns }
130143

131144
/**
132145
* Gets a data flow node that may refer to the socket.io namespace created at `ns`.
@@ -185,7 +198,7 @@ module SocketIO {
185198
}
186199

187200
/** Gets the namespace to which this socket belongs. */
188-
NamespaceObject getNamespace() { result = ns }
201+
override NamespaceObject getNamespace() { result = ns }
189202

190203
private DataFlow::SourceNode socket(DataFlow::TypeTracker t) {
191204
result = this and t.start()
@@ -314,18 +327,14 @@ module SocketIO {
314327

315328
/**
316329
* Gets the socket through which data is sent to the client.
317-
*
318-
* This predicate is not defined for broadcasting sends.
319330
*/
320331
SocketObject getSocket() { result = emitter }
321332

322333
/**
323334
* Gets the namespace to which data is sent.
324335
*/
325336
NamespaceObject getNamespace() {
326-
result = emitter.(ServerObject).getDefaultNamespace() or
327-
result = emitter.(NamespaceBase).getNamespace() or
328-
result = emitter.(SocketObject).getNamespace()
337+
result = emitter.getNamespace()
329338
}
330339

331340
/** Gets the event name associated with the data, if it can be determined. */
@@ -357,7 +366,10 @@ module SocketIO {
357366
srv.ref().getAMethodCall("of").getArgument(0).mayHaveStringValue(path)
358367
}
359368

360-
/** An acknowledgment callback from sending message. */
369+
/**
370+
* An acknowledgment callback registered when sending a message to a client.
371+
* Responses from clients are received using this callback.
372+
*/
361373
class SendCallback extends EventRegistration::Range, DataFlow::FunctionNode {
362374
SendNode send;
363375

@@ -373,7 +385,7 @@ module SocketIO {
373385
override DataFlow::Node getReceivedItem(int i) { result = this.getParameter(i) }
374386

375387
/**
376-
* Get the send node where this callback was registered.
388+
* Gets the send node where this callback was registered.
377389
*/
378390
SendNode getSendNode() { result = send }
379391
}
@@ -552,7 +564,7 @@ module SocketIOClient {
552564
override SocketIO::SendCallback getAReceiver() { result.getSendNode().getAReceiver() = rcv }
553565

554566
/**
555-
* Get the receive node where this callback was registered.
567+
* Gets the receive node where this callback was registered.
556568
*/
557569
ReceiveNode getReceiveNode() { result = rcv }
558570
}
@@ -607,7 +619,10 @@ module SocketIOClient {
607619
}
608620
}
609621

610-
/** An acknowledgment callback from sending message. */
622+
/**
623+
* An acknowledgment callback registered when sending a message to a server.
624+
* Responses from servers are received using this callback.
625+
*/
611626
class SendCallback extends EventRegistration::Range, DataFlow::FunctionNode {
612627
SendNode send;
613628

@@ -621,7 +636,7 @@ module SocketIOClient {
621636
override DataFlow::Node getReceivedItem(int i) { result = this.getParameter(i) }
622637

623638
/**
624-
* Get the SendNode where this callback was registered.
639+
* Gets the SendNode where this callback was registered.
625640
*/
626641
SendNode getSendNode() { result = send }
627642
}

‎javascript/ql/test/library-tests/frameworks/SocketIO/tests.expected‎

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ test_NamespaceNode
2626
| tst.js:45:1:45:7 | ns.json | socket.io namespace with path '/' |
2727
| tst.js:46:1:46:11 | ns.volatile | socket.io namespace with path '/' |
2828
| tst.js:47:1:47:8 | ns.local | socket.io namespace with path '/' |
29-
| tst.js:50:1:66:2 | io.on(' ... cal;\\n}) | socket.io namespace with path '/' |
30-
| tst.js:67:1:67:35 | io.on(' ... => {}) | socket.io namespace with path '/' |
31-
| tst.js:68:1:68:32 | ns.on(' ... => {}) | socket.io namespace with path '/' |
32-
| tst.js:69:1:73:2 | ns.on(' ... {});\\n}) | socket.io namespace with path '/' |
29+
| tst.js:50:1:67:2 | io.on(' ... t');\\n}) | socket.io namespace with path '/' |
30+
| tst.js:68:1:68:35 | io.on(' ... => {}) | socket.io namespace with path '/' |
31+
| tst.js:69:1:69:32 | ns.on(' ... => {}) | socket.io namespace with path '/' |
32+
| tst.js:70:1:74:2 | ns.on(' ... {});\\n}) | socket.io namespace with path '/' |
3333
test_ClientReceiveNode_getASender
3434
| client2.js:4:1:6:2 | sock.on ... y);\\n}) | tst.js:30:1:30:28 | ns.emit ... event') |
3535
| client2.js:4:1:6:2 | sock.on ... y);\\n}) | tst.js:31:1:31:20 | ns.send('a message') |
@@ -39,6 +39,7 @@ test_ClientReceiveNode_getASender
3939
| client2.js:4:1:6:2 | sock.on ... y);\\n}) | tst.js:51:3:51:22 | socket.emit('event') |
4040
| client2.js:4:1:6:2 | sock.on ... y);\\n}) | tst.js:54:3:54:43 | socket. ... => {}) |
4141
| client2.js:4:1:6:2 | sock.on ... y);\\n}) | tst.js:55:3:55:27 | socket. ... ssage') |
42+
| client2.js:4:1:6:2 | sock.on ... y);\\n}) | tst.js:66:3:66:36 | socket. ... dcast') |
4243
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:30:1:30:28 | ns.emit ... event') |
4344
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:31:1:31:20 | ns.send('a message') |
4445
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:39:1:39:31 | io.emit ... ssage') |
@@ -47,6 +48,7 @@ test_ClientReceiveNode_getASender
4748
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:51:3:51:22 | socket.emit('event') |
4849
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:54:3:54:43 | socket. ... => {}) |
4950
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:55:3:55:27 | socket. ... ssage') |
51+
| client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:66:3:66:36 | socket. ... dcast') |
5052
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:30:1:30:28 | ns.emit ... event') |
5153
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:31:1:31:20 | ns.send('a message') |
5254
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:39:1:39:31 | io.emit ... ssage') |
@@ -55,11 +57,12 @@ test_ClientReceiveNode_getASender
5557
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:51:3:51:22 | socket.emit('event') |
5658
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:54:3:54:43 | socket. ... => {}) |
5759
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:55:3:55:27 | socket. ... ssage') |
60+
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:66:3:66:36 | socket. ... dcast') |
5861
| client2.js:18:1:18:41 | sock2.o ... dler')) | tst.js:32:1:32:22 | ns2.wri ... ssage') |
5962
test_ReceiveNode
60-
| tst.js:70:3:70:35 | socket. ... => {}) | tst.js:69:22:69:27 | socket |
61-
| tst.js:71:3:71:46 | socket. ... => {}) | tst.js:69:22:69:27 | socket |
62-
| tst.js:72:3:72:43 | socket. ... => {}) | tst.js:69:22:69:27 | socket |
63+
| tst.js:71:3:71:35 | socket. ... => {}) | tst.js:70:22:70:27 | socket |
64+
| tst.js:72:3:72:46 | socket. ... => {}) | tst.js:70:22:70:27 | socket |
65+
| tst.js:73:3:73:43 | socket. ... => {}) | tst.js:70:22:70:27 | socket |
6366
test_SendNode_getSentItem
6467
| tst.js:30:1:30:28 | ns.emit ... event') | 0 | tst.js:30:18:30:27 | 'an event' |
6568
| tst.js:31:1:31:20 | ns.send('a message') | 0 | tst.js:31:9:31:19 | 'a message' |
@@ -71,8 +74,8 @@ test_SendNode_getSentItem
7174
| tst.js:54:3:54:43 | socket. ... => {}) | 1 | tst.js:54:20:54:28 | 'message' |
7275
| tst.js:55:3:55:27 | socket. ... ssage') | 0 | tst.js:55:16:55:26 | 'a message' |
7376
test_AdditionalFlowStep
74-
| client2.js:16:12:16:25 | "do you copy?" | tst.js:70:25:70:27 | msg |
75-
| client2.js:16:12:16:25 | "do you copy?" | tst.js:71:27:71:31 | data1 |
77+
| client2.js:16:12:16:25 | "do you copy?" | tst.js:71:25:71:27 | msg |
78+
| client2.js:16:12:16:25 | "do you copy?" | tst.js:72:27:72:31 | data1 |
7679
| client3.js:1:8:1:9 | io | client3.js:1:8:1:9 | io |
7780
| client4.js:1:8:1:9 | io | client4.js:1:8:1:9 | io |
7881
| tst.js:30:18:30:27 | 'an event' | client2.js:8:23:8:25 | msg |
@@ -110,12 +113,14 @@ test_SocketNode
110113
| tst.js:63:3:63:17 | socket.volatile | socket.io namespace with path '/' |
111114
| tst.js:64:3:64:18 | socket.broadcast | socket.io namespace with path '/' |
112115
| tst.js:65:3:65:14 | socket.local | socket.io namespace with path '/' |
113-
| tst.js:67:22:67:27 | socket | socket.io namespace with path '/' |
114-
| tst.js:68:19:68:24 | socket | socket.io namespace with path '/' |
115-
| tst.js:69:22:69:27 | socket | socket.io namespace with path '/' |
116-
| tst.js:70:3:70:35 | socket. ... => {}) | socket.io namespace with path '/' |
117-
| tst.js:71:3:71:46 | socket. ... => {}) | socket.io namespace with path '/' |
118-
| tst.js:72:3:72:43 | socket. ... => {}) | socket.io namespace with path '/' |
116+
| tst.js:66:3:66:18 | socket.broadcast | socket.io namespace with path '/' |
117+
| tst.js:66:3:66:36 | socket. ... dcast') | socket.io namespace with path '/' |
118+
| tst.js:68:22:68:27 | socket | socket.io namespace with path '/' |
119+
| tst.js:69:19:69:24 | socket | socket.io namespace with path '/' |
120+
| tst.js:70:22:70:27 | socket | socket.io namespace with path '/' |
121+
| tst.js:71:3:71:35 | socket. ... => {}) | socket.io namespace with path '/' |
122+
| tst.js:72:3:72:46 | socket. ... => {}) | socket.io namespace with path '/' |
123+
| tst.js:73:3:73:43 | socket. ... => {}) | socket.io namespace with path '/' |
119124
test_ClientSendNode_getEventName
120125
| client2.js:14:1:14:32 | sock.em ... there") | data |
121126
| client2.js:16:1:16:36 | sock.wr ... => {}) | message |
@@ -124,8 +129,8 @@ test_ClientSendNode_getSentItem
124129
| client2.js:14:1:14:32 | sock.em ... there") | 1 | client2.js:14:25:14:31 | "there" |
125130
| client2.js:16:1:16:36 | sock.wr ... => {}) | 0 | client2.js:16:12:16:25 | "do you copy?" |
126131
test_ReceiveNode_getEventName
127-
| tst.js:70:3:70:35 | socket. ... => {}) | message |
128-
| tst.js:71:3:71:46 | socket. ... => {}) | message |
132+
| tst.js:71:3:71:35 | socket. ... => {}) | message |
133+
| tst.js:72:3:72:46 | socket. ... => {}) | message |
129134
test_ClientSocketNode
130135
| client1.js:1:1:1:4 | io() | / |
131136
| client1.js:2:1:2:23 | io.conn ... sages") | /messages |
@@ -135,20 +140,21 @@ test_ClientSocketNode
135140
| client4.js:3:1:3:4 | io() | / |
136141
| client4.js:4:1:4:23 | io.conn ... sages") | /messages |
137142
test_ReceiveNode_getASender
138-
| tst.js:70:3:70:35 | socket. ... => {}) | client2.js:14:1:14:32 | sock.em ... there") |
139-
| tst.js:70:3:70:35 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) |
140-
| tst.js:71:3:71:46 | socket. ... => {}) | client2.js:14:1:14:32 | sock.em ... there") |
141-
| tst.js:71:3:71:46 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) |
142-
| tst.js:72:3:72:43 | socket. ... => {}) | client2.js:14:1:14:32 | sock.em ... there") |
143-
| tst.js:72:3:72:43 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) |
143+
| tst.js:71:3:71:35 | socket. ... => {}) | client2.js:14:1:14:32 | sock.em ... there") |
144+
| tst.js:71:3:71:35 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) |
145+
| tst.js:72:3:72:46 | socket. ... => {}) | client2.js:14:1:14:32 | sock.em ... there") |
146+
| tst.js:72:3:72:46 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) |
147+
| tst.js:73:3:73:43 | socket. ... => {}) | client2.js:14:1:14:32 | sock.em ... there") |
148+
| tst.js:73:3:73:43 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) |
144149
test_ReceiveNode_getReceivedItem
145-
| tst.js:70:3:70:35 | socket. ... => {}) | 0 | tst.js:70:25:70:27 | msg |
146-
| tst.js:71:3:71:46 | socket. ... => {}) | 0 | tst.js:71:27:71:31 | data1 |
147-
| tst.js:71:3:71:46 | socket. ... => {}) | 1 | tst.js:71:34:71:38 | data2 |
150+
| tst.js:71:3:71:35 | socket. ... => {}) | 0 | tst.js:71:25:71:27 | msg |
151+
| tst.js:72:3:72:46 | socket. ... => {}) | 0 | tst.js:72:27:72:31 | data1 |
152+
| tst.js:72:3:72:46 | socket. ... => {}) | 1 | tst.js:72:34:72:38 | data2 |
148153
test_SendNode_getSocket
149154
| tst.js:51:3:51:22 | socket.emit('event') | tst.js:50:19:50:24 | socket |
150155
| tst.js:54:3:54:43 | socket. ... => {}) | tst.js:50:19:50:24 | socket |
151156
| tst.js:55:3:55:27 | socket. ... ssage') | tst.js:50:19:50:24 | socket |
157+
| tst.js:66:3:66:36 | socket. ... dcast') | tst.js:50:19:50:24 | socket |
152158
test_ServerNode
153159
| tst.js:1:12:1:33 | require ... .io')() | tst.js:1:12:1:33 | require ... .io')() |
154160
| tst.js:4:13:4:24 | new Server() | tst.js:4:13:4:24 | new Server() |
@@ -162,16 +168,16 @@ test_ServerNode
162168
| tst.js:15:1:15:15 | io.attach(http) | tst.js:1:12:1:33 | require ... .io')() |
163169
| tst.js:16:1:16:15 | io.bind(engine) | tst.js:1:12:1:33 | require ... .io')() |
164170
| tst.js:17:1:17:23 | io.onco ... socket) | tst.js:1:12:1:33 | require ... .io')() |
165-
| tst.js:50:1:66:2 | io.on(' ... cal;\\n}) | tst.js:1:12:1:33 | require ... .io')() |
166-
| tst.js:67:1:67:35 | io.on(' ... => {}) | tst.js:1:12:1:33 | require ... .io')() |
167-
| tst.js:79:1:79:10 | obj.server | tst.js:1:12:1:33 | require ... .io')() |
171+
| tst.js:50:1:67:2 | io.on(' ... t');\\n}) | tst.js:1:12:1:33 | require ... .io')() |
172+
| tst.js:68:1:68:35 | io.on(' ... => {}) | tst.js:1:12:1:33 | require ... .io')() |
173+
| tst.js:80:1:80:10 | obj.server | tst.js:1:12:1:33 | require ... .io')() |
168174
test_ClientSendNode_getAReceiver
169-
| client2.js:14:1:14:32 | sock.em ... there") | tst.js:70:3:70:35 | socket. ... => {}) |
170-
| client2.js:14:1:14:32 | sock.em ... there") | tst.js:71:3:71:46 | socket. ... => {}) |
171-
| client2.js:14:1:14:32 | sock.em ... there") | tst.js:72:3:72:43 | socket. ... => {}) |
172-
| client2.js:16:1:16:36 | sock.wr ... => {}) | tst.js:70:3:70:35 | socket. ... => {}) |
173-
| client2.js:16:1:16:36 | sock.wr ... => {}) | tst.js:71:3:71:46 | socket. ... => {}) |
174-
| client2.js:16:1:16:36 | sock.wr ... => {}) | tst.js:72:3:72:43 | socket. ... => {}) |
175+
| client2.js:14:1:14:32 | sock.em ... there") | tst.js:71:3:71:35 | socket. ... => {}) |
176+
| client2.js:14:1:14:32 | sock.em ... there") | tst.js:72:3:72:46 | socket. ... => {}) |
177+
| client2.js:14:1:14:32 | sock.em ... there") | tst.js:73:3:73:43 | socket. ... => {}) |
178+
| client2.js:16:1:16:36 | sock.wr ... => {}) | tst.js:71:3:71:35 | socket. ... => {}) |
179+
| client2.js:16:1:16:36 | sock.wr ... => {}) | tst.js:72:3:72:46 | socket. ... => {}) |
180+
| client2.js:16:1:16:36 | sock.wr ... => {}) | tst.js:73:3:73:43 | socket. ... => {}) |
175181
test_ClientReceiveNode_getAck
176182
| client2.js:10:1:12:2 | sock.on ... d");\\n}) | client2.js:10:22:10:23 | cb |
177183
test_ClientReceiveNode_getReceivedItem
@@ -205,6 +211,7 @@ test_SendNode
205211
| tst.js:51:3:51:22 | socket.emit('event') | socket.io namespace with path '/' |
206212
| tst.js:54:3:54:43 | socket. ... => {}) | socket.io namespace with path '/' |
207213
| tst.js:55:3:55:27 | socket. ... ssage') | socket.io namespace with path '/' |
214+
| tst.js:66:3:66:36 | socket. ... dcast') | socket.io namespace with path '/' |
208215
test_SendNode_getAReceiver
209216
| tst.js:30:1:30:28 | ns.emit ... event') | client2.js:4:1:6:2 | sock.on ... y);\\n}) |
210217
| tst.js:30:1:30:28 | ns.emit ... event') | client2.js:8:1:8:33 | sock.on ... => {}) |
@@ -231,6 +238,9 @@ test_SendNode_getAReceiver
231238
| tst.js:55:3:55:27 | socket. ... ssage') | client2.js:4:1:6:2 | sock.on ... y);\\n}) |
232239
| tst.js:55:3:55:27 | socket. ... ssage') | client2.js:8:1:8:33 | sock.on ... => {}) |
233240
| tst.js:55:3:55:27 | socket. ... ssage') | client2.js:10:1:12:2 | sock.on ... d");\\n}) |
241+
| tst.js:66:3:66:36 | socket. ... dcast') | client2.js:4:1:6:2 | sock.on ... y);\\n}) |
242+
| tst.js:66:3:66:36 | socket. ... dcast') | client2.js:8:1:8:33 | sock.on ... => {}) |
243+
| tst.js:66:3:66:36 | socket. ... dcast') | client2.js:10:1:12:2 | sock.on ... d");\\n}) |
234244
test_ServerObject
235245
| tst.js:1:12:1:33 | require ... .io')() | tst.js:1:12:1:33 | require ... .io')() | socket.io namespace with path '/' |
236246
| tst.js:4:13:4:24 | new Server() | tst.js:4:13:4:24 | new Server() | socket.io namespace with path '/' |

‎javascript/ql/test/library-tests/frameworks/SocketIO/tst.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ io.on('connect', (socket) => {
6363
socket.volatile;
6464
socket.broadcast;
6565
socket.local;
66+
socket.broadcast.emit('broadcast');
6667
});
6768
io.on('connection', (socket) => {});
6869
ns.on('connect', (socket) => {});

0 commit comments

Comments
 (0)