don't run into infinite lopps caused by symbolic links. PR 45499.

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@693870 13f79535-47bb-0310-9956-ffa450edef68

src/main/org/apache/tools/ant/DirectoryScanner.java

@@ -27,6 +27,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.Stack;
 import java.util.Vector;
 
 import org.apache.tools.ant.taskdefs.condition.Os;
@@ -37,6 +38,7 @@ import org.apache.tools.ant.types.selectors.FileSelector;
 import org.apache.tools.ant.types.selectors.PathPattern;
 import org.apache.tools.ant.types.selectors.SelectorScanner;
 import org.apache.tools.ant.types.selectors.SelectorUtils;
+import org.apache.tools.ant.util.CollectionUtils;
 import org.apache.tools.ant.util.FileUtils;
 
 /**
@@ -168,6 +170,8 @@ public class DirectoryScanner
         "**/.DS_Store"
     };
 
+    public static final int MAX_LEVELS_OF_SYMLINKS = 1;
+
     /** Helper. */
     private static final FileUtils FILE_UTILS = FileUtils.getFileUtils();
 
@@ -375,6 +379,14 @@ public class DirectoryScanner
      */
     private IllegalStateException illegal = null;
 
+    /**
+     * The maximum number of times a symbolic link may be followed
+     * during a scan.
+     *
+     * @since Ant 1.8.0
+     */
+    private int maxLevelsOfSymlinks = MAX_LEVELS_OF_SYMLINKS;
+
     /**
      * Sole constructor.
      */
@@ -643,6 +655,16 @@ public class DirectoryScanner
         this.followSymlinks = followSymlinks;
     }
 
