diff --git a/src/main/java/org/apache/commons/io/output/DeferredFileOutputStream.java b/src/main/java/org/apache/commons/io/output/DeferredFileOutputStream.java index 1df7978db14..18697a7623f 100644 --- a/src/main/java/org/apache/commons/io/output/DeferredFileOutputStream.java +++ b/src/main/java/org/apache/commons/io/output/DeferredFileOutputStream.java @@ -479,14 +479,14 @@ protected void thresholdReached() throws IOException { tempFile = true; } PathUtils.createParentDirectories(outputPath, null, PathUtils.EMPTY_FILE_ATTRIBUTE_ARRAY); - final OutputStream fos = Files.newOutputStream(outputPath); + final OutputStream os = Files.newOutputStream(Objects.requireNonNull(outputPath, "Either output file or prefix must be specified.")); try { - memoryOutputStream.writeTo(fos); + memoryOutputStream.writeTo(os); } catch (final IOException e) { - fos.close(); + os.close(); throw e; } - currentOutputStream = fos; + currentOutputStream = os; memoryOutputStream = null; } diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java b/src/test/java/org/apache/commons/io/FileUtilsTest.java index f340a0e1c0c..138c1ac07c2 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java @@ -83,6 +83,7 @@ import org.apache.commons.io.file.AbstractTempDirTest; import org.apache.commons.io.file.Counters.PathCounters; +import org.apache.commons.io.file.NioFileSystem; import org.apache.commons.io.file.PathUtils; import org.apache.commons.io.file.TempDirectory; import org.apache.commons.io.file.TempFile; @@ -96,7 +97,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledIf; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.io.TempDir; @@ -2843,12 +2843,11 @@ void testReadFileToByteArray_Errors() { } @Test - @EnabledIf("isPosixFilePermissionsSupported") void testReadFileToByteArray_IOExceptionOnPosixFileSystem() throws Exception { + assumeTrue(NioFileSystem.isPosix(tempDirPath)); final File file = TestUtils.newFile(tempDirFile, "cant-read.txt"); TestUtils.createFile(file, 100); Files.setPosixFilePermissions(file.toPath(), PosixFilePermissions.fromString("---------")); - assertThrows(IOException.class, () -> FileUtils.readFileToByteArray(file)); } @@ -2861,12 +2860,11 @@ void testReadFileToString_Errors() { } @Test - @EnabledIf("isPosixFilePermissionsSupported") void testReadFileToString_IOExceptionOnPosixFileSystem() throws Exception { + assumeTrue(NioFileSystem.isPosix(tempDirPath)); final File file = TestUtils.newFile(tempDirFile, "cant-read.txt"); TestUtils.createFile(file, 100); Files.setPosixFilePermissions(file.toPath(), PosixFilePermissions.fromString("---------")); - assertThrows(IOException.class, () -> FileUtils.readFileToString(file)); } @@ -2890,8 +2888,8 @@ void testReadFileToStringWithEncoding() throws Exception { } @Test - @EnabledIf("isPosixFilePermissionsSupported") void testReadLines_IOExceptionOnPosixFileSystem() throws Exception { + assumeTrue(NioFileSystem.isPosix(tempDirPath)); final File file = TestUtils.newFile(tempDirFile, "cant-read.txt"); TestUtils.createFile(file, 100); Files.setPosixFilePermissions(file.toPath(), PosixFilePermissions.fromString("---------")); diff --git a/src/test/java/org/apache/commons/io/file/AbstractTempDirTest.java b/src/test/java/org/apache/commons/io/file/AbstractTempDirTest.java index 2afe9ca656c..a27353e41db 100644 --- a/src/test/java/org/apache/commons/io/file/AbstractTempDirTest.java +++ b/src/test/java/org/apache/commons/io/file/AbstractTempDirTest.java @@ -22,7 +22,6 @@ import java.io.File; import java.io.IOException; -import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; @@ -97,8 +96,4 @@ public void beforeEachCreateTempDirs() throws IOException { tempDirFile = tempDirPath.toFile(); } - @SuppressWarnings("resource") // no FileSystem allocation - protected final boolean isPosixFilePermissionsSupported() { - return FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); - } } diff --git a/src/test/java/org/apache/commons/io/file/NioFileSystem.java b/src/test/java/org/apache/commons/io/file/NioFileSystem.java new file mode 100644 index 00000000000..54d19027497 --- /dev/null +++ b/src/test/java/org/apache/commons/io/file/NioFileSystem.java @@ -0,0 +1,129 @@ +/* + * 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 + * + * https://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.commons.io.file; + +import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.util.Set; + +/** + * Helps tests use {@link FileSystem}s. + */ +public final class NioFileSystem { + // macOS: + // jshell> FileSystems.getDefault().supportedFileAttributeViews() + // $1 ==> [owner, basic, posix, user, unix] + + /** + * The {@value} name from {@link FileSystem#supportedFileAttributeViews()}. + */ + public static final String BASIC = "basic"; + + /** + * The {@value} name from {@link FileSystem#supportedFileAttributeViews()}. + */ + public static final String DOS = "dos"; + + /** + * The {@value} name from {@link FileSystem#supportedFileAttributeViews()}. + */ + public static final String OWNER = "owner"; + + /** + * The {@value} name from {@link FileSystem#supportedFileAttributeViews()}. + */ + public static final String POSIX = "posix"; + + /** + * The {@value} name from {@link FileSystem#supportedFileAttributeViews()}. + */ + public static final String UNIX = "unix"; + + /** + * The {@value} name from {@link FileSystem#supportedFileAttributeViews()}. + */ + public static final String USER = "user"; + + /** + * Tests whether the given view names contains the {@link #DOS} name. + * + * @param views The names to test. + * @return whether the given view names contains the {@link #DOS} name. + */ + public static boolean isDos(final Set views) { + return views.contains(DOS); + } + + /** + * Tests whether the given FileSystem contains the {@link #POSIX} file attribute view name. + * + * @param fileSystem The FileSystem to test. + * @return whether the given FileSystem contains the {@link #POSIX} file attribute view name. + */ + public static boolean isPosix(final FileSystem fileSystem) { + return supportsFileAttributeView(fileSystem, POSIX); + } + + /** + * Tests whether the given Path contains the {@link #POSIX} file attribute view name. + * + * @param path The Path to test. + * @return whether the given FileSystem contains the {@link #POSIX} file attribute view name. + */ + public static boolean isPosix(final Path path) { + return supportsFileAttributeView(path.getFileSystem(), POSIX); + } + + /** + * Tests whether the given view names contains the {@link #POSIX} name. + * + * @param views The names to test. + * @return whether the given view names contains the {@link #POSIX} name. + */ + public static boolean isPosix(final Set views) { + return views.contains(POSIX); + } + + /** + * Tests whether the given view names contains the {@link #UNIX} name. + * + * @param views The names to test. + * @return whether the given view names contains the {@link #UNIX} name. + */ + public static boolean isUnix(final Set views) { + return views.contains(UNIX); + } + + /** + * Tests whether the given FileSystem contains the {@code view} file attribute view name. + * + * @param fileSystem The FileSystem to test. + * @param view The view name to test. + * @return whether the given FileSystem contains the {@code view} file attribute view name. + */ + public static boolean supportsFileAttributeView(final FileSystem fileSystem, final String view) { + return fileSystem.supportedFileAttributeViews().contains(view); + } + + /** + * No instances needed. + */ + private NioFileSystem() { + // empty + } +} diff --git a/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java b/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java index 34b01be6186..739c4ba0580 100644 --- a/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java +++ b/src/test/java/org/apache/commons/io/output/DeferredFileOutputStreamTest.java @@ -24,17 +24,22 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.IntStream; import org.apache.commons.io.IOUtils; import org.apache.commons.io.file.AbstractTempDirTest; +import org.apache.commons.io.file.NioFileSystem; import org.apache.commons.io.file.PathUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -205,6 +210,57 @@ void testBelowThresholdGetInputStream(final int initialBufferSize) throws IOExce } } + /** + * Tests that the temporary file is created with owner-only permissions on POSIX file systems. + */ + @Test + void testPersistedFileIsOwnerOnlyOnPosix() throws IOException { + assumeTrue(NioFileSystem.isPosix(tempDirPath)); + // Mirror production, where the supplier only resolves a not-yet-existing path. + final Path target = tempDirPath.resolve("posixPerms.bin"); + final AtomicReference pathRef = new AtomicReference<>(); + try (DeferredFileOutputStream dos = DeferredFileOutputStream.builder() + // @formatter:off + .setPath(target) + .setPrefix(getClass().getSimpleName()) + .setDeleteTempFileOnClose(false) + .get() + // @formatter:on + ) { + dos.write(testBytes); + pathRef.set(dos.getPath()); + } + final Path pathTemp = pathRef.get(); + assertTrue(Files.exists(pathTemp)); + assertTrue(Files.isRegularFile(pathTemp)); + assertEquals("rw-------", PosixFilePermissions.toString(Files.getPosixFilePermissions(pathTemp))); + } + + /** + * Tests that persisting over an already existing path truncates and overwrites it. + */ + @Test + void testPersistedFileOverwritesExisting() throws IOException { + final Path target = tempDirPath.resolve("overwrite.bin"); + Files.write(target, "stale-and-longer".getBytes(StandardCharsets.UTF_8)); + final AtomicReference pathRef = new AtomicReference<>(); + try (DeferredFileOutputStream dos = DeferredFileOutputStream.builder() + // @formatter:off + .setPath(target) + .setPrefix(getClass().getSimpleName()) + .setDeleteTempFileOnClose(false) + .get() + // @formatter:on + ) { + dos.write(testBytes); + pathRef.set(dos.getPath()); + } + final Path pathTemp = pathRef.get(); + assertTrue(Files.exists(pathTemp)); + assertTrue(Files.isRegularFile(pathTemp)); + assertArrayEquals(testBytes, Files.readAllBytes(pathTemp)); + } + /** * Tests that writing beyond the threshold throws a {@link NoSuchFileException} when the directory supplied to * {@link DeferredFileOutputStream.Builder#setDirectory(Path)} does not exist. @@ -516,8 +572,7 @@ void testWriteToLarge(final int initialBufferSize) throws IOException { assertThrows(IOException.class, () -> dfos.writeTo(baos)); dfos.close(); dfos.writeTo(baos); - final byte[] copiedBytes = baos.toByteArray(); - assertArrayEquals(testBytes, copiedBytes); + assertArrayEquals(testBytes, baos.toByteArray()); verifyResultFile(testFile); } } @@ -541,8 +596,7 @@ void testWriteToLargeCtor(final int initialBufferSize) throws IOException { assertEquals(testBytes.length, dfos.getByteCount()); dfos.close(); dfos.writeTo(baos); - final byte[] copiedBytes = baos.toByteArray(); - assertArrayEquals(testBytes, copiedBytes); + assertArrayEquals(testBytes, baos.toByteArray()); verifyResultFile(testFile); } } @@ -567,8 +621,7 @@ void testWriteToSmall(final int initialBufferSize) throws IOException { assertEquals(testBytes.length, dfos.getByteCount()); dfos.close(); dfos.writeTo(baos); - final byte[] copiedBytes = baos.toByteArray(); - assertArrayEquals(testBytes, copiedBytes); + assertArrayEquals(testBytes, baos.toByteArray()); } }