Repository navigation
Commit 69d4702
fix(submodule): check Windows names before path normalization
The Windows 3.10 CI job failed the new destination tests for `space ` and
`period.` with `AssertionError: clone attempted`. Windows'
`GetFullPathName`, used by `os.path.abspath`, removes those trailing
characters before `_to_relative_path` returns. The filename check therefore
received an already-altered path and allowed cloning.
Validate the Windows filename components before normalizing the checkout
path. Exclude the drive or UNC share prefix from filename validation so
ordinary absolute destinations remain supported; the existing containment
check still rejects paths outside the worktree and drive-relative paths.
This retains the filename rules from Git's `is_valid_win32_path` while
applying them before the native path transformation can erase evidence.
Add two regressions that simulate the Windows normalizer on every platform.
Both failed before this change. Focused destination, relative-path, and move
tests now pass: 24 passed and 2 Windows-only tests skipped locally.
`mypy --platform win32`, Ruff lint, formatting, and `git diff --check` pass.1 parent faf32c2 commit 69d4702
2 files changed
Lines changed: 12 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -320,7 +320,7 @@ def _validated_name(cls, name: str) -> str: | |||
| 320 | 320 | def _validate_windows_path(path: PathLike) -> None: | |
| 321 | 321 | """Apply Git for Windows' filename checks before creating directories.""" | |
| 322 | 322 | if sys.platform == "win32": | |
| 323 | - for component in os.fspath(path).replace("\\", "/").split("/"): | ||
| 323 | + for component in ntpath.splitdrive(os.fspath(path))[1].replace("\\", "/").split("/"): | ||
| 324 | 324 | if component in (".", ".."): | |
| 325 | 325 | continue | |
| 326 | 326 | stem = component.split(".", 1)[0].rstrip(" ").upper() | |
@@ -462,6 +462,7 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike: | |||
| 462 | 462 | :raise ValueError: | |
| 463 | 463 | If path is outside the working tree or is unsafe as a submodule checkout. | |
| 464 | 464 | """ | |
| 465 | + cls._validate_windows_path(path) | ||
| 465 | 466 | if parent_repo.working_tree_dir: | |
| 466 | 467 | path = _to_relative_path(parent_repo.working_tree_dir, path) | |
| 467 | 468 | else: | |
@@ -472,7 +473,6 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike: | |||
| 472 | 473 | raise ValueError("Submodule checkout path must not be the repository root") | |
| 473 | 474 | ||
| 474 | 475 | _validate_repo_path(path) | |
| 475 | - cls._validate_windows_path(path) | ||
| 476 | 476 | return path | |
| 477 | 477 | ||
| 478 | 478 | @property | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -276,6 +276,16 @@ def test_submodule_rejects_windows_destination_names_before_mutation(movable_sub | |||
| 276 | 276 | assert set(Path(sm.repo.working_tree_dir).rglob("*")) == paths | |
| 277 | 277 | ||
| 278 | 278 | ||
| 279 | + @pytest.mark.parametrize("path", ["nested/space ", "nested/period."]) | ||
| 280 | + def test_windows_destination_validation_precedes_normalization(tmp_path, path): | ||
| 281 | + parent = SimpleNamespace(working_tree_dir=str(tmp_path)) | ||
| 282 | + # Windows' GetFullPathName removes trailing spaces and periods. | ||
| 283 | + with mock.patch("git.objects.submodule.base._to_relative_path", return_value=path.rstrip(" .")): | ||
| 284 | + with mock.patch("git.objects.submodule.base.sys", SimpleNamespace(platform="win32")): | ||
| 285 | + with pytest.raises(ValueError, match="Invalid submodule path on Windows"): | ||
| 286 | + Submodule._to_relative_path(parent, path) | ||
| 287 | + | ||
| 288 | + | ||
| 279 | 289 | def test_submodule_can_relocate_its_own_metadata(movable_submodule): | |
| 280 | 290 | sm = movable_submodule | |
| 281 | 291 | sm.rename(f"{sm.name}/child") | |
0 commit comments