ant-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jaiki...@apache.org
Subject [1/2] ant git commit: BZ-60644 Skip copying files, to avoid corrupting source files, if the source file and dest file are symlinked to each other
Date Thu, 28 Sep 2017 13:02:06 GMT
Repository: ant
Updated Branches:
  refs/heads/1.9.x 01613e0d8 -> 3522f7435


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/1.9.x
Commit: 07c54f0bb1ee4a273798552f4df9148e96313c21
Parents: 01613e0
Author: Jaikiran Pai <jaikiran.pai@gmail.com>
Authored: Sun Jul 23 19:51:30 2017 +0530
Committer: Jaikiran Pai <jaikiran.pai@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);
+    }
 }


Mime
View raw message