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

fix: symlinks handling #3298

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
0189750
don't warn on ignored directory symlinks
2ZeroSix Jan 18, 2022
7320081
Don't crash on directory symlinks (default behavior is copy-paste of …
2ZeroSix Jan 18, 2022
963b645
add missing await
2ZeroSix Feb 1, 2022
b87e533
remove unnecessary brackets in string interpolation
2ZeroSix Feb 1, 2022
c087295
codestyle
2ZeroSix Feb 1, 2022
6e23dcc
add symlink cycle test
2ZeroSix Feb 1, 2022
b9b9314
use ignore rule without delimiter in checked-in but ignored warning test
2ZeroSix Feb 1, 2022
4486f00
probable fix for cycles on windows
2ZeroSix Feb 3, 2022
e49c45b
don't follow links
2ZeroSix Feb 3, 2022
7e1cb20
use appropriate error message (wrongly cherry-picked)
2ZeroSix Feb 3, 2022
05f6cd9
double check for directory symlinks
2ZeroSix Feb 3, 2022
0a6ffaa
replace "throws" => "not throws on valid directory symlinks"
2ZeroSix Feb 3, 2022
10b8e3e
Revert "double check for directory symlinks"
2ZeroSix Feb 12, 2022
f6bd8d2
Revert "don't follow links"
2ZeroSix Feb 12, 2022
bfc30e5
Revert "probable fix for cycles on windows"
2ZeroSix Feb 12, 2022
b1b69bc
try to resolve symbolic link manually before listSync invocation
2ZeroSix Feb 12, 2022
d97bfe2
Revert "Revert "probable fix for cycles on windows""
2ZeroSix Feb 12, 2022
add6e5c
Revert "Revert "don't follow links""
2ZeroSix Feb 12, 2022
380a1e0
Revert "Revert "double check for directory symlinks""
2ZeroSix Feb 12, 2022
6623ea2
add additional cycles tests
2ZeroSix Feb 12, 2022
8e6aa02
try to see detailed log on windows
2ZeroSix Feb 13, 2022
aa391dc
maintain list of visited symlink dirs
2ZeroSix Jun 3, 2022
dfb47e5
use resolvedLinkTargets
2ZeroSix Jun 3, 2022
2705410
fix assertLinksResolvable
2ZeroSix Jun 3, 2022
f3aa7c3
update message
2ZeroSix Jun 3, 2022
ad092c5
add a bit more tests
2ZeroSix Jun 3, 2022
003a59d
try to resolve or else count occurrences
2ZeroSix Jun 3, 2022
73455c6
typo: symlinks => symlink
2ZeroSix Jun 3, 2022
fa09a85
rewrite symlinks detection
2ZeroSix Jun 4, 2022
1a72dc3
create and append in single assertion
2ZeroSix Jun 4, 2022
3a58e92
posix dirname for internal representation
2ZeroSix Jun 4, 2022
ce5544a
fix copy on write solution
2ZeroSix Jun 5, 2022
d5b595b
code style
2ZeroSix Jun 5, 2022
7745fc7
explicitly say that error happened because of loop
2ZeroSix Jun 5, 2022
af93eb1
not throws on ignored broken directory symlinks
2ZeroSix Jun 5, 2022
8aa575d
not throws on valid links to the same directory
2ZeroSix Jun 5, 2022
9700d92
instant loop is actually non-resolving
2ZeroSix Jun 5, 2022
0dcf557
add detailed comment with explanation
2ZeroSix Jun 5, 2022
9c2c78d
make assert methods static
2ZeroSix Jun 5, 2022
42b0821
add test for nested loop
2ZeroSix Jun 5, 2022
020f465
rename: posixDir => internalDir
2ZeroSix Jun 5, 2022
07b8cf9
make assertions private
2ZeroSix Jun 5, 2022
68831dc
use join instead of raw posix path
2ZeroSix Jun 5, 2022
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
88 changes: 74 additions & 14 deletions lib/src/package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -230,25 +230,38 @@ class Package {
return p.join(root, path);
}

