diff --git a/changelog/11394.bugfix.rst b/changelog/11394.bugfix.rst new file mode 100644 index 000000000..aa89c81b0 --- /dev/null +++ b/changelog/11394.bugfix.rst @@ -0,0 +1 @@ +Fixed crash when parsing long command line arguments that might be interpreted as files. diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index dc2b9f6a1..e3990d175 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -57,6 +57,7 @@ from _pytest.pathlib import bestrelpath from _pytest.pathlib import import_path from _pytest.pathlib import ImportMode from _pytest.pathlib import resolve_package_path +from _pytest.pathlib import safe_exists from _pytest.stash import Stash from _pytest.warning_types import PytestConfigWarning from _pytest.warning_types import warn_explicit_for @@ -557,12 +558,8 @@ class PytestPluginManager(PluginManager): anchor = absolutepath(current / path) # Ensure we do not break if what appears to be an anchor - # is in fact a very long option (#10169). - try: - anchor_exists = anchor.exists() - except OSError: # pragma: no cover - anchor_exists = False - if anchor_exists: + # is in fact a very long option (#10169, #11394). + if safe_exists(anchor): self._try_load_conftest(anchor, importmode, rootpath) foundanchor = True if not foundanchor: diff --git a/src/_pytest/config/findpaths.py b/src/_pytest/config/findpaths.py index 234b9e129..02674ffae 100644 --- a/src/_pytest/config/findpaths.py +++ b/src/_pytest/config/findpaths.py @@ -16,6 +16,7 @@ from .exceptions import UsageError from _pytest.outcomes import fail from _pytest.pathlib import absolutepath from _pytest.pathlib import commonpath +from _pytest.pathlib import safe_exists if TYPE_CHECKING: from . import Config @@ -151,14 +152,6 @@ def get_dirs_from_args(args: Iterable[str]) -> List[Path]: return path return path.parent - def safe_exists(path: Path) -> bool: - # This can throw on paths that contain characters unrepresentable at the OS level, - # or with invalid syntax on Windows (https://bugs.python.org/issue35306) - try: - return path.exists() - except OSError: - return False - # These look like paths but may not exist possible_paths = ( absolutepath(get_file_part_from_node_id(arg)) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 803b95a20..ea89a63fa 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -36,6 +36,7 @@ from _pytest.outcomes import exit from _pytest.pathlib import absolutepath from _pytest.pathlib import bestrelpath from _pytest.pathlib import fnmatch_ex +from _pytest.pathlib import safe_exists from _pytest.pathlib import visit from _pytest.reports import CollectReport from _pytest.reports import TestReport @@ -895,7 +896,7 @@ def resolve_collection_argument( strpath = search_pypath(strpath) fspath = invocation_path / strpath fspath = absolutepath(fspath) - if not fspath.exists(): + if not safe_exists(fspath): msg = ( "module or package not found: {arg} (missing __init__.py?)" if as_pypath diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 7cf64f03b..5c765c683 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -791,3 +791,13 @@ def copytree(source: Path, target: Path) -> None: shutil.copyfile(x, newx) elif x.is_dir(): newx.mkdir(exist_ok=True) + + +def safe_exists(p: Path) -> bool: + """Like Path.exists(), but account for input arguments that might be too long (#11394).""" + try: + return p.exists() + except (ValueError, OSError): + # ValueError: stat: path too long for Windows + # OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect + return False diff --git a/testing/test_main.py b/testing/test_main.py index 715976267..3c8998c1a 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -262,3 +262,34 @@ def test_module_full_path_without_drive(pytester: Pytester) -> None: "* 1 passed in *", ] ) + + +def test_very_long_cmdline_arg(pytester: Pytester) -> None: + """ + Regression test for #11394. + + Note: we could not manage to actually reproduce the error with this code, we suspect + GitHub runners are configured to support very long paths, however decided to leave + the test in place in case this ever regresses in the future. + """ + pytester.makeconftest( + """ + import pytest + + def pytest_addoption(parser): + parser.addoption("--long-list", dest="long_list", action="store", default="all", help="List of things") + + @pytest.fixture(scope="module") + def specified_feeds(request): + list_string = request.config.getoption("--long-list") + return list_string.split(',') + """ + ) + pytester.makepyfile( + """ + def test_foo(specified_feeds): + assert len(specified_feeds) == 100_000 + """ + ) + result = pytester.runpytest("--long-list", ",".join(["helloworld"] * 100_000)) + result.stdout.fnmatch_lines("* 1 passed *") diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 1ca641437..678fd27fe 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -1,3 +1,4 @@ +import errno import os.path import pickle import sys @@ -24,6 +25,7 @@ from _pytest.pathlib import insert_missing_modules from _pytest.pathlib import maybe_delete_a_numbered_dir from _pytest.pathlib import module_name_from_path from _pytest.pathlib import resolve_package_path +from _pytest.pathlib import safe_exists from _pytest.pathlib import symlink_or_skip from _pytest.pathlib import visit from _pytest.tmpdir import TempPathFactory @@ -660,3 +662,32 @@ class TestImportLibMode: mod = import_path(init, root=tmp_path, mode=ImportMode.importlib) assert len(mod.instance.INSTANCES) == 1 + + +def test_safe_exists(tmp_path: Path) -> None: + d = tmp_path.joinpath("some_dir") + d.mkdir() + assert safe_exists(d) is True + + f = tmp_path.joinpath("some_file") + f.touch() + assert safe_exists(f) is True + + # Use unittest.mock() as a context manager to have a very narrow + # patch lifetime. + p = tmp_path.joinpath("some long filename" * 100) + with unittest.mock.patch.object( + Path, + "exists", + autospec=True, + side_effect=OSError(errno.ENAMETOOLONG, "name too long"), + ): + assert safe_exists(p) is False + + with unittest.mock.patch.object( + Path, + "exists", + autospec=True, + side_effect=ValueError("name too long"), + ): + assert safe_exists(p) is False