Skip to content

UnifiedTree.isRecursiveLink() links check may silently ignore some links #2221

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 5, 2025
Merged

UnifiedTree.isRecursiveLink() links check may silently ignore some links #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 47 additions & 15 deletions ...dles/org.eclipse.core.resources/src/org/eclipse/core/internal/localstore/UnifiedTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,6 @@ private static class PatternHolder {
//Pattern: A UNIX or Windows relative path that just points backward
static final Pattern TRIVIAL_SYMLINK_PATTERN = Pattern.compile( //
Platform.OS.isWindows() ? "\\.[.\\\\]*" : "\\.[./]*"); //$NON-NLS-1$//$NON-NLS-2$

static final Pattern REPEATING_BACKWARDS_PATTERN = Pattern.compile( //
Platform.OS.isWindows() ? "(\\.\\.\\\\)+.*" : "(\\.\\./)+.*"); //$NON-NLS-1$//$NON-NLS-2$
}

/**
Expand Down Expand Up @@ -522,20 +519,10 @@ private boolean isRecursiveLink(IFileStore parentStore, IFileInfo localInfo) {
Path realParentPath = parent.toRealPath();
if (disable_advanced_recursive_link_checks) {
// Multiple ../ backwards links can go outside the project tree
if (linkTarget != null && PatternHolder.REPEATING_BACKWARDS_PATTERN.matcher(linkTarget).matches()) {
Path targetPath = parent.resolve(linkTarget).normalize();

// Recursive if literal target points to the literal parent of this tree
if (parent.normalize().startsWith(targetPath)) {
if (linkTarget != null) {
if (isRecursiveBackwardsLink(realParentPath, linkTarget)) {
return true;
}

// Recursive if resolved target points to the resolved parent of this tree
Path realTargetPath = targetPath.toRealPath();
if (realParentPath.startsWith(realTargetPath)) {
return true;
}

// If link is outside the project tree, consider as non recursive
// The link still can create recursion in the tree, but we can't detect it here.
}
Expand Down Expand Up @@ -573,6 +560,51 @@ private boolean isRecursiveLink(IFileStore parentStore, IFileInfo localInfo) {
return false;
}

/**
* @param realParentPath real parent path object obtained as a result
* of @code{Path.toRealPath()}
* @param linkTarget the link target path as a string, may be relative or
* absolute
* @return true if the given target points backwards recursively to the given
* parent path
* @throws IOException
*/
private static boolean isRecursiveBackwardsLink(Path realParentPath, String linkTarget)
throws IOException {
// Cheap test first: literal target points to the literal parent
Path normalizedLink = realParentPath.resolve(linkTarget).normalize();
if (realParentPath.startsWith(normalizedLink)) {
return true;
}
// Next check costs more time because it does real IO when resolving paths
Path realTarget = normalizedLink.toRealPath();
if (realParentPath.startsWith(realTarget)) {
return true;
}
return false;
}

/**
* Disable or enable advanced recursive link checks. Advanced link checks may
* hide valid directories in some cases, see bug 537449.
*
* @param enable <code>true</code> to enable advanced recursive link checks,
* <code>false</code> to disable them.
*/
public static void enableAdvancedRecursiveLinkChecks(boolean enable) {
disable_advanced_recursive_link_checks = !enable;
}

/**
* Returns whether advanced recursive link checks are enabled.
*
* @return <code>true</code> if advanced recursive link checks are enabled,
* <code>false</code> otherwise.
*/
public static boolean isAdvancedRecursiveLinkChecksEnabled() {
return !disable_advanced_recursive_link_checks;
}

