Skip to content

Commit 3e13056

Browse files
yoffyoff
committed
Python: Address most review comments
1 parent e8ce62e commit 3e13056

File tree

3 files changed

+117
-65
lines changed

3 files changed

+117
-65
lines changed

‎python/ql/test/experimental/dataflow/coverage/dataflow.expected

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,39 @@
11
edges
2-
| test.py:23:9:23:14 | ControlFlowNode for SOURCE | test.py:24:10:24:10 | ControlFlowNode for x |
3-
| test.py:28:9:28:16 | ControlFlowNode for Str | test.py:29:10:29:10 | ControlFlowNode for x |
4-
| test.py:32:9:32:17 | ControlFlowNode for Str | test.py:33:10:33:10 | ControlFlowNode for x |
5-
| test.py:36:9:36:10 | ControlFlowNode for IntegerLiteral | test.py:37:10:37:10 | ControlFlowNode for x |
6-
| test.py:40:9:40:12 | ControlFlowNode for FloatLiteral | test.py:41:10:41:10 | ControlFlowNode for x |
7-
| test.py:49:10:49:15 | ControlFlowNode for SOURCE | test.py:50:10:50:10 | ControlFlowNode for x |
8-
| test.py:226:15:226:20 | ControlFlowNode for SOURCE | test.py:226:10:226:21 | ControlFlowNode for f() |
9-
| test.py:282:12:282:17 | ControlFlowNode for SOURCE | test.py:282:10:282:18 | ControlFlowNode for f() |
2+
| test.py:35:9:35:14 | ControlFlowNode for SOURCE | test.py:36:10:36:10 | ControlFlowNode for x |
3+
| test.py:40:9:40:16 | ControlFlowNode for Str | test.py:41:10:41:10 | ControlFlowNode for x |
4+
| test.py:44:9:44:17 | ControlFlowNode for Str | test.py:45:10:45:10 | ControlFlowNode for x |
5+
| test.py:48:9:48:10 | ControlFlowNode for IntegerLiteral | test.py:49:10:49:10 | ControlFlowNode for x |
6+
| test.py:52:9:52:12 | ControlFlowNode for FloatLiteral | test.py:53:10:53:10 | ControlFlowNode for x |
7+
| test.py:61:10:61:15 | ControlFlowNode for SOURCE | test.py:62:10:62:10 | ControlFlowNode for x |
8+
| test.py:238:28:238:33 | ControlFlowNode for SOURCE | test.py:238:10:238:34 | ControlFlowNode for second() |
9+
| test.py:297:12:297:17 | ControlFlowNode for SOURCE | test.py:297:10:297:18 | ControlFlowNode for f() |
10+
| test.py:301:28:301:33 | ControlFlowNode for SOURCE | test.py:301:10:301:34 | ControlFlowNode for second() |
1011
nodes
11-
| test.py:23:9:23:14 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
12-
| test.py:24:10:24:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
13-
| test.py:28:9:28:16 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
14-
| test.py:29:10:29:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
15-
| test.py:32:9:32:17 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
16-
| test.py:33:10:33:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
17-
| test.py:36:9:36:10 | ControlFlowNode for IntegerLiteral | semmle.label | ControlFlowNode for IntegerLiteral |
18-
| test.py:37:10:37:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
19-
| test.py:40:9:40:12 | ControlFlowNode for FloatLiteral | semmle.label | ControlFlowNode for FloatLiteral |
12+
| test.py:35:9:35:14 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
13+
| test.py:36:10:36:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
14+
| test.py:40:9:40:16 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
2015
| test.py:41:10:41:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
21-
| test.py:49:10:49:15 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
22-
| test.py:50:10:50:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
23-
| test.py:226:10:226:21 | ControlFlowNode for f() | semmle.label | ControlFlowNode for f() |
24-
| test.py:226:15:226:20 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
25-
| test.py:282:10:282:18 | ControlFlowNode for f() | semmle.label | ControlFlowNode for f() |
26-
| test.py:282:12:282:17 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
16+
| test.py:44:9:44:17 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
17+
| test.py:45:10:45:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
18+
| test.py:48:9:48:10 | ControlFlowNode for IntegerLiteral | semmle.label | ControlFlowNode for IntegerLiteral |
19+
| test.py:49:10:49:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
20+
| test.py:52:9:52:12 | ControlFlowNode for FloatLiteral | semmle.label | ControlFlowNode for FloatLiteral |
21+
| test.py:53:10:53:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
22+
| test.py:61:10:61:15 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
23+
| test.py:62:10:62:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
24+
| test.py:238:10:238:34 | ControlFlowNode for second() | semmle.label | ControlFlowNode for second() |
25+
| test.py:238:28:238:33 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
26+
| test.py:297:10:297:18 | ControlFlowNode for f() | semmle.label | ControlFlowNode for f() |
27+
| test.py:297:12:297:17 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
28+
| test.py:301:10:301:34 | ControlFlowNode for second() | semmle.label | ControlFlowNode for second() |
29+
| test.py:301:28:301:33 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
2730
#select
28-
| test.py:24:10:24:10 | ControlFlowNode for x | test.py:23:9:23:14 | ControlFlowNode for SOURCE | test.py:24:10:24:10 | ControlFlowNode for x | <message> |
29-
| test.py:29:10:29:10 | ControlFlowNode for x | test.py:28:9:28:16 | ControlFlowNode for Str | test.py:29:10:29:10 | ControlFlowNode for x | <message> |
30-
| test.py:33:10:33:10 | ControlFlowNode for x | test.py:32:9:32:17 | ControlFlowNode for Str | test.py:33:10:33:10 | ControlFlowNode for x | <message> |
31-
| test.py:37:10:37:10 | ControlFlowNode for x | test.py:36:9:36:10 | ControlFlowNode for IntegerLiteral | test.py:37:10:37:10 | ControlFlowNode for x | <message> |
32-
| test.py:41:10:41:10 | ControlFlowNode for x | test.py:40:9:40:12 | ControlFlowNode for FloatLiteral | test.py:41:10:41:10 | ControlFlowNode for x | <message> |
33-
| test.py:50:10:50:10 | ControlFlowNode for x | test.py:49:10:49:15 | ControlFlowNode for SOURCE | test.py:50:10:50:10 | ControlFlowNode for x | <message> |
34-
| test.py:226:10:226:21 | ControlFlowNode for f() | test.py:226:15:226:20 | ControlFlowNode for SOURCE | test.py:226:10:226:21 | ControlFlowNode for f() | <message> |
35-
| test.py:282:10:282:18 | ControlFlowNode for f() | test.py:282:12:282:17 | ControlFlowNode for SOURCE | test.py:282:10:282:18 | ControlFlowNode for f() | <message> |
31+
| test.py:36:10:36:10 | ControlFlowNode for x | test.py:35:9:35:14 | ControlFlowNode for SOURCE | test.py:36:10:36:10 | ControlFlowNode for x | <message> |
32+
| test.py:41:10:41:10 | ControlFlowNode for x | test.py:40:9:40:16 | ControlFlowNode for Str | test.py:41:10:41:10 | ControlFlowNode for x | <message> |
33+
| test.py:45:10:45:10 | ControlFlowNode for x | test.py:44:9:44:17 | ControlFlowNode for Str | test.py:45:10:45:10 | ControlFlowNode for x | <message> |
34+
| test.py:49:10:49:10 | ControlFlowNode for x | test.py:48:9:48:10 | ControlFlowNode for IntegerLiteral | test.py:49:10:49:10 | ControlFlowNode for x | <message> |
35+
| test.py:53:10:53:10 | ControlFlowNode for x | test.py:52:9:52:12 | ControlFlowNode for FloatLiteral | test.py:53:10:53:10 | ControlFlowNode for x | <message> |
36+
| test.py:62:10:62:10 | ControlFlowNode for x | test.py:61:10:61:15 | ControlFlowNode for SOURCE | test.py:62:10:62:10 | ControlFlowNode for x | <message> |
37+
| test.py:238:10:238:34 | ControlFlowNode for second() | test.py:238:28:238:33 | ControlFlowNode for SOURCE | test.py:238:10:238:34 | ControlFlowNode for second() | <message> |
38+
| test.py:297:10:297:18 | ControlFlowNode for f() | test.py:297:12:297:17 | ControlFlowNode for SOURCE | test.py:297:10:297:18 | ControlFlowNode for f() | <message> |
39+
| test.py:301:10:301:34 | ControlFlowNode for second() | test.py:301:28:301:33 | ControlFlowNode for SOURCE | test.py:301:10:301:34 | ControlFlowNode for second() | <message> |

