pylint-dev / pylint-dev/astroid

ZipFinder is overzealous in searching for imports

Open
#607 2 comments 0 reactions 0 assignees View on GitHub
Bug 🪳
Dominant language
Python
Stars
582
Forks
357
Avg merge
1d 1h
Merged PRs (30d)
23

Description

This is perhaps best documented with the code I've used to find the flaws.
```python
from astroid.modutils import file_info_from_modpath
import sys
import os

# Create a basic zip file with some test modules and packages inside
if not os.path.exists("zip_test.zip"):
import zipfile
print("creating zip_test.zip")
with zipfile.ZipFile("zip_test.zip", "x") as test_file:
w = test_file.writestr
w("zip_member.py", "var = 'member'")
w("zip_pkg/__init__.py", "var = 'pkg'")
w("zip_pkg/pkg_mod.py", "var = 'pkg/mod'")
w("zip_pkg/pkg_inv/inv_mod.py", "var = 'pkg/inv/inv'")

w("zip_ns/ns_inv.py", "var = 'ns/inv'")
w("zip_ns/ns_pkg/__init__.py", "var = 'ns/pkg'")
w("zip_ns/ns_pkg/ns_mod.py", "var = 'ns/pkg/mod'")

# It appars zipimport requires namespace directories to have an entry in
# the zip file. Is that standard with zip files?
w("zip_ns/", "")

# Test if a given module can be found with the given path
def test(mod, path):
curent_dir = os.path.dirname(os.path.abspath(__file__))
try:
info = file_info_from_modpath(mod.split('.'), path=path)
except ImportError:
print(f"{mod}, {path}: not found")
else:
display_type = info.type.name if hasattr(info.type, 'name') else info.type
if info.location is not None and info.location.startswith(curent_dir):
display_path = info.location[len(curent_dir)+1:]
# This is perhaps a bit too simple, and may not work on all python installs
elif info.location is not None and info.location.startswith(sys.path[2]):
display_path = "sys"+info.location[len(sys.path[2]):]
else:
display_path = info.location
print(f"{mod}, {path}: type {display_type} found at {display_path}")

# NOTE: with the exception of the ns_pkg, all of the zip imports are
# reporting paths without the .py suffix

test("xml", ["/invalid"]) # incorrect, should not be found
test("xml", []) # incorrect, should not be found

print() # These are unrelated to ZipFinder, but worth noting.
test("os.path", ["/invalid"]) # incorrect, should not be found, type is int(1)
test("os.path", []) # incorrect, should not be found, type is int(1)
test("os.path", None) # incorrect, type is int(1)

print()
test("zip_member", None) # ok, not found
test("zip_member", ["zip_test.zip"]) # ok, found
# ZipFinder cached the path to the zip provided and now searches it

print()
test("zip_member", None) # incorrect, should not be found
test("zip_pkg", None) # incorrect, should not be found
test("zip_pkg.pkg_mod", None) # incorrect, should not be found

print()
test("zip_pkg", ["zip_test.zip"]) # incorrect, path does not go to __init__.py
test("zip_pkg.pkg_mod", ["zip_test.zip"]) # ok
test("zip_ns", ["zip_test.zip"]) # ok
# Only the first level is checked for actually being a package
test("zip_pkg.pkg_inv.inv_mod", ["zip_test.zip"]) # incorrect, not importable
# The check doesn't allow namespace packages, which means these fail
test("zip_ns.ns_pkg.ns_mod", ["zip_test.zip"]) # incorrect, not found
test("zip_ns.ns_pkg", ["zip_test.zip"]) # incorrect, type is None
# the path to the namespace package was cached and ZipFinder now searches it
test("ns_pkg", None) # Incorrect, should not be found

print()
# paths can be specified, which doesn't really make much sense.
test("zip_pkg/pkg_mod", ["zip_test.zip"]) # incorrect, invalid module name
# The normal finder also suffers from this
test("urllib/request", None) # incorrect, invalid module name

print()
test("pkg_mod", None) # ok, not found
test("ns_mod", None) # ok, not found

# Importing a package should not affect the module found but the import
# machinery adds imports to sys.path_importer_cache, the same cache used
# by the ZipFinder.
sys.path.append("zip_test.zip")
import zip_pkg.pkg_mod
import zip_ns.ns_pkg.ns_mod

test("pkg_mod", None) # Incorrect, should not be found
test("ns_mod", None) # Incorrect, should not be found
```

Output
```
xml, ['/invalid']: type PY_ZIPMODULE found at sys/xml
xml, []: type PY_ZIPMODULE found at sys/xml

os.path, ['/invalid']: type 1 found at sys/posixpath.py
os.path, []: type 1 found at sys/posixpath.py
os.path, None: type 1 found at sys/posixpath.py

zip_member, None: not found
zip_member, ['zip_test.zip']: type PY_ZIPMODULE found at zip_test.zip/zip_member

zip_member, None: type PY_ZIPMODULE found at zip_test.zip/zip_member
zip_pkg, None: type PY_ZIPMODULE found at zip_test.zip/zip_pkg
zip_pkg.pkg_mod, None: type PY_ZIPMODULE found at zip_test.zip/zip_pkg/pkg_mod

zip_pkg, ['zip_test.zip']: type PY_ZIPMODULE found at zip_test.zip/zip_pkg
zip_pkg.pkg_mod, ['zip_test.zip']: type PY_ZIPMODULE found at zip_test.zip/zip_pkg/pkg_mod
zip_ns, ['zip_test.zip']: type PY_NAMESPACE found at None
zip_pkg.pkg_inv.inv_mod, ['zip_test.zip']: type PY_ZIPMODULE found at zip_test.zip/zip_pkg/pkg_inv/inv_mod
zip_ns.ns_pkg.ns_mod, ['zip_test.zip']: not found
zip_ns.ns_pkg, ['zip_test.zip']: type None found at zip_test.zip/zip_ns/ns_pkg/__init__.py
ns_pkg, None: type PY_ZIPMODULE found at zip_test.zip/zip_ns/ns_pkg

zip_pkg/pkg_mod, ['zip_test.zip']: type PY_ZIPMODULE found at zip_test.zip/zip_pkg/pkg_mod
urllib/request, None: type PY_SOURCE found at sys/urllib/request.py

pkg_mod, None: not found
ns_mod, None: not found
pkg_mod, None: type PY_ZIPMODULE found at zip_test.zip/zip_pkg/pkg_mod
ns_mod, None: type PY_ZIPMODULE found at zip_test.zip/zip_ns/ns_pkg/ns_mod
```
The TL;DR is ZipFinder searches every path ever, even if it isn't a zip file, even if it isn't supposed to search any path in the first place. There's not filter, and the passed path argument is only used to add more paths to search for. It also doesn't properly handle namespace packages inside zip files.

(tested on commit 3f0fdc5d "version" 2.0.4 using Python 3.7)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by running the provided Python reproduction against astroid.modutils.file_info_from_modpath and inspect the ZipFinder path-search and caching behavior. Trace how explicit paths, cached paths, invalid module names, and namespace packages are handled. Done means unrelated paths are not searched or retained, invalid names are rejected, and namespace packages inside zip files are handled correctly with regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.