protected boolean isValidLevel(int currentLevel, int depth) {
return switch (depth) {
case IResource.DEPTH_INFINITE -> true;
Expand Down
175 changes: 134 additions & 41 deletions ...e.tests.resources/src/org/eclipse/core/tests/internal/localstore/SymlinkResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,33 @@
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createTestMonitor;
import static org.eclipse.core.tests.resources.ResourceTestUtil.waitForRefresh;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.filesystem.IFileStore;
import org.eclipse.core.internal.localstore.UnifiedTree;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.IResourceVisitor;
import org.eclipse.core.resources.IWorkspaceRunnable;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.tests.resources.WorkspaceTestRule;
import org.junit.Rule;
import org.junit.Test;

import org.eclipse.core.runtime.Platform.OS;
import org.eclipse.core.tests.resources.util.WorkspaceResetExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

@ExtendWith(WorkspaceResetExtension.class)
public class SymlinkResourceTest {

@Rule
public WorkspaceTestRule workspaceRule = new WorkspaceTestRule();

private void mkLink(IFileStore dir, String src, String tgt, boolean isDir) throws CoreException, IOException {
createSymLink(dir.toLocalFile(EFS.NONE, createTestMonitor()), src, tgt, isDir);
}
Expand Down Expand Up @@ -70,6 +75,88 @@ protected void createBug358830Structure(IFileStore rootDir) throws CoreException
mkLink(folderA, "link", IPath.fromOSString("../").toOSString(), true);
}

/**
* Test a case of both recursive and non recursive symbolic links that uncovered
* issue described in <a href=
* "https://github.com/eclipse-platform/eclipse.platform/issues/2220">GitHub bug
* 2220</a>.
*
* <pre>{@code
* /A/B -> /X/Y/Z (B is symbolic link as precondition)
* /A/B/C/D -> ../../D (non-recursive)
* /A/B/C/E -> ../../Z (recursive)
* }</pre>
*
* The starting path /A/B/C is already based on link. The real path of start is
* /X/Y/Z/C so ../../Z points (recursive) to /X/Y/Z. /X/Y/D and /X/Y/Z is what
* we expect to resolve but we fail in both cases with NoSuchFileException.
*/
@ParameterizedTest
@ValueSource(booleans = { false, true })
public void testGithubBug2220(boolean useAdvancedLinkCheck) throws Exception {
assumeTrue(canCreateSymLinks(), "only relevant for platforms supporting symbolic links");
assumeTrue(!OS.isWindows(), "Windows file system handles recursive links differently");
final boolean originalValue = UnifiedTree.isAdvancedRecursiveLinkChecksEnabled();
try {
UnifiedTree.enableAdvancedRecursiveLinkChecks(useAdvancedLinkCheck);
IProject project = getWorkspace().getRoot().getProject("testGithubBug2220");
createInWorkspace(project);

/* Re-use projects which are cleaned up automatically */
getWorkspace().run((IWorkspaceRunnable) monitor -> {
/* delete open project because we must re-open with BACKGROUND_REFRESH */
project.delete(IResource.NEVER_DELETE_PROJECT_CONTENT, createTestMonitor());
project.create(null);
try {
createGithubBug2220Structure(EFS.getStore(project.getLocationURI()));
} catch (IOException e) {
throw new IllegalStateException("unexpected IOException occurred", e);
}
// Bug only happens with BACKGROUND_REFRESH.
project.open(IResource.BACKGROUND_REFRESH, createTestMonitor());
}, null);

// wait for BACKGROUND_REFRESH to complete.
waitForRefresh();
project.accept(new IResourceVisitor() {
int resourceCount = 0;

@Override
public boolean visit(IResource resource) {
resourceCount++;
// We have 1 root + .settings + prefs + + .project + 10 elements --> 14 elements
// to visit at most
System.out.println(resourceCount + " visited: " + resource.getFullPath());
assertTrue(resourceCount <= 15, "Expected max 15 elements to visit, got: " + resourceCount);
return true;
}
});
} finally {
UnifiedTree.enableAdvancedRecursiveLinkChecks(originalValue);
}
}

/**
* <pre>{@code
* /A/B -> /X/Y/Z (B is symbolic link as precondition)
* /A/B/C/D -> ../../D (non-recursive)
* /A/B/C/E -> ../../Z (recursive)
* }</pre>
*
* The starting path /A/B/C is already based on link. The real path of start is
* /X/Y/Z/C so ../../Z points (recursive) to /X/Y/Z.
*/
protected void createGithubBug2220Structure(IFileStore rootDir) throws CoreException, IOException {
Path root = rootDir.toLocalFile(EFS.NONE, createTestMonitor()).toPath();
Files.createDirectories(root.resolve("A"));
Files.createDirectories(root.resolve("X/Y/Z"));
Files.createDirectories(root.resolve("X/Y/D"));
Files.createSymbolicLink(root.resolve("A/B"), root.resolve("X/Y/Z"));
Files.createDirectories(root.resolve("A/B/C"));
Files.createSymbolicLink(root.resolve("A/B/C/D"), Paths.get("../../D"));
Files.createSymbolicLink(root.resolve("A/B/C/E"), Paths.get("../../Z"));
}

/**
* Test a very specific case of mutually recursive symbolic links:
* <pre> {@code
Expand All @@ -85,7 +172,7 @@ protected void createBug358830Structure(IFileStore rootDir) throws CoreException
*/
@Test
public void testBug232426() throws Exception {
assumeTrue("only relevant for platforms supporting symbolic links", canCreateSymLinks());
assumeTrue(canCreateSymLinks(), "only relevant for platforms supporting symbolic links");

IProject project = getWorkspace().getRoot().getProject("Project");
createInWorkspace(project);
Expand Down Expand Up @@ -118,36 +205,42 @@ public boolean visit(IResource resource) {
});
}

@Test
public void testBug358830() throws Exception {
assumeTrue("only relevant for platforms supporting symbolic links", canCreateSymLinks());

IProject project = getWorkspace().getRoot().getProject("Project");
createInWorkspace(project);
/* Re-use projects which are cleaned up automatically */
getWorkspace().run((IWorkspaceRunnable) monitor -> {
/* delete open project because we must re-open with BACKGROUND_REFRESH */
project.delete(IResource.NEVER_DELETE_PROJECT_CONTENT, createTestMonitor());
project.create(null);
try {
createBug358830Structure(EFS.getStore(project.getLocationURI()));
} catch (IOException e) {
throw new IllegalStateException("unexpected IOException occurred", e);
}
project.open(IResource.BACKGROUND_REFRESH, createTestMonitor());
}, null);

//wait for BACKGROUND_REFRESH to complete.
waitForRefresh();
final int resourceCount[] = new int[] {0};
project.accept(resource -> {
resourceCount[0]++;
return true;
});
// We have 1 root + 1 folder + 1 file (.project)
// + .settings / resources prefs
// --> 5 elements to visit
assertEquals(5, resourceCount[0]);
@ParameterizedTest
@ValueSource(booleans = { false, true })
public void testBug358830(boolean useAdvancedLinkCheck) throws Exception {
assumeTrue(canCreateSymLinks(), "only relevant for platforms supporting symbolic links");
final boolean originalValue = UnifiedTree.isAdvancedRecursiveLinkChecksEnabled();
try {
UnifiedTree.enableAdvancedRecursiveLinkChecks(useAdvancedLinkCheck);
IProject project = getWorkspace().getRoot().getProject("Project");
createInWorkspace(project);
/* Re-use projects which are cleaned up automatically */
getWorkspace().run((IWorkspaceRunnable) monitor -> {
/* delete open project because we must re-open with BACKGROUND_REFRESH */
project.delete(IResource.NEVER_DELETE_PROJECT_CONTENT, createTestMonitor());
project.create(null);
try {
createBug358830Structure(EFS.getStore(project.getLocationURI()));
} catch (IOException e) {
throw new IllegalStateException("unexpected IOException occurred", e);
}
project.open(IResource.BACKGROUND_REFRESH, createTestMonitor());
}, null);

// wait for BACKGROUND_REFRESH to complete.
waitForRefresh();
final int resourceCount[] = new int[] { 0 };
project.accept(resource -> {
resourceCount[0]++;
return true;
});
// We have 1 root + 1 folder + 1 file (.project)
// + .settings / resources prefs
// --> 5 elements to visit
assertEquals(5, resourceCount[0]);
} finally {
UnifiedTree.enableAdvancedRecursiveLinkChecks(originalValue);
}
}

}
Loading
Loading