‎python/ql/test/experimental/dataflow/coverage/localFlow.expected

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
| test.py:17:5:17:5 | SSA variable x | test.py:16:1:16:33 | Exit node for Function test_tuple_with_local_flow |
2-
| test.py:17:5:17:5 | SSA variable x | test.py:18:9:18:9 | ControlFlowNode for x |
3-
| test.py:17:10:17:18 | ControlFlowNode for Tuple | test.py:17:5:17:5 | SSA variable x |
4-
| test.py:18:5:18:5 | SSA variable y | test.py:19:5:19:11 | SSA variable y |
5-
| test.py:18:5:18:5 | SSA variable y | test.py:19:10:19:10 | ControlFlowNode for y |
6-
| test.py:18:9:18:12 | ControlFlowNode for Subscript | test.py:18:5:18:5 | SSA variable y |
7-
| test.py:19:5:19:11 | SSA variable y | test.py:16:1:16:33 | Exit node for Function test_tuple_with_local_flow |
1+
| test.py:24:5:24:5 | SSA variable x | test.py:23:1:23:33 | Exit node for Function test_tuple_with_local_flow |
2+
| test.py:24:5:24:5 | SSA variable x | test.py:25:9:25:9 | ControlFlowNode for x |
3+
| test.py:24:10:24:26 | ControlFlowNode for Tuple | test.py:24:5:24:5 | SSA variable x |
4+
| test.py:25:5:25:5 | SSA variable y | test.py:26:5:26:11 | SSA variable y |
5+
| test.py:25:5:25:5 | SSA variable y | test.py:26:10:26:10 | ControlFlowNode for y |
6+
| test.py:25:9:25:12 | ControlFlowNode for Subscript | test.py:25:5:25:5 | SSA variable y |
7+
| test.py:26:5:26:11 | SSA variable y | test.py:23:1:23:33 | Exit node for Function test_tuple_with_local_flow |

