Additional symlink loop check for disable_advanced_recursive_link_checks

If the (non default) system property
`-Dorg.eclipse.core.resources.disable_advanced_recursive_link_checks"`
is set, expensive advanced symlink checks are disabled to avoid
performance / OOM issues. This is a known tradeoff between correctness
and performance.

This commit adds one more "symlink loop" use case detected by the "non
advanced" recursive symlink checks code path.

Author: iloveeclipse

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/localstore/UnifiedTree.java

@@ -520,7 +520,7 @@ private boolean isRecursiveLink(IFileStore parentStore, IFileInfo localInfo) {
 			if (disable_advanced_recursive_link_checks) {
 				// Multiple ../ backwards links can go outside the project tree
 				if (linkTarget != null) {
-					if (isRecursiveBackwardsLink(realParentPath, linkTarget)) {
+					if (isRecursiveBackwardsLink(parent, realParentPath, linkTarget)) {
 						return true;
 					}
 					// If link is outside the project tree, consider as non recursive
@@ -561,20 +561,24 @@ private boolean isRecursiveLink(IFileStore parentStore, IFileInfo localInfo) {
 	}
 
 	/**
-	 * @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
+	 * @param parent         the parent path object (may not be a real path), coming
+	 *                       from {@link #isRecursiveLink(IFileStore, IFileInfo)}
+	 * @param realParentPath real parent path object obtained as a result of
+	 *                       <code>Path.toRealPath()</code>
+	 * @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)
+	private static boolean isRecursiveBackwardsLink(Path parent, 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;
+		} else if (parent.startsWith(normalizedLink)) {
+			return true;
 		}
 		// Next check costs more time because it does real IO when resolving paths
 		Path realTarget = normalizedLink.toRealPath();

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/localstore/SymlinkResourceTest.java

@@ -173,6 +173,7 @@ protected void createGithubBug2220Structure(IFileStore rootDir) throws CoreExcep
 	@Test
 	public void testBug232426() throws Exception {
 		assumeTrue(canCreateSymLinks(), "only relevant for platforms supporting symbolic links");
+		assumeTrue(UnifiedTree.isAdvancedRecursiveLinkChecksEnabled(), "Will hang the simple implementation");
 
 		IProject project = getWorkspace().getRoot().getProject("Project");
 		createInWorkspace(project);
@@ -199,7 +200,7 @@ public void testBug232426() throws Exception {
 			public boolean visit(IResource resource) {
 				resourceCount++;
 				// We have 1 root + 4 folders + 5 elements --> 10 elements to visit at most
-				assertTrue(resourceCount <= 10);
+				assertTrue(resourceCount <= 10, "Expected max 10 elements to visit, got: " + resourceCount);
 				return true;
 			}
 		});

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_185247_recursiveLinks.java

@@ -27,6 +27,7 @@
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IProjectDescription;
 import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.IResourceVisitor;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.tests.resources.util.WorkspaceResetExtension;
@@ -184,7 +185,76 @@ public void test5_linkParentDirectoyTwiceWithAbsolutePath(boolean useAdvancedLin
 		runTest(createSymlinks, useAdvancedLinkCheck);
 	}
 
-	private void runTest(CreateTestProjectStructure createSymlinks, boolean useAdvancedLinkCheck)
+	/**
+	 * Test project structure:
+	 *
+	 * <pre>
+	 * project root
+	 *   |-- directory
+	 *         |
+	 *         |- directory_1/
+	 *         |    |
+	 *         |    |- directory_1_1/
+	 *         |         |
+	 *         |         |- directory_1_1_1/
+	 *         |              |
+	 *         |              |- link_directory_2 -> ../../../directory_2/
+	 *         |
+	 *         |- directory_2/
+	 *              |
+	 *              |- directory_2_2/
+	 *                   |
+	 *                   |- link_directory_1 -> ../../directory_1/
+	 * </pre>
+	 */
+	@ParameterizedTest
+	@ValueSource(booleans = { false, true })
+	public void test6_linkGrandparentTwice(boolean useAdvancedLinkCheck) throws Exception {
+		CreateTestProjectStructure createSymlinks = directory -> {
+
+			// directory_1/directory_1_1/directory_1_1_1/
+			File directory1 = new File(directory, "directory_1");
+			File directory1_1 = new File(directory1, "directory_1_1");
+			File directory1_1_1 = new File(directory1_1, "directory_1_1_1");
+			createDirectory(directory1_1_1);
+
+			// directory_2/directory_2_2/
+			File directory2 = new File(directory, "directory_2");
+			File directory2_2 = new File(directory2, "directory_2_2");
+			createDirectory(directory2_2);
+
+			// link_directory_2 -> ../../../directory_2/ (from directory_1_1_1, goes up to
+			// root directory)
+			createSymlink(directory1_1_1, "link_directory_2", "../../../directory_2/");
+
+			// link_directory_1 -> ../../directory_1/ (from directory_2_2, goes up to
+			// root directory)
+			createSymlink(directory2_2, "link_directory_1", "../../directory_1/");
+		};
+
+		IProject project = runTest(createSymlinks, useAdvancedLinkCheck);
+		final boolean originalValue = UnifiedTree.isAdvancedRecursiveLinkChecksEnabled();
+		UnifiedTree.enableAdvancedRecursiveLinkChecks(useAdvancedLinkCheck);
+		try {
+			// visiting the project should not visit forever
+			project.accept(new IResourceVisitor() {
+				int resourceCount = 0;
+
+				@Override
+				public boolean visit(IResource resource) {
+					resourceCount++;
+					System.out.println(resourceCount + " visited: " + resource.getFullPath());
+					assertTrue(resourceCount <= 13, "Expected max 13 elements to visit, got: " + resourceCount);
+					return true;
+				}
+			});
+		} finally {
+			UnifiedTree.enableAdvancedRecursiveLinkChecks(originalValue);
+		}
+
+	}
+
+	private IProject runTest(CreateTestProjectStructure createSymlinks, boolean useAdvancedLinkCheck)
 			throws MalformedURLException, Exception {
 		final boolean originalValue = UnifiedTree.isAdvancedRecursiveLinkChecksEnabled();
 		try {
@@ -199,7 +269,8 @@ private void runTest(CreateTestProjectStructure createSymlinks, boolean useAdvan
 
 			URI projectRootLocation = URIUtil.toURI((projectRoot));
 			// refreshing the project with recursive symlinks should not hang
-			importProjectAndRefresh(projectName, projectRootLocation);
+			IProject project = importProjectAndRefresh(projectName, projectRootLocation);
+			return project;
 		} finally {
 			UnifiedTree.enableAdvancedRecursiveLinkChecks(originalValue);
 		}
@@ -215,9 +286,10 @@ void createSymlink(File directory, String linkName, String linkTarget) throws IO
 		createSymLink(directory, linkName, linkTarget, isDir);
 	}
 
-	private void importProjectAndRefresh(String projectName, URI projectRootLocation) throws Exception {
+	private IProject importProjectAndRefresh(String projectName, URI projectRootLocation) throws Exception {
 		IProject project = importTestProject(projectName, projectRootLocation);
 		project.refreshLocal(IResource.DEPTH_INFINITE, createTestMonitor());
+		return project;
 	}
 
 	private IProject importTestProject(String projectName, URI projectRootLocation) throws Exception {