garydgregory commented on a change in pull request #131: URL: https://github.com/apache/commons-io/pull/131#discussion_r461675952
########## File path: src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java ########## @@ -73,6 +83,48 @@ public void copyDirectoryToDirectoryThrowsNullPointerExceptionWithCorrectMessage assertExceptionTypeAndMessage(srcDir, destDir, NullPointerException.class, "destinationDir"); } + + @Test + public void copyFileWrongPermissions() throws IOException { + + + final File destDir = createTemporaryFolderIn(null); + final Path srcFile = Files.createTempFile("tmp-output", ".xml"); + final Path path = Paths.get(destDir.getAbsolutePath(), "newFile.xml"); + + try { + FileUtils.copyFile(srcFile.toFile(), path.toFile()); + } catch (IllegalArgumentException iae) { + iae.printStackTrace(); Review comment: Don't catch and log to sys err. Just let the exception percolate up. ########## File path: src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java ########## @@ -35,6 +40,11 @@ @TempDir public File temporaryFolder; + Review comment: Delete extra blank line. ########## File path: src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java ########## @@ -73,6 +83,48 @@ public void copyDirectoryToDirectoryThrowsNullPointerExceptionWithCorrectMessage assertExceptionTypeAndMessage(srcDir, destDir, NullPointerException.class, "destinationDir"); } + + @Test + public void copyFileWrongPermissions() throws IOException { + + + final File destDir = createTemporaryFolderIn(null); + final Path srcFile = Files.createTempFile("tmp-output", ".xml"); + final Path path = Paths.get(destDir.getAbsolutePath(), "newFile.xml"); + + try { + FileUtils.copyFile(srcFile.toFile(), path.toFile()); + } catch (IllegalArgumentException iae) { + iae.printStackTrace(); + } + + assertTrue(Files.getPosixFilePermissions(path).contains(PosixFilePermission.OTHERS_READ), Files.getPosixFilePermissions(path).toString()); + + } + + + + private File createTemporaryFolderIn(File parentFolder) throws IOException { Review comment: What is this all about? Since we are testing `FileUtils.copyFile()`, why do we need an extra level of temp folder beyond what the existing `temporaryFolder` provide? ########## File path: src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTestCase.java ########## @@ -73,6 +83,48 @@ public void copyDirectoryToDirectoryThrowsNullPointerExceptionWithCorrectMessage assertExceptionTypeAndMessage(srcDir, destDir, NullPointerException.class, "destinationDir"); } + + @Test + public void copyFileWrongPermissions() throws IOException { + Review comment: Delete extra blank lines. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org