‎python/ql/test/experimental/dataflow/coverage/test.py

Lines changed: 75 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,30 @@
66
#
77
# Functions whose name ends with "_with_local_flow" will also be tested for local flow.
88
#
9-
# All functions starting with "test_" should run and print a source (sources are defined in testConfig.qll).
9+
# All functions starting with "test_" should run and either
10+
# - print a source (sources are defined in testConfig.qll).
11+
# - print "Unexpected flow: " and a non-source
12+
# (The idea is to later write a script to autimatically confirm this.)
1013

1114
# These are defined so that we can evaluate the test code.
15+
NONSOURCE = "not a source"
1216
SOURCE = "source"
1317
def SINK(x):
1418
print(x)
1519

20+
def SINK_F(x):
21+
print("Unexpected flow: ", x)
22+
1623
def test_tuple_with_local_flow():
17-
x = (3, SOURCE)
24+
x = (NONSOURCE, SOURCE)
1825
y = x[1]
1926
SINK(y) # Flow missing
2027

28+
def test_tuple_negative():
29+
x = (NONSOURCE, SOURCE)
30+
y = x[0]
31+
SINK_F(y)
32+
2133
# 6.2.1. Identifiers (Names)
2234
def test_names():
2335
x = SOURCE
@@ -54,8 +66,12 @@ def test_list_display():
5466
x = [SOURCE]
5567
SINK(x[0]) # Flow missing
5668