+    /**
+     * The maximum number of times a symbolic link may be followed
+     * during a scan.
+     *
+     * @since Ant 1.8.0
+     */
+    public void setMaxLevelsOfSymlinks(int max) {
+        maxLevelsOfSymlinks = max;
+    }
+
     /**
      * Set the list of include patterns to use. All '/' and '\' characters
      * are replaced by <code>File.separatorChar</code>, so the separator used
@@ -1086,11 +1108,11 @@ public class DirectoryScanner
                                          + dir.getAbsolutePath() + "'");
             }
         }
-        scandir(dir, vpath, fast, newfiles);
+        scandir(dir, vpath, fast, newfiles, new Stack());
     }
 
     private void scandir(File dir, String vpath, boolean fast,
-                         String[] newfiles) {
+                         String[] newfiles, Stack directoryNamesFollowed) {
         // avoid double scanning of directories, can only happen in fast mode
         if (fast && hasBeenScanned(vpath)) {
             return;
@@ -1116,7 +1138,10 @@ public class DirectoryScanner
                 }
             }
             newfiles = (String[]) (noLinks.toArray(new String[noLinks.size()]));
+        } else {
+            directoryNamesFollowed.push(dir.getName());
         }
+
         for (int i = 0; i < newfiles.length; i++) {
             String name = vpath + newfiles[i];
             File file = new File(dir, newfiles[i]);
@@ -1129,20 +1154,39 @@ public class DirectoryScanner
                     filesNotIncluded.addElement(name);
                 }
             } else { // dir
+
+                if (followSymlinks
+                    && causesIllegalSymlinkLoop(newfiles[i], dir,
+                                                directoryNamesFollowed)) {
+                    // will be caught and redirected to Ant's logging system
+                    System.err.println("skipping symbolic link "
+                                       + file.getAbsolutePath()
+                                       + " -- too many levels of symbolic"
+                                       + " links.");
+                    continue;
+                }
+
                 if (isIncluded(name)) {
-                    accountForIncludedDir(name, file, fast, children);
+                    accountForIncludedDir(name, file, fast, children,
+                                          directoryNamesFollowed);
                 } else {
                     everythingIncluded = false;
                     dirsNotIncluded.addElement(name);
                     if (fast && couldHoldIncluded(name)) {
-                        scandir(file, name + File.separator, fast, children);
+                        scandir(file, name + File.separator, fast, children,
+                                directoryNamesFollowed);
                     }
                 }
                 if (!fast) {
-                    scandir(file, name + File.separator, fast, children);
+                    scandir(file, name + File.separator, fast, children,
+                            directoryNamesFollowed);
                 }
             }
         }
+
+        if (followSymlinks) {
+            directoryNamesFollowed.pop();
+        }
     }
 
     /**
@@ -1170,10 +1214,12 @@ public class DirectoryScanner
     }
 
     private void accountForIncludedDir(String name, File file, boolean fast,
-                                       String[] children) {
+                                       String[] children,
+                                       Stack directoryNamesFollowed) {
         processIncluded(name, file, dirsIncluded, dirsExcluded, dirsDeselected);
         if (fast && couldHoldIncluded(name) && !contentsExcluded(name)) {
-            scandir(file, name + File.separator, fast, children);
+            scandir(file, name + File.separator, fast, children,
+                    directoryNamesFollowed);
         }
     }
 
@@ -1744,4 +1790,50 @@ public class DirectoryScanner
         return (PathPattern[]) al.toArray(new PathPattern[al.size()]);
     }
 
+    /**
+     * Would following the given directory cause a loop of symbolic
+     * links deeper than allowed?
+     *
+     * <p>Can only happen if the given directory has been seen at
+     * least more often than allowed during the current scan and it is
+     * a symbolic link and enough other occurences of the same name
+     * higher up are symbolic links that point to the same place.</p>
+     *
+     * @since Ant 1.8.0
+     */
+    private boolean causesIllegalSymlinkLoop(String dirName, File parent,
+                                             Stack directoryNamesFollowed) {
+        try {
+            if (CollectionUtils.frequency(directoryNamesFollowed, dirName)
+                   >= maxLevelsOfSymlinks
+                && FILE_UTILS.isSymbolicLink(parent, dirName)) {
+
+                Stack s = (Stack) directoryNamesFollowed.clone();
+                ArrayList files = new ArrayList();
+                File f = FILE_UTILS.resolveFile(parent, dirName);
+                String target = f.getCanonicalPath();
+                files.add(target);
+
+                String relPath = "";
+                while (!s.empty()) {
+                    relPath += "../";
+                    String dir = (String) s.pop();
+                    if (dirName.equals(dir)) {
+                        f = FILE_UTILS.resolveFile(parent, relPath + dir);
+                        files.add(f.getCanonicalPath());
+                        if (CollectionUtils.frequency(files, target)
+                            > maxLevelsOfSymlinks) {
+                            return true;
+                        }
+                    }
+                }
+
+            }
+            return false;
+        } catch (IOException ex) {
+            throw new BuildException("Caught error while checking for"
+                                     + " symbolic links", ex);
+        }
+    }
+
 }

src/main/org/apache/tools/ant/types/AbstractFileSet.java

@@ -68,6 +68,7 @@ public abstract class AbstractFileSet extends DataType
     private boolean caseSensitive = true;
     private boolean followSymlinks = true;
     private boolean errorOnMissingDir = true;
+    private int maxLevelsOfSymlinks = DirectoryScanner.MAX_LEVELS_OF_SYMLINKS;
 
     /* cached DirectoryScanner instance for our own Project only */
     private DirectoryScanner directoryScanner = null;
@@ -93,6 +94,7 @@ public abstract class AbstractFileSet extends DataType
         this.caseSensitive = fileset.caseSensitive;
         this.followSymlinks = fileset.followSymlinks;
         this.errorOnMissingDir = fileset.errorOnMissingDir;
+        this.maxLevelsOfSymlinks = fileset.maxLevelsOfSymlinks;
         setProject(fileset.getProject());
     }
 
@@ -396,6 +398,16 @@ public abstract class AbstractFileSet extends DataType
             ? getRef(getProject()).isFollowSymlinks() : followSymlinks;
     }
 
