Repository: ant Updated Branches: refs/heads/master 7ccb90371 -> e18de97c4
BZ-60644 Skip copying files, to avoid corrupting source files, if the source file and dest file are symlinked to each other Project: http://git-wip-us.apache.org/repos/asf/ant/repo Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/07c54f0b Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/07c54f0b Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/07c54f0b Branch: refs/heads/master Commit: 07c54f0bb1ee4a273798552f4df9148e96313c21 Parents: 01613e0 Author: Jaikiran Pai <jaikiran....@gmail.com> Authored: Sun Jul 23 19:51:30 2017 +0530 Committer: Jaikiran Pai <jaikiran....@gmail.com> Committed: Thu Sep 28 17:11:33 2017 +0530 ---------------------------------------------------------------------- src/etc/testcases/taskdefs/copy.xml | 27 ++++++++ .../apache/tools/ant/util/ResourceUtils.java | 57 +++++++++++++++- .../org/apache/tools/ant/taskdefs/CopyTest.java | 71 ++++++++++++++++++-- 3 files changed, 147 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ant/blob/07c54f0b/src/etc/testcases/taskdefs/copy.xml ---------------------------------------------------------------------- diff --git a/src/etc/testcases/taskdefs/copy.xml b/src/etc/testcases/taskdefs/copy.xml index bf4441c..2601556 100644 --- a/src/etc/testcases/taskdefs/copy.xml +++ b/src/etc/testcases/taskdefs/copy.xml @@ -265,4 +265,31 @@ a=b= </fail> </target> + <target name="setupSelfCopyTesting" description="Sets up the necessary files and directories for testSelfCopy task which + will be called by the test cases"> + <property name="self.copy.test.root.dir" location="${output}/self-copy-test"/> + <mkdir dir="${self.copy.test.root.dir}"/> + <echo file="${self.copy.test.root.dir}/file.txt" message="hello-world"/> + </target> + + <target name="testSelfCopy"> + <property name="self.copy.test.root.dir" location="${output}/self-copy-test"/> + <!-- straightforward self-copy --> + <copy file="${self.copy.test.root.dir}/file.txt" tofile="${self.copy.test.root.dir}/file.txt" overwrite="true"/> + + <!-- create a symlink of the source file and then attempt a copy of the original file and the symlink --> + <symlink link="${self.copy.test.root.dir}/sylmink-file.txt" resource="${self.copy.test.root.dir}/file.txt"/> + <copy file="${self.copy.test.root.dir}/file.txt" tofile="${self.copy.test.root.dir}/sylmink-file.txt" overwrite="true"/> + <copy file="${self.copy.test.root.dir}/sylmink-file.txt" tofile="${self.copy.test.root.dir}/file.txt" overwrite="true"/> + + <!-- create a symlink of the dir containing the source file and then attempt a copy of the original file and the symlink dir --> + <property name="self.copy.test.symlinked.dir" location="${output}/symlink-for-output-dir"/> + <symlink link="${self.copy.test.symlinked.dir}" resource="${self.copy.test.root.dir}"/> + <copy file="${self.copy.test.symlinked.dir}/file.txt" tofile="${self.copy.test.root.dir}/sylmink-file.txt" overwrite="true"/> + <copy file="${self.copy.test.root.dir}/sylmink-file.txt" tofile="${self.copy.test.symlinked.dir}/file.txt" overwrite="true"/> + + <copy todir="${self.copy.test.symlinked.dir}" overwrite="true"> + <fileset dir="${self.copy.test.root.dir}"/> + </copy> + </target> </project> http://git-wip-us.apache.org/repos/asf/ant/blob/07c54f0b/src/main/org/apache/tools/ant/util/ResourceUtils.java ---------------------------------------------------------------------- diff --git a/src/main/org/apache/tools/ant/util/ResourceUtils.java b/src/main/org/apache/tools/ant/util/ResourceUtils.java index 518ed5a..0c319ad 100644 --- a/src/main/org/apache/tools/ant/util/ResourceUtils.java +++ b/src/main/org/apache/tools/ant/util/ResourceUtils.java @@ -432,7 +432,7 @@ public class ResourceUtils { final File sourceFile = source.as(FileProvider.class).getFile(); try { - copyUsingFileChannels(sourceFile, destFile); + copyUsingFileChannels(sourceFile, destFile, project); copied = true; } catch (final IOException ex) { String msg = "Attempt to copy " + sourceFile @@ -666,6 +666,13 @@ public class ResourceUtils { final String outputEncoding, final Project project) throws IOException { + + if (areSame(source, dest)) { + // copying the "same" file to itself will corrupt the file, so we skip it + log(project, "Skipping (self) copy of " + source + " to " + dest); + return; + } + BufferedReader in = null; BufferedWriter out = null; try { @@ -724,6 +731,13 @@ public class ResourceUtils { final String outputEncoding, final Project project) throws IOException { + + if (areSame(source, dest)) { + // copying the "same" file to itself will corrupt the file, so we skip it + log(project, "Skipping (self) copy of " + source + " to " + dest); + return; + } + BufferedReader in = null; BufferedWriter out = null; try { @@ -767,9 +781,14 @@ public class ResourceUtils { } private static void copyUsingFileChannels(final File sourceFile, - final File destFile) + final File destFile, final Project project) throws IOException { + if (FileUtils.getFileUtils().areSame(sourceFile, destFile)) { + // copying the "same" file to itself will corrupt the file, so we skip it + log(project, "Skipping (self) copy of " + sourceFile + " to " + destFile); + return; + } final File parent = destFile.getParentFile(); if (parent != null && !parent.isDirectory() && !(parent.mkdirs() || parent.isDirectory())) { @@ -807,6 +826,13 @@ public class ResourceUtils { private static void copyUsingStreams(final Resource source, final Resource dest, final boolean append, final Project project) throws IOException { + + if (areSame(source, dest)) { + // copying the "same" file to itself will corrupt the file, so we skip it + log(project, "Skipping (self) copy of " + source + " to " + dest); + return; + } + InputStream in = null; OutputStream out = null; try { @@ -843,6 +869,33 @@ public class ResourceUtils { return resource.getOutputStream(); } + private static boolean areSame(final Resource resource1, final Resource resource2) throws IOException { + if (resource1 == null || resource2 == null) { + return false; + } + final FileProvider fileResource1 = resource1.as(FileProvider.class); + if (fileResource1 == null) { + return false; + } + final FileProvider fileResource2 = resource2.as(FileProvider.class); + if (fileResource2 == null) { + return false; + } + return FileUtils.getFileUtils().areSame(fileResource1.getFile(), fileResource2.getFile()); + } + + private static void log(final Project project, final String message) { + log(project, message, Project.MSG_VERBOSE); + } + + private static void log(final Project project, final String message, final int level) { + if (project == null) { + System.out.println(message); + } else { + project.log(message, level); + } + } + public interface ResourceSelectorProvider { ResourceSelector getTargetSelectorForSource(Resource source); } http://git-wip-us.apache.org/repos/asf/ant/blob/07c54f0b/src/tests/junit/org/apache/tools/ant/taskdefs/CopyTest.java ---------------------------------------------------------------------- diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/CopyTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/CopyTest.java index a7a32a9..719bc44 100644 --- a/src/tests/junit/org/apache/tools/ant/taskdefs/CopyTest.java +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/CopyTest.java @@ -21,12 +21,16 @@ package org.apache.tools.ant.taskdefs; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.BuildFileRule; import org.apache.tools.ant.FileUtilities; +import org.apache.tools.ant.taskdefs.condition.Os; import org.apache.tools.ant.util.FileUtils; +import org.junit.Assert; +import org.junit.Assume; import org.junit.Before; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +import java.io.BufferedReader; import java.io.File; import java.io.FileReader; import java.io.IOException; @@ -189,7 +193,7 @@ public class CopyTest { assertTrue(ex.getMessage().endsWith(" does not exist.")); } } - + @Test public void testFileResourcePlain() { buildRule.executeTarget("testFileResourcePlain"); @@ -212,7 +216,7 @@ public class CopyTest { assertTrue(file2.exists()); assertTrue(file3.exists()); } - + @Test public void testFileResourceWithFilter() { buildRule.executeTarget("testFileResourceWithFilter"); @@ -225,7 +229,7 @@ public class CopyTest { // no-op: not a real business error } } - + @Test public void testPathAsResource() { buildRule.executeTarget("testPathAsResource"); @@ -236,7 +240,7 @@ public class CopyTest { assertTrue(file2.exists()); assertTrue(file3.exists()); } - + @Test public void testZipfileset() { buildRule.executeTarget("testZipfileset"); @@ -252,7 +256,7 @@ public class CopyTest { public void testDirset() { buildRule.executeTarget("testDirset"); } - + @Ignore("Previously ignored due to naming convention") @Test public void testResourcePlain() { @@ -276,5 +280,60 @@ public class CopyTest { public void testOnlineResources() { buildRule.executeTarget("testOnlineResources"); } - + + /** + * Tests that the {@code copy} task doesn't corrupt the source file, if the target of the copy operation is a symlink + * to the source file being copied + * + * @throws Exception + * @see <a href="https://bz.apache.org/bugzilla/show_bug.cgi?id=60644";>issue 60644</a> + */ + @Test + public void testCopyToSymlinkedSelf() throws Exception { + // we are only going to test against systems that support symlinks + Assume.assumeTrue("Symlinks not supported on this operating system", Os.isFamily(Os.FAMILY_UNIX)); + + // setup the source files to run copying against + buildRule.executeTarget("setupSelfCopyTesting"); + final File testDir = new File(buildRule.getProject().getProperty("self.copy.test.root.dir")); + Assert.assertTrue(testDir + " was expected to be a directory", testDir.isDirectory()); + final File srcFile = new File(testDir, "file.txt"); + Assert.assertTrue("Source file " + srcFile + " was expected to be a file", srcFile.isFile()); + final long originalFileSize = srcFile.length(); + final String originalContent; + final BufferedReader reader = new BufferedReader(new FileReader(srcFile)); + try { + originalContent = FileUtils.readFully(reader); + } finally { + reader.close(); + } + Assert.assertTrue("Content missing in file " + srcFile, originalContent != null && originalContent.length() > 0); + + // run the copy tests + buildRule.executeTarget("testSelfCopy"); + // make sure the source file hasn't been impacted by the copy + assertSizeAndContent(srcFile, originalFileSize, originalContent); + final File symlinkedFile = new File(testDir, "sylmink-file.txt"); + Assert.assertTrue(symlinkedFile + " was expected to be a file", symlinkedFile.isFile()); + assertSizeAndContent(symlinkedFile, originalFileSize, originalContent); + + final File symlinkedTestDir = new File(buildRule.getProject().getProperty("self.copy.test.symlinked.dir")); + Assert.assertTrue(symlinkedTestDir + " was expected to be a directory", symlinkedTestDir.isDirectory()); + assertSizeAndContent(new File(symlinkedTestDir, "file.txt"), originalFileSize, originalContent); + assertSizeAndContent(new File(symlinkedTestDir, "sylmink-file.txt"), originalFileSize, originalContent); + + } + + private void assertSizeAndContent(final File file, final long expectedSize, final String expectedContent) throws IOException { + Assert.assertTrue(file + " was expected to be a file", file.isFile()); + Assert.assertEquals("Unexpected size of file " + file, expectedSize, file.length()); + final BufferedReader reader = new BufferedReader(new FileReader(file)); + final String content; + try { + content = FileUtils.readFully(reader); + } finally { + reader.close(); + } + Assert.assertEquals("Unexpected content in file " + file, expectedContent, content); + } }
