BZ-60644 Skip copying files, to avoid corrupting source files, if the source file and dest file are symlinked to each other

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>

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

@@ -417,7 +417,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
@@ -641,11 +641,17 @@ public class ResourceUtils {
                                            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;
+        }
+
         try (Reader in = filterWith(project, inputEncoding, filterChains,
-            source.getInputStream());
-                BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
-                    getOutputStream(dest, append, project),
-                    charsetFor(outputEncoding)))) {
+                source.getInputStream());
+             BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
+                     getOutputStream(dest, append, project),
+                     charsetFor(outputEncoding)))) {
 
             final LineTokenizer lineTokenizer = new LineTokenizer();
             lineTokenizer.setIncludeDelims(true);
@@ -691,11 +697,18 @@ 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;
+        }
+
         try (Reader in = filterWith(project, inputEncoding, filterChains,
-            source.getInputStream());
-                BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
-                    getOutputStream(dest, append, project),
-                    charsetFor(outputEncoding)))) {
+                source.getInputStream());
+             BufferedWriter out = new BufferedWriter(new OutputStreamWriter(
+                     getOutputStream(dest, append, project),
+                     charsetFor(outputEncoding)))) {
             final char[] buffer = new char[FileUtils.BUF_SIZE];
             while (true) {
                 final int nRead = in.read(buffer, 0, buffer.length);
@@ -705,12 +718,18 @@ public class ResourceUtils {
                 out.write(buffer, 0, nRead);
             }
         }
+
     }
 
     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())) {
@@ -738,8 +757,14 @@ 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;
+        }
         try (InputStream in = source.getInputStream();
-                OutputStream out = getOutputStream(dest, append, project)) {
+             OutputStream out = getOutputStream(dest, append, project)) {
 
             final byte[] buffer = new byte[FileUtils.BUF_SIZE];
             int count = 0;
@@ -768,6 +793,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);
     }

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);
+    }
 }