69+
def test_list_display_negative():
70+
x = [SOURCE]
71+
SINK_F(x)
72+
5773
def test_list_comprehension():
58-
x = [SOURCE for y in [3]]
74+
x = [SOURCE for y in [NONSOURCE]]
5975
SINK(x[0]) # Flow missing
6076

6177
def test_nested_list_display():
@@ -68,7 +84,7 @@ def test_set_display():
6884
SINK(x.pop()) # Flow missing
6985

7086
def test_set_comprehension():
71-
x = {SOURCE for y in [3]}
87+
x = {SOURCE for y in [NONSOURCE]}
7288
SINK(x.pop()) # Flow missing
7389

7490
def test_nested_set_display():
@@ -90,7 +106,7 @@ def test_nested_dict_display():
90106

91107
# 6.2.8. Generator expressions
92108
def test_generator():
93-
x = (SOURCE for y in [3])
109+
x = (SOURCE for y in [NONSOURCE])
94110
SINK([*x][0]) # Flow missing
95111

96112
# 6.2.9. Yield expressions
@@ -123,13 +139,13 @@ def gen2(x):
123139
yield m
124140

125141
def test_send():
126-
g = gen2(3)
142+
g = gen2(NONSOURCE)
127143
n = next(g)
128144
SINK(g.send(SOURCE)) # Flow missing
129145

130146
def gen_ex(x):
131147
try:
132-
yield 3
148+
yield NONSOURCE
133149
except:
134150
yield x # `x` has to flow to call to `throw`
135151

@@ -163,7 +179,7 @@ async def agen2(x):
163179
yield m
164180

165181
async def atest_asend():
166-
g = agen2(3)
182+
g = agen2(NONSOURCE)
167183
n = await g.__anext__()
168184
SINK(await g.asend(SOURCE)) # Flow missing
169185

@@ -172,7 +188,7 @@ def test_asend():
172188

173189
async def agen_ex(x):
174190
try:
175-
yield 3
191+
yield NONSOURCE
176192
except:
177193
yield x # `x` has to flow to call to `athrow`
178194

@@ -194,10 +210,6 @@ def test_attribute_reference():
194210
# overriding __getattr__ should be tested by the class coverage tests
195211

196212
# 6.3.2. Subscriptions
197-
# This does not constitute dataflow (but could be taint flow)
198-
def example_subscription_string():
199-
SINK("source"[0]) # Flow not expected
200-
201213
def test_subscription_tuple():
202214
SINK((SOURCE,)[0]) # Flow missing
203215

@@ -219,32 +231,35 @@ def test_slicing():
219231
# The grammar seems to allow `l[0:1:1, 0:1]`, but the interpreter does not like it
220232

221233
# 6.3.4. Calls
222-
def f(a, b):
234+
def second(a, b):
223235
return b
224236

225237
def test_call_positional():
226-
SINK(f(3, SOURCE))
238+
SINK(second(NONSOURCE, SOURCE))
239+
240+
def test_call_positional_negative():
241+
SINK_F(second(SOURCE, NONSOURCE))
227242

228243
def test_call_keyword():
229-
SINK(f(3, b=SOURCE)) # Flow missing
244+
SINK(second(NONSOURCE, b=SOURCE)) # Flow missing
230245

231246
def test_call_unpack_iterable():
232-
SINK(f(3, *[SOURCE])) # Flow missing
247+
SINK(second(NONSOURCE, *[SOURCE])) # Flow missing
233248

234249
def test_call_unpack_mapping():
235-
SINK(f(3, **{"b": SOURCE})) # Flow missing
250+
SINK(second(NONSOURCE, **{"b": SOURCE})) # Flow missing
236251

