Skip to content

Commit

Permalink
Merge pull request #11406 from nicoddemus/backport-11404-to-7.4.x
Browse files Browse the repository at this point in the history
[7.4.x] Fix crash when passing a very long cmdline argument (#11404)
  • Loading branch information
nicoddemus committed Sep 7, 2023
2 parents 6e49a74 + 63b0c6f commit e4f022f
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 15 deletions.
1 change: 1 addition & 0 deletions changelog/11394.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed crash when parsing long command line arguments that might be interpreted as files.
9 changes: 3 additions & 6 deletions src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 1 addition & 8 deletions src/_pytest/config/findpaths.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
3 changes: 2 additions & 1 deletion src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/_pytest/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
31 changes: 31 additions & 0 deletions testing/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 *")
31 changes: 31 additions & 0 deletions testing/test_pathlib.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import errno
import os.path
import pickle
import sys
Expand All @@ -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
Expand Down Expand Up @@ -660,3 +662,32 @@ 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=ValueError("name too long"),
):
assert safe_exists(p) is False

0 comments on commit e4f022f

Please sign in to comment.