From cceb22cc9c32309f26c4ad1dae430a44706770ad Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 11 Nov 2020 15:55:25 +0100 Subject: [PATCH 1/2] Python: Add example of top-level module shadowing stdlib Although this test is added under the `wrong` folder, the current results from this CodeQL test is actually correct (compared with the Python interpreter). However, they don't match what the extractor does when invoked with `codeql database create`. I want to use this test-case as a reference, when updating the tool invocation from our testing setup to match what happens in the extractor. It would be better to just fix the extractor, but that is not trivial, since it requires us to correctly guess the root of source-code in a project (so we can simulate it becoming the first element of `sys.path`). Making the behavior of our testing setup the same as the production environment is a step in the right direction in my opinion (even if that means having the same known problems). Inspired by the debugging in https://github.com/github/codeql/issues/4640 --- .../module-imports/conflict-stdlib/README.md | 6 ++++++ .../ReferenceToLocalModule.expected | 2 ++ .../conflict-stdlib/ReferenceToLocalModule.ql | 15 +++++++++++++++ .../wrong/module-imports/conflict-stdlib/cmd.py | 2 ++ .../wrong/module-imports/conflict-stdlib/options | 1 + .../module-imports/conflict-stdlib/test_fail.py | 3 +++ .../module-imports/conflict-stdlib/test_ok.py | 2 ++ 7 files changed, 31 insertions(+) create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.ql create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/cmd.py create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_fail.py create mode 100644 python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_ok.py diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md new file mode 100644 index 000000000000..0886d49788b8 --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md @@ -0,0 +1,6 @@ +This test shows how we handle modules the shadow a module in the standard library. + +Because we have a `cmd.py` file, whenever the python interpreter sees `import cmd`, that is the file that will be used! -- + +* `python test_ok.py` works as intended, and prints `Foo` +* `python test_fail.py` raises an exception, since it imports `pdb.py` from the standard library, which (at least in Python 3.8) tries to import `cmd.py` from the standard library, but instead is served our `cmd.py` module. Therefore it fails with `AttributeError: module 'cmd' has no attribute 'Cmd'` diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected new file mode 100644 index 000000000000..90f666a99c02 --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected @@ -0,0 +1,2 @@ +| Local module 'cmd' referenced in module 'pdb' which is NOT part of this projects code | +| Local module 'cmd' referenced in module 'test_ok' which is part of this projects code | diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.ql b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.ql new file mode 100644 index 000000000000..e577d90d7301 --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.ql @@ -0,0 +1,15 @@ +import python + +from ModuleValue project_module, ControlFlowNode ref, string part_of_project +where + exists(project_module.getScope().getFile().getRelativePath()) and + ref = project_module.getAReference() and + ( + exists(ref.getLocation().getFile().getRelativePath()) and + part_of_project = "is part of this projects code" + or + not exists(ref.getLocation().getFile().getRelativePath()) and + part_of_project = "is NOT part of this projects code" + ) +select "Local module '" + project_module.getName() + "' referenced in module '" + + ref.getEnclosingModule().getName() + "' which " + part_of_project diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/cmd.py b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/cmd.py new file mode 100644 index 000000000000..58bbb12f69c1 --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/cmd.py @@ -0,0 +1,2 @@ +foo = "Foo" +print("my own cmd imported") diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options new file mode 100644 index 000000000000..b91afde07678 --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options @@ -0,0 +1 @@ +semmle-extractor-options: --max-import-depth=2 diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_fail.py b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_fail.py new file mode 100644 index 000000000000..f9621b260937 --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_fail.py @@ -0,0 +1,3 @@ +# we import `pdb` which import the `cmd` module from the standard library +# and allows us to set --max-import-depth=2, to make the test run fast +import pdb diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_ok.py b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_ok.py new file mode 100644 index 000000000000..6dcf8eb614eb --- /dev/null +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_ok.py @@ -0,0 +1,2 @@ +from cmd import foo +print(foo) From 62b7fddf6415468bbf83f5404a4843786d0978c2 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 18 Nov 2020 11:09:56 +0100 Subject: [PATCH 2/2] Python: run test like `codeql database create` Changing the way qltest invokes the extractor was a bit of a challenge, and there are more important things to do right now... creating an internal tracking issue for this instead. --- .../regressions/wrong/module-imports/conflict-stdlib/README.md | 2 ++ .../conflict-stdlib/ReferenceToLocalModule.expected | 2 -- .../wrong/module-imports/conflict-stdlib/{ => code}/cmd.py | 0 .../module-imports/conflict-stdlib/{ => code}/test_fail.py | 0 .../wrong/module-imports/conflict-stdlib/{ => code}/test_ok.py | 0 .../regressions/wrong/module-imports/conflict-stdlib/options | 2 +- 6 files changed, 3 insertions(+), 3 deletions(-) rename python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/{ => code}/cmd.py (100%) rename python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/{ => code}/test_fail.py (100%) rename python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/{ => code}/test_ok.py (100%) diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md index 0886d49788b8..cd607bc0cf78 100644 --- a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/README.md @@ -1,5 +1,7 @@ This test shows how we handle modules the shadow a module in the standard library. +We manually replicate the behavior of `codeql database create --source-root `, which will use `-R `. By default, the way qltest invokes the extractor will cause different behavior. Therefore, we also need to move our code outside of the top-level folder, and it lives in `code/`. + Because we have a `cmd.py` file, whenever the python interpreter sees `import cmd`, that is the file that will be used! -- * `python test_ok.py` works as intended, and prints `Foo` diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected index 90f666a99c02..e69de29bb2d1 100644 --- a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/ReferenceToLocalModule.expected @@ -1,2 +0,0 @@ -| Local module 'cmd' referenced in module 'pdb' which is NOT part of this projects code | -| Local module 'cmd' referenced in module 'test_ok' which is part of this projects code | diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/cmd.py b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/code/cmd.py similarity index 100% rename from python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/cmd.py rename to python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/code/cmd.py diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_fail.py b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/code/test_fail.py similarity index 100% rename from python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_fail.py rename to python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/code/test_fail.py diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_ok.py b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/code/test_ok.py similarity index 100% rename from python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/test_ok.py rename to python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/code/test_ok.py diff --git a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options index b91afde07678..572393de94a5 100644 --- a/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options +++ b/python/ql/test/library-tests/PointsTo/regressions/wrong/module-imports/conflict-stdlib/options @@ -1 +1 @@ -semmle-extractor-options: --max-import-depth=2 +semmle-extractor-options: --max-import-depth=2 -R code/