Merge pull request #11406 from nicoddemus/backport-11404-to-7.4.x

[7.4.x] Fix crash when passing a very long cmdline argument (#11404)

changelog/11394.bugfix.rst

@@ -0,0 +1 @@
+Fixed crash when parsing long command line arguments that might be interpreted as files.

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).
+            # is in fact a very long option (#10169, #11394).
-            try:
+            if safe_exists(anchor):
-                anchor_exists = anchor.exists()
-            except OSError:  # pragma: no cover
-                anchor_exists = False
-            if anchor_exists:
                 self._try_load_conftest(anchor, importmode, rootpath)
                 foundanchor = True
         if not foundanchor:

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))

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

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

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 *")

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