237252
def f_extra_pos(a, *b):
238253
return b[0]
239254

240255
def test_call_extra_pos():
241-
SINK(f_extra_pos(3, SOURCE)) # Flow missing
256+
SINK(f_extra_pos(NONSOURCE, SOURCE)) # Flow missing
242257

243258
def f_extra_keyword(a, **b):
244259
return b["b"]
245260

246261
def test_call_extra_keyword():
247-
SINK(f_extra_keyword(3, b=SOURCE)) # Flow missing
262+
SINK(f_extra_keyword(NONSOURCE, b=SOURCE)) # Flow missing
248263

249264
# return the name of the first extra keyword argument
250265
def f_extra_keyword_flow(**a):
@@ -256,27 +271,60 @@ def test_call_extra_keyword_flow():
256271

257272
# 6.12. Assignment expressions
258273
def test_assignment_expression():
259-
x = 3
274+
x = NONSOURCE
260275
SINK(x := SOURCE) # Flow missing
261276

262277
# 6.13. Conditional expressions
263278
def test_conditional_true():
264-
SINK(SOURCE if True else 3) # Flow missing
279+
SINK(SOURCE if True else NONSOURCE) # Flow missing
265280

266281
def test_conditional_false():
267-
SINK(3 if False else SOURCE) # Flow missing
282+
SINK(NONSOURCE if False else SOURCE) # Flow missing
268283

269284
# Condition is evaluated first, so x is SOURCE once chosen
270285
def test_conditional_evaluation_true():
271-
x = 3
272-
SINK(x if (SOURCE == (x := SOURCE)) else 3) # Flow missing
286+
x = NONSOURCE
287+
SINK(x if (SOURCE == (x := SOURCE)) else NONSOURCE) # Flow missing
273288

274289
# Condition is evaluated first, so x is SOURCE once chosen
275290
def test_conditional_evaluation_false():
276-
x = 3
277-
SINK(3 if (3 == (x := SOURCE)) else x) # Flow missing
291+
x = NONSOURCE
292+
SINK(NONSOURCE if (NONSOURCE == (x := SOURCE)) else x) # Flow missing
278293

279294
# 6.14. Lambdas
280295
def test_lambda():
281296
f = lambda x : x
282297
SINK(f(SOURCE))
298+
299+
def test_lambda_positional():
300+
second = lambda a, b : b
301+
SINK(second(NONSOURCE, SOURCE))
302+
303+
def test_lambda_positional_negative():
304+
second = lambda a, b : b
305+
SINK_F(second(SOURCE, NONSOURCE))
306+
307+
def test_lambda_keyword():
308+
second = lambda a, b : b
309+
SINK(second(NONSOURCE, b=SOURCE)) # Flow missing
310+
311+
def test_lambda_unpack_iterable():
312+
second = lambda a, b : b
313+
SINK(second(NONSOURCE, *[SOURCE])) # Flow missing
314+
315+
def test_lambda_unpack_mapping():
316+
second = lambda a, b : b
317+
SINK(second(NONSOURCE, **{"b": SOURCE})) # Flow missing
318+
319+
def test_lambda_extra_pos():
320+
f_extra_pos = lambda a, *b : b[0]
321+
SINK(f_extra_pos(NONSOURCE, SOURCE)) # Flow missing
322+
323+
def test_lambda_extra_keyword():
324+
f_extra_keyword = lambda a, **b : b["b"]
325+
SINK(f_extra_keyword(NONSOURCE, b=SOURCE)) # Flow missing
326+
327+
# call the function with our source as the name of the keyword arguemnt
328+
def test_lambda_extra_keyword_flow():
329+
f_extra_keyword_flow = lambda **a : [*a][0] # return the name of the first extra keyword argument
330+
SINK(f_extra_keyword_flow(**{SOURCE: None})) # Flow missing

0 commit comments

Comments
 (0)