Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/11394.bugfix.rst
@@ -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
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
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
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
Expand Up @@ -791,3 +791,13 @@
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):

Check warning on line 800 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L800

Added line #L800 was not covered by tests
# ValueError: stat: path too long for Windows
# OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect
return False

Check warning on line 803 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L803

Added line #L803 was not covered by tests
31 changes: 31 additions & 0 deletions testing/test_main.py
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
@@ -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