From 884b911a9cbf3c97689b473ae7f732fc72e06eea Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 7 Sep 2023 12:49:25 -0300 Subject: [PATCH 1/2] Fix crash when passing a very long cmdline argument (#11404) Fixes #11394 (cherry picked from commit 28ccf476b91be32ffda303f0d7a8b57e475b465b) --- changelog/11394.bugfix.rst | 1 + src/_pytest/main.py | 3 ++- src/_pytest/pathlib.py | 11 +++++++++++ testing/test_main.py | 31 +++++++++++++++++++++++++++++++ testing/test_pathlib.py | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 changelog/11394.bugfix.rst diff --git a/changelog/11394.bugfix.rst b/changelog/11394.bugfix.rst new file mode 100644 index 00000000000..aa89c81b01f --- /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/main.py b/src/_pytest/main.py index 803b95a2033..ea89a63fa1b 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -36,6 +36,7 @@ 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 7cf64f03b2c..b5c2d86452f 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -1,5 +1,6 @@ import atexit import contextlib +import errno import fnmatch import importlib.util import itertools @@ -791,3 +792,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 OSError as e: + if e.errno == errno.ENAMETOOLONG: + return False + raise diff --git a/testing/test_main.py b/testing/test_main.py index 71597626790..3c8998c1a35 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -262,3 +262,34 @@ def test(fix): "* 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 1ca6414375e..8a9659aabd9 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 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,33 @@ def __init__(self) -> None: 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=OSError(errno.EIO, "another kind of error"), + ): + with pytest.raises(OSError): + _ = safe_exists(p) From 63b0c6f75f218b400a5a42305f9fa3830448f7e8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 7 Sep 2023 13:15:11 -0300 Subject: [PATCH 2/2] Use _pytest.pathlib.safe_exists in get_dirs_from_args Related to #11394 --- src/_pytest/config/__init__.py | 9 +++------ src/_pytest/config/findpaths.py | 9 +-------- src/_pytest/pathlib.py | 9 ++++----- testing/test_pathlib.py | 5 ++--- 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index dc2b9f6a160..e3990d175df 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -57,6 +57,7 @@ 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 @@ def _set_initial_conftests( 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 234b9e12906..02674ffae3b 100644 --- a/src/_pytest/config/findpaths.py +++ b/src/_pytest/config/findpaths.py @@ -16,6 +16,7 @@ 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_dir_from_path(path: Path) -> 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/pathlib.py b/src/_pytest/pathlib.py index b5c2d86452f..5c765c68348 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -1,6 +1,5 @@ import atexit import contextlib -import errno import fnmatch import importlib.util import itertools @@ -798,7 +797,7 @@ 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 OSError as e: - if e.errno == errno.ENAMETOOLONG: - return False - raise + 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_pathlib.py b/testing/test_pathlib.py index 8a9659aabd9..678fd27feac 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -688,7 +688,6 @@ def test_safe_exists(tmp_path: Path) -> None: Path, "exists", autospec=True, - side_effect=OSError(errno.EIO, "another kind of error"), + side_effect=ValueError("name too long"), ): - with pytest.raises(OSError): - _ = safe_exists(p) + assert safe_exists(p) is False