Skip to content

Commit

Permalink
Simplify check for symlinks that resolve outside root
Browse files Browse the repository at this point in the history
This PR does not change any behaviour.

There have been 1-2 issues about symlinks recently. Both over and under
resolving can cause problems. This makes a case where we resolve more
explicit and prevent a resolved path from leaking out via the return.
  • Loading branch information
hauntsaninja committed Feb 11, 2024
1 parent dab37a6 commit 289ad77
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 24 deletions.
11 changes: 4 additions & 7 deletions src/black/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@
gen_python_files,
get_gitignore,
get_root_relative_path,
normalize_path_maybe_ignore,
parse_pyproject_toml,
path_is_excluded,
resolves_outside_root_or_cannot_stat,
wrap_stream_for_windows,
)
from black.handle_ipynb_magics import (
Expand Down Expand Up @@ -763,12 +763,9 @@ def get_sources(
)
continue

normalized_path: Optional[str] = normalize_path_maybe_ignore(
path, root, report
)
if normalized_path is None:
if resolves_outside_root_or_cannot_stat(path, root, report):
if verbose:
out(f'Skipping invalid source: "{normalized_path}"', fg="red")
out(f'Skipping invalid source: "{path}"', fg="red")
continue

if is_stdin:
Expand All @@ -780,7 +777,7 @@ def get_sources(
continue

if verbose:
out(f'Found input source: "{normalized_path}"', fg="blue")
out(f'Found input source: "{path}"', fg="blue")
sources.add(path)
elif path.is_dir():
path = root / (path.resolve().relative_to(root))
Expand Down
24 changes: 11 additions & 13 deletions src/black/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,26 +254,25 @@ def get_gitignore(root: Path) -> PathSpec:
raise


def normalize_path_maybe_ignore(
def resolves_outside_root_or_cannot_stat(
path: Path,
root: Path,
report: Optional[Report] = None,
) -> Optional[str]:
"""Normalize `path`. May return `None` if `path` was ignored.
`report` is where "path ignored" output goes.
) -> bool:
"""
Returns whether the path is a symbolic link that points outside the
root directory. Also returns True if we failed to resolve the path.
"""
if sys.version_info < (3, 8, 6):
path = path.absolute() # https://bugs.python.org/issue33660
try:
abspath = path if path.is_absolute() else Path.cwd() / path
normalized_path = abspath.resolve()
root_relative_path = get_root_relative_path(normalized_path, root, report)

resolved_path = path.resolve()
except OSError as e:
if report:
report.path_ignored(path, f"cannot be read because {e}")
return None
return True

return root_relative_path
return get_root_relative_path(resolved_path, root, report) is None


def get_root_relative_path(
Expand Down Expand Up @@ -369,8 +368,7 @@ def gen_python_files(
report.path_ignored(child, "matches the --force-exclude regular expression")
continue

normalized_path = normalize_path_maybe_ignore(child, root, report)
if normalized_path is None:
if resolves_outside_root_or_cannot_stat(child, root, report):
continue

if child.is_dir():
Expand Down
11 changes: 7 additions & 4 deletions tests/test_black.py
Original file line number Diff line number Diff line change
Expand Up @@ -1760,12 +1760,15 @@ def test_bpo_33660_workaround(self) -> None:
return

# https://bugs.python.org/issue33660
# Can be removed when we drop support for Python 3.8.5
root = Path("/")
with change_directory(root):
path = Path("workspace") / "project"
report = black.Report(verbose=True)
normalized_path = black.normalize_path_maybe_ignore(path, root, report)
self.assertEqual(normalized_path, "workspace/project")
resolves_outside = black.resolves_outside_root_or_cannot_stat(
path, root, report
)
self.assertEqual(resolves_outside, False)

def test_normalize_path_ignore_windows_junctions_outside_of_root(self) -> None:
if system() != "Windows":
Expand All @@ -1778,13 +1781,13 @@ def test_normalize_path_ignore_windows_junctions_outside_of_root(self) -> None:
os.system(f"mklink /J {junction_dir} {junction_target_outside_of_root}")

report = black.Report(verbose=True)
normalized_path = black.normalize_path_maybe_ignore(
resolves_outside = black.resolves_outside_root_or_cannot_stat(
junction_dir, root, report
)
# Manually delete for Python < 3.8
os.system(f"rmdir {junction_dir}")

self.assertEqual(normalized_path, None)
self.assertIs(resolves_outside, True)

def test_newline_comment_interaction(self) -> None:
source = "class A:\\\r\n# type: ignore\n pass\n"
Expand Down

0 comments on commit 289ad77

Please sign in to comment.