From 07c54f0bb1ee4a273798552f4df9148e96313c21 Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Sun, 23 Jul 2017 19:51:30 +0530 Subject: [PATCH 1/3] 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 | 27 +++++++ .../apache/tools/ant/util/ResourceUtils.java | 57 ++++++++++++++- .../apache/tools/ant/taskdefs/CopyTest.java | 71 +++++++++++++++++-- 3 files changed, 147 insertions(+), 8 deletions(-) diff --git a/src/etc/testcases/taskdefs/copy.xml b/src/etc/testcases/taskdefs/copy.xml index bf4441c17..2601556fe 100644 --- a/src/etc/testcases/taskdefs/copy.xml +++ b/src/etc/testcases/taskdefs/copy.xml @@ -265,4 +265,31 @@ a=b= + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/org/apache/tools/ant/util/ResourceUtils.java b/src/main/org/apache/tools/ant/util/ResourceUtils.java index 518ed5ad5..0c319addc 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); } 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 a7a32a9a5..719bc4456 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 issue 60644 + */ + @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); + } } From 3522f74357d942e0c28aff873a5b3b5c5a9b3458 Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Thu, 28 Sep 2017 18:31:30 +0530 Subject: [PATCH 2/3] Update release notes. This closes #37 pull request at github/apache/ant --- WHATSNEW | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/WHATSNEW b/WHATSNEW index eeae3e868..42938853c 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -13,6 +13,11 @@ Fixed bugs: parent directory of a transferred file, instead of changing it on the transferred file itself. + * Bugzilla Report 60644 - Fixed the issue where the source file + being copied could end up being corrupted if the target of the + copy happened to be the same source file (symlinked back + to itself). + Other changes: -------------- From 3a5bf997b7aae95f707ac0df562fd8ea05be6bb3 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Sat, 30 Sep 2017 06:40:37 +0200 Subject: [PATCH 3/3] line ends --- .../apache/tools/ant/util/DateUtilsTest.java | 212 +++++++++--------- 1 file changed, 106 insertions(+), 106 deletions(-) diff --git a/src/tests/junit/org/apache/tools/ant/util/DateUtilsTest.java b/src/tests/junit/org/apache/tools/ant/util/DateUtilsTest.java index 8a0a8dc23..4f5aa844d 100644 --- a/src/tests/junit/org/apache/tools/ant/util/DateUtilsTest.java +++ b/src/tests/junit/org/apache/tools/ant/util/DateUtilsTest.java @@ -1,106 +1,106 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ -package org.apache.tools.ant.util; - -import java.util.Calendar; -import java.util.TimeZone; - -import org.junit.Test; - -import static org.junit.Assert.assertEquals; - -/** - * TestCase for DateUtils. - * - */ -public class DateUtilsTest { - - - @Test - public void testElapsedTime(){ - String text = DateUtils.formatElapsedTime(50*1000); - assertEquals("50 seconds", text); - text = DateUtils.formatElapsedTime(65*1000); - assertEquals("1 minute 5 seconds", text); - text = DateUtils.formatElapsedTime(120*1000); - assertEquals("2 minutes 0 seconds", text); - text = DateUtils.formatElapsedTime(121*1000); - assertEquals("2 minutes 1 second", text); - } - - // https://issues.apache.org/bugzilla/show_bug.cgi?id=44659 - @Test - public void testLongElapsedTime(){ - assertEquals("2926 minutes 13 seconds", - DateUtils.formatElapsedTime(1000 * 175573)); - assertEquals("153722867280912 minutes 55 seconds", - DateUtils.formatElapsedTime(Long.MAX_VALUE)); - } - - @Test - public void testDateTimeISO(){ - TimeZone timeZone = TimeZone.getTimeZone("GMT+1"); - Calendar cal = Calendar.getInstance(timeZone); - cal.set(2002,1,23,10,11,12); - String text = DateUtils.format(cal.getTime(), - DateUtils.ISO8601_DATETIME_PATTERN); - assertEquals("2002-02-23T09:11:12", text); - } - - @Test - public void testDateISO(){ - TimeZone timeZone = TimeZone.getTimeZone("GMT"); - Calendar cal = Calendar.getInstance(timeZone); - cal.set(2002,1,23); - String text = DateUtils.format(cal.getTime(), - DateUtils.ISO8601_DATE_PATTERN); - assertEquals("2002-02-23", text); - } - - @Test - public void testTimeISODate(){ - // make sure that elapsed time in set via date works - TimeZone timeZone = TimeZone.getTimeZone("GMT+1"); - Calendar cal = Calendar.getInstance(timeZone); - cal.set(2002,1,23, 21, 11, 12); - String text = DateUtils.format(cal.getTime(), - DateUtils.ISO8601_TIME_PATTERN); - assertEquals("20:11:12", text); - } - - @Test - public void testTimeISO(){ - // make sure that elapsed time in ms works - long ms = (20*3600 + 11*60 + 12)*1000; - String text = DateUtils.format(ms, - DateUtils.ISO8601_TIME_PATTERN); - assertEquals("20:11:12", text); - } - - @Test - public void testPhaseOfMoon() { - TimeZone timeZone = TimeZone.getTimeZone("GMT"); - Calendar cal = Calendar.getInstance(timeZone); - // should be full moon - cal.set(2002, 2, 27); - assertEquals(4, DateUtils.getPhaseOfMoon(cal)); - // should be new moon - cal.set(2002, 2, 12); - assertEquals(0, DateUtils.getPhaseOfMoon(cal)); - } -} +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package org.apache.tools.ant.util; + +import java.util.Calendar; +import java.util.TimeZone; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * TestCase for DateUtils. + * + */ +public class DateUtilsTest { + + + @Test + public void testElapsedTime(){ + String text = DateUtils.formatElapsedTime(50*1000); + assertEquals("50 seconds", text); + text = DateUtils.formatElapsedTime(65*1000); + assertEquals("1 minute 5 seconds", text); + text = DateUtils.formatElapsedTime(120*1000); + assertEquals("2 minutes 0 seconds", text); + text = DateUtils.formatElapsedTime(121*1000); + assertEquals("2 minutes 1 second", text); + } + + // https://issues.apache.org/bugzilla/show_bug.cgi?id=44659 + @Test + public void testLongElapsedTime(){ + assertEquals("2926 minutes 13 seconds", + DateUtils.formatElapsedTime(1000 * 175573)); + assertEquals("153722867280912 minutes 55 seconds", + DateUtils.formatElapsedTime(Long.MAX_VALUE)); + } + + @Test + public void testDateTimeISO(){ + TimeZone timeZone = TimeZone.getTimeZone("GMT+1"); + Calendar cal = Calendar.getInstance(timeZone); + cal.set(2002,1,23,10,11,12); + String text = DateUtils.format(cal.getTime(), + DateUtils.ISO8601_DATETIME_PATTERN); + assertEquals("2002-02-23T09:11:12", text); + } + + @Test + public void testDateISO(){ + TimeZone timeZone = TimeZone.getTimeZone("GMT"); + Calendar cal = Calendar.getInstance(timeZone); + cal.set(2002,1,23); + String text = DateUtils.format(cal.getTime(), + DateUtils.ISO8601_DATE_PATTERN); + assertEquals("2002-02-23", text); + } + + @Test + public void testTimeISODate(){ + // make sure that elapsed time in set via date works + TimeZone timeZone = TimeZone.getTimeZone("GMT+1"); + Calendar cal = Calendar.getInstance(timeZone); + cal.set(2002,1,23, 21, 11, 12); + String text = DateUtils.format(cal.getTime(), + DateUtils.ISO8601_TIME_PATTERN); + assertEquals("20:11:12", text); + } + + @Test + public void testTimeISO(){ + // make sure that elapsed time in ms works + long ms = (20*3600 + 11*60 + 12)*1000; + String text = DateUtils.format(ms, + DateUtils.ISO8601_TIME_PATTERN); + assertEquals("20:11:12", text); + } + + @Test + public void testPhaseOfMoon() { + TimeZone timeZone = TimeZone.getTimeZone("GMT"); + Calendar cal = Calendar.getInstance(timeZone); + // should be full moon + cal.set(2002, 2, 27); + assertEquals(4, DateUtils.getPhaseOfMoon(cal)); + // should be new moon + cal.set(2002, 2, 12); + assertEquals(0, DateUtils.getPhaseOfMoon(cal)); + } +}