Author: bodewig Date: Wed Sep 10 08:41:49 2008 New Revision: 693870 URL: http://svn.apache.org/viewvc?rev=693870&view=rev Log: don't run into infinite lopps caused by symbolic links. PR 45499.
Modified: ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java ant/core/trunk/src/main/org/apache/tools/ant/util/CollectionUtils.java ant/core/trunk/src/tests/antunit/core/dirscanner-symlinks-test.xml Modified: ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java?rev=693870&r1=693869&r2=693870&view=diff ============================================================================== --- ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java (original) +++ ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java Wed Sep 10 08:41:49 2008 @@ -27,6 +27,7 @@ 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.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 @@ "**/.DS_Store" }; + public static final int MAX_LEVELS_OF_SYMLINKS = 1; + /** Helper. */ private static final FileUtils FILE_UTILS = FileUtils.getFileUtils(); @@ -376,6 +380,14 @@ 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. */ public DirectoryScanner() { @@ -644,6 +656,16 @@ } /** + * 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 * need not match <code>File.separatorChar</code>. @@ -1086,11 +1108,11 @@ + 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 @@ } } 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 @@ 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 @@ } 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 @@ 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); + } + } + } Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java?rev=693870&r1=693869&r2=693870&view=diff ============================================================================== --- ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java (original) +++ ant/core/trunk/src/main/org/apache/tools/ant/types/AbstractFileSet.java Wed Sep 10 08:41:49 2008 @@ -68,6 +68,7 @@ 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 @@ this.caseSensitive = fileset.caseSensitive; this.followSymlinks = fileset.followSymlinks; this.errorOnMissingDir = fileset.errorOnMissingDir; + this.maxLevelsOfSymlinks = fileset.maxLevelsOfSymlinks; setProject(fileset.getProject()); } @@ -397,6 +399,16 @@ } /** + * 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. * * @param errorOnMissingDir true if missing directories cause errors, @@ -444,6 +456,7 @@ setupDirectoryScanner(ds, p); ds.setFollowSymlinks(followSymlinks); ds.setErrorOnMissingDir(errorOnMissingDir); + ds.setMaxLevelsOfSymlinks(maxLevelsOfSymlinks); directoryScanner = (p == getProject()) ? ds : directoryScanner; } } Modified: ant/core/trunk/src/main/org/apache/tools/ant/util/CollectionUtils.java URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/util/CollectionUtils.java?rev=693870&r1=693869&r2=693870&view=diff ============================================================================== --- ant/core/trunk/src/main/org/apache/tools/ant/util/CollectionUtils.java (original) +++ ant/core/trunk/src/main/org/apache/tools/ant/util/CollectionUtils.java Wed Sep 10 08:41:49 2008 @@ -222,4 +222,24 @@ } + /** + * 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; + } + } Modified: ant/core/trunk/src/tests/antunit/core/dirscanner-symlinks-test.xml URL: http://svn.apache.org/viewvc/ant/core/trunk/src/tests/antunit/core/dirscanner-symlinks-test.xml?rev=693870&r1=693869&r2=693870&view=diff ============================================================================== --- ant/core/trunk/src/tests/antunit/core/dirscanner-symlinks-test.xml (original) +++ ant/core/trunk/src/tests/antunit/core/dirscanner-symlinks-test.xml Wed Sep 10 08:41:49 2008 @@ -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"/>