+    /**
+     * The maximum number of times a symbolic link may be followed
+     * during a scan.
+     *
+     * @since Ant 1.8.0
+     */
+    public void setMaxLevelsOfSymlinks(int max) {
+        maxLevelsOfSymlinks = max;
+    }
+
     /**
      * Sets whether an error is thrown if a directory does not exist.
      *
@@ -444,6 +456,7 @@ public abstract class AbstractFileSet extends DataType
                 setupDirectoryScanner(ds, p);
                 ds.setFollowSymlinks(followSymlinks);
                 ds.setErrorOnMissingDir(errorOnMissingDir);
+                ds.setMaxLevelsOfSymlinks(maxLevelsOfSymlinks);
                 directoryScanner = (p == getProject()) ? ds : directoryScanner;
             }
         }

src/main/org/apache/tools/ant/util/CollectionUtils.java

@@ -222,4 +222,24 @@ public class CollectionUtils {
 
     }
 
+    /**
+     * Counts how often the given Object occurs in the given
+     * collection using equals() for comparison.
+     *
+     * @since Ant 1.8.0
+     */
+    public static int frequency(Collection c, Object o) {
+        // same as Collections.frequency introduced with JDK 1.5
+        int freq = 0;
+        if (c != null) {
+            for (Iterator i = c.iterator(); i.hasNext(); ) {
+                Object test = i.next();
+                if (o == null ? test == null : o.equals(test)) {
+                    freq++;
+                }
+            }
+        }
+        return freq;
+    }
+            
 }

src/tests/antunit/core/dirscanner-symlinks-test.xml

@@ -75,14 +75,26 @@
     <au:assertFileDoesntExist file="${output}/file.txt"/>
   </target>
 
-  <target name="INFINITEtestLinkToParentFollow"
+  <target name="testLinkToParentFollow"
           depends="checkOs, setUp, -link-to-parent"
           if="unix">
     <copy todir="${output}">
-      <fileset dir="${base}" followsymlinks="true"/>
+      <fileset dir="${base}" followsymlinks="true" maxLevelsOfSymlinks="1"/>
+    </copy>
+    <symlink action="delete" link="${base}/A"/>
+    <au:assertFileExists file="${output}/A/B/file.txt"/>
+    <au:assertFileDoesntExist file="${output}/A/base/A/B/file.txt"/>
+  </target>
+
+  <target name="testLinkToParentFollowMax2"
+          depends="checkOs, setUp, -link-to-parent"
+          if="unix">
+    <copy todir="${output}">
+      <fileset dir="${base}" followsymlinks="true" maxLevelsOfSymlinks="2"/>
     </copy>
     <symlink action="delete" link="${base}/A"/>
     <au:assertFileExists file="${output}/A/B/file.txt"/>
+    <au:assertFileExists file="${output}/A/base/A/B/file.txt"/>
   </target>
 
   <target name="testLinkToParentFollowWithInclude"
@@ -120,7 +132,7 @@
     <au:assertFileDoesntExist file="${output}/A/B/file.txt"/>
   </target>
 
-  <target name="INFINITE testSillyLoopFollow"
+  <target name="testSillyLoopFollow"
           depends="checkOs, setUp, -silly-loop"
           if="unix">
     <copy todir="${output}">
@@ -140,6 +152,28 @@
     <au:assertFileDoesntExist file="${output}"/>
   </target>
 
+  <target name="testRepeatedName"
+          depends="setUp">
+    <mkdir dir="${base}/A/A/A/A"/>
+    <touch file="${base}/A/A/A/A/file.txt"/>
+    <copy todir="${output}">
+      <fileset dir="${base}" followsymlinks="true" maxLevelsOfSymlinks="1"/>
+    </copy>
+    <au:assertFileExists file="${output}/A/A/A/A/file.txt"/>
+  </target>
+
+  <target name="testRepeatedNameWithLinkButNoLoop"
+          depends="checkOs, setUp"
+          if="unix">
+    <mkdir dir="${base}/A/A/A/B"/>
+    <touch file="${base}/A/A/A/B/file.txt"/>
+    <symlink link="${base}/A/A/A/A" resource="${base}/A/A/A/B"/>
+    <copy todir="${output}">
+      <fileset dir="${base}" followsymlinks="true" maxLevelsOfSymlinks="1"/>
+    </copy>
+    <au:assertFileExists file="${output}/A/A/A/A/file.txt"/>
+  </target>
+
   <target name="-sibling" if="unix">
     <mkdir dir="${base}/A"/>
     <touch file="${base}/A/file.txt"/>