// Maintain set of visited symlinks for every directory.
// If the same symlink is visited twice while moving down the tree,
// then we have faced a loop.
//
// additional complexity:
// N - number of directory symlinks
// memory and time complexity are roughly the same:
// from O(N) (without nested symlinks):
// single set is created for each symlink and never copied
// up to O(N^2) (each symlink is nested in previous one):
// for each symlink clone set with all visited symlinks.
// i-th set contains i symlinks.
final visitedSymlinks = <String, Set<String>>{};

return Ignore.listFiles(
beneath: beneath,
listDir: (dir) {
var contents = Directory(resolve(dir)).listSync();
final resolvedDir = resolve(dir);
_assertSymlinkLoop(dir, resolvedDir, visitedSymlinks);

var contents = Directory(resolvedDir).listSync(followLinks: false);

if (!recursive) {
contents = contents.where((entity) => entity is! Directory).toList();
contents = contents
.where(
(entity) =>
entity is! Directory &&
!(linkExists(entity.path) && dirExists(entity.path)),
)
.toList();
}
return contents.map((entity) {
if (linkExists(entity.path)) {
final target = Link(entity.path).targetSync();
if (dirExists(entity.path)) {
throw DataException(
'''Pub does not support publishing packages with directory symlinks: `${entity.path}`.''');
}
if (!fileExists(entity.path)) {
throw DataException(
'''Pub does not support publishing packages with non-resolving symlink: `${entity.path}` => `$target`.''');
}
}
final relative = p.relative(entity.path, from: root);
if (Platform.isWindows) {
return p.posix.joinAll(p.split(relative));
Expand Down Expand Up @@ -313,7 +326,54 @@ class Package {
);
},
isDir: (dir) => dirExists(resolve(dir)),
).map(resolve).toList();
).map(resolve).map(_assertFileLinksResolvable).toList();
}

static void _assertSymlinkLoop(
String internalDir,
String resolvedDir,
Map<String, Set<String>> visitedSymlinks,
) {
final link = Link(resolvedDir);

var currentSymlinks = visitedSymlinks[p.posix.dirname(internalDir)];
currentSymlinks ??= <String>{};

if (link.existsSync()) {
// copy on write
currentSymlinks = currentSymlinks.toSet();

// "normalize" link path by resolving all links above it.
final resolvedLinkPath = p.join(
link.parent.resolveSymbolicLinksSync(),
p.basename(resolvedDir),
);

if (!currentSymlinks.add(resolvedLinkPath)) {
final link = Link(resolvedDir);
final target = link.targetSync();
throw DataException(
'Pub does not support publishing packages with symlinks loop: '
'`$resolvedDir` => `$target`. '
'Full list of visited symlinks: $currentSymlinks',
);
}
}

visitedSymlinks[internalDir] = currentSymlinks;
}

static String _assertFileLinksResolvable(String path) {
if (!linkExists(path)) {
return path;
}
if (!fileExists(path)) {
final target = Link(path).targetSync();
throw DataException(
'Pub does not support publishing packages with non-resolving symlink: '
'`$path` => `$target`.');
}
return path;
}
}

Expand Down
13 changes: 7 additions & 6 deletions lib/src/validator/gitignore.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@ class GitignoreValidator extends Validator {
final unignoredByGitignore = Ignore.listFiles(
beneath: beneath,
listDir: (dir) {
var contents = Directory(resolve(dir)).listSync();
return contents
.where((e) => !(linkExists(e.path) && dirExists(e.path)))
.map((entity) => p.posix
.joinAll(p.split(p.relative(entity.path, from: root))));
var contents = Directory(resolve(dir)).listSync(followLinks: false);
return contents.map((entity) =>
p.posix.joinAll(p.split(p.relative(entity.path, from: root))));
},
ignoreForDir: (dir) {
final gitIgnore = resolve('$dir/.gitignore');
Expand All @@ -65,7 +63,10 @@ class GitignoreValidator extends Validator {
];
return rules.isEmpty ? null : Ignore(rules);
},
isDir: (dir) => dirExists(resolve(dir)),
isDir: (dir) {
final resolved = resolve(dir);
return dirExists(resolved) && !linkExists(resolved);
},
).map((file) {
final relative = p.relative(resolve(file), from: entrypoint.root.dir);
return Platform.isWindows
Expand Down