Skip to content

Commit de58f50

Browse files
committed
forbid symlinks and files in the path (#301)
We now definitely do extra work in some cases, but save a lot in others thanks to the sorted input.
1 parent a3501df commit de58f50

File tree

2 files changed

+103
-15
lines changed

2 files changed

+103
-15
lines changed

git-worktree/src/index/checkout.rs

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ pub struct PathCache {
2626
valid_relative: PathBuf,
2727
/// The amount of path components of 'valid' beyond the roots components. If `root` has 2, and this is 2, `valid` has 4 components.
2828
valid_components: usize,
29+
30+
/// just for testing
31+
#[cfg(debug_assertions)]
32+
pub test_mkdir_calls: usize,
2933
}
3034

3135
mod cache {
@@ -42,6 +46,7 @@ mod cache {
4246
valid_relative: PathBuf::with_capacity(128),
4347
valid_components: 0,
4448
root,
49+
test_mkdir_calls: 0,
4550
}
4651
}
4752

@@ -51,20 +56,14 @@ mod cache {
5156
/// The full path to `relative` will be returned for use on the file system.
5257
pub fn append_relative_path_assure_leading_dir(
5358
&mut self,
54-
relative: &Path,
59+
relative: impl AsRef<Path>,
5560
mode: git_index::entry::Mode,
5661
) -> std::io::Result<&Path> {
62+
let relative = relative.as_ref();
5763
debug_assert!(
5864
relative.is_relative(),
5965
"only index paths are handled correctly here, must be relative"
6066
);
61-
assert!(
62-
(git_index::entry::Mode::SYMLINK
63-
| git_index::entry::Mode::FILE
64-
| git_index::entry::Mode::FILE_EXECUTABLE)
65-
.contains(mode),
66-
"can only handle file-like items right now"
67-
);
6867

6968
let mut components = relative.components().peekable();
7069
let mut existing_components = self.valid_relative.components();
@@ -84,15 +83,26 @@ mod cache {
8483
}
8584

8685
self.valid_components = matching_components;
87-
for comp in components {
86+
87+
let target_is_dir = mode == git_index::entry::Mode::COMMIT || mode == git_index::entry::Mode::DIR;
88+
while let Some(comp) = components.next() {
8889
self.valid.push(comp);
8990
self.valid_relative.push(comp);
9091
self.valid_components += 1;
92+
if components.peek().is_some() || target_is_dir {
93+
self.test_mkdir_calls += 1;
94+
match std::fs::create_dir(&self.valid) {
95+
Ok(()) => {}
96+
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
97+
if !self.valid.symlink_metadata()?.is_dir() {
98+
return Err(err);
99+
}
100+
}
101+
Err(err) => return Err(err),
102+
}
103+
}
91104
}
92105

93-
if self.valid_components > 1 {
94-
std::fs::create_dir_all(self.valid.parent().expect("directory"))?;
95-
}
96106
Ok(&self.valid)
97107
}
98108
}

git-worktree/tests/index/mod.rs

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,81 @@
11
mod checkout {
2+
mod cache {
3+
use git_index::entry::Mode;
4+
use git_worktree::index::checkout::PathCache;
5+
use std::path::Path;
6+
use tempfile::{tempdir, TempDir};
7+
8+
#[test]
9+
fn root_is_assumed_to_exist_and_files_in_root_do_not_create_directory() {
10+
let dir = tempdir().unwrap();
11+
let mut cache = PathCache::new(dir.path().join("non-existing-root"));
12+
assert_eq!(cache.test_mkdir_calls, 0);
13+
14+
let path = cache
15+
.append_relative_path_assure_leading_dir("hello", Mode::FILE)
16+
.unwrap();
17+
assert!(!path.parent().unwrap().exists(), "prefix itself is never created");
18+
assert_eq!(cache.test_mkdir_calls, 0);
19+
}
20+
21+
#[test]
22+
fn directory_paths_are_created_in_full() {
23+
let (mut cache, _tmp) = new_cache();
24+
25+
for (name, mode) in &[
26+
("dir", Mode::DIR),
27+
("submodule", Mode::COMMIT),
28+
("file", Mode::FILE),
29+
("exe", Mode::FILE_EXECUTABLE),
30+
("link", Mode::SYMLINK),
31+
] {
32+
let path = cache
33+
.append_relative_path_assure_leading_dir(Path::new("dir").join(name), *mode)
34+
.unwrap();
35+
assert!(path.parent().unwrap().is_dir(), "dir exists");
36+
}
37+
38+
assert_eq!(cache.test_mkdir_calls, 3);
39+
}
40+
41+
#[test]
42+
fn existing_directories_are_fine() {
43+
let (mut cache, tmp) = new_cache();
44+
std::fs::create_dir(tmp.path().join("dir")).unwrap();
45+
46+
let path = cache
47+
.append_relative_path_assure_leading_dir("dir/file", Mode::FILE)
48+
.unwrap();
49+
assert!(path.parent().unwrap().is_dir(), "directory is still present");
50+
assert!(!path.exists(), "it won't create the file");
51+
}
52+
53+
#[test]
54+
fn symlinks_or_files_in_path_are_forbidden_or_unlinked_when_forced() {
55+
let (mut cache, tmp) = new_cache();
56+
let forbidden = tmp.path().join("forbidden");
57+
std::fs::create_dir(&forbidden).unwrap();
58+
symlink::symlink_dir(&forbidden, tmp.path().join("link-to-dir")).unwrap();
59+
std::fs::write(tmp.path().join("file-in-dir"), &[]).unwrap();
60+
61+
for dirname in &["link-to-dir", "file-in-dir"] {
62+
assert_eq!(
63+
cache
64+
.append_relative_path_assure_leading_dir(format!("{}/file", dirname), Mode::FILE)
65+
.unwrap_err()
66+
.kind(),
67+
std::io::ErrorKind::AlreadyExists
68+
);
69+
}
70+
}
71+
72+
fn new_cache() -> (PathCache, TempDir) {
73+
let dir = tempdir().unwrap();
74+
let cache = PathCache::new(dir.path());
75+
(cache, dir)
76+
}
77+
}
78+
279
#[cfg(unix)]
380
use std::os::unix::prelude::MetadataExt;
481
use std::{
@@ -16,13 +93,14 @@ mod checkout {
1693
use crate::fixture_path;
1794

1895
#[test]
19-
fn symlinks_become_files_if_disabled() {
96+
fn symlinks_become_files_if_disabled() -> crate::Result {
2097
let opts = opts_with_symlink(false);
2198
let (source_tree, destination, _index, outcome) =
22-
checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules").unwrap();
99+
checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?;
23100

24-
assert_equality(&source_tree, &destination, opts.fs.symlink).unwrap();
101+
assert_equality(&source_tree, &destination, opts.fs.symlink)?;
25102
assert!(outcome.collisions.is_empty());
103+
Ok(())
26104
}
27105

28106
#[test]

0 commit comments

Comments
 (0)