From 289ad7724791edafc135dde8d93bbf4d2b2d581b Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 10 Feb 2024 17:11:13 -0800 Subject: [PATCH 1/3] Simplify check for symlinks that resolve outside root 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. --- src/black/__init__.py | 11 ++++------- src/black/files.py | 24 +++++++++++------------- tests/test_black.py | 11 +++++++---- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 8ab5b47f974..2d4c7f655ad 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -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 ( @@ -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: @@ -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)) diff --git a/src/black/files.py b/src/black/files.py index 960f13ee270..806152ab279 100644 --- a/src/black/files.py +++ b/src/black/files.py @@ -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( @@ -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(): diff --git a/tests/test_black.py b/tests/test_black.py index f876d365b12..6c15f32d54c 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -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": @@ -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" From d1e56839bbc94f7a36d7eeb9777f4a9fe7c9b0df Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 10 Feb 2024 18:07:54 -0800 Subject: [PATCH 2/3] catch os.getcwd --- src/black/files.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/black/files.py b/src/black/files.py index 806152ab279..6c05105450c 100644 --- a/src/black/files.py +++ b/src/black/files.py @@ -263,17 +263,16 @@ def resolves_outside_root_or_cannot_stat( 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: + if sys.version_info < (3, 8, 6): + path = path.absolute() # https://bugs.python.org/issue33660 resolved_path = path.resolve() + return get_root_relative_path(resolved_path, root, report) is None except OSError as e: if report: report.path_ignored(path, f"cannot be read because {e}") return True - return get_root_relative_path(resolved_path, root, report) is None - def get_root_relative_path( path: Path, From 800afb3f498df61adb981ee6199619d35fe61bb3 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 10 Feb 2024 20:12:37 -0800 Subject: [PATCH 3/3] Update tests/test_black.py --- tests/test_black.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_black.py b/tests/test_black.py index 6c15f32d54c..5c6920c2b77 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -1768,7 +1768,7 @@ def test_bpo_33660_workaround(self) -> None: resolves_outside = black.resolves_outside_root_or_cannot_stat( path, root, report ) - self.assertEqual(resolves_outside, False) + self.assertIs(resolves_outside, False) def test_normalize_path_ignore_windows_junctions_outside_of_root(self) -> None: if system() != "Windows":