Use NIO Files for creating temporary files

File.createTempFile should not be used due to security issues

Author: slawekjaranowski

src/main/java/org/codehaus/plexus/archiver/zip/DeferredScatterOutputStream.java

@@ -29,7 +29,7 @@ public class DeferredScatterOutputStream implements ScatterGatherBackingStore
 
     public DeferredScatterOutputStream( int threshold )
     {
-        dfos = new OffloadingOutputStream( threshold, "scatterzipfragment", "zip", null );
+        dfos = new OffloadingOutputStream( threshold, "scatterzipfragment", "zip" );
     }
 
     @Override

src/main/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStream.java

@@ -23,11 +23,12 @@
 import java.io.OutputStream;
 import java.io.SequenceInputStream;
 import java.nio.file.Files;
+import java.nio.file.Path;
 
 import org.apache.commons.io.output.ThresholdingOutputStream;
 
 /**
- * Offloads to disk when a given memory consumption has been reacehd
+ * Offloads to disk when a given memory consumption has been reached
  */
 class OffloadingOutputStream extends ThresholdingOutputStream
 {
@@ -48,9 +49,9 @@ class OffloadingOutputStream extends ThresholdingOutputStream
     private OutputStream currentOutputStream;
 
     /**
-     * The file to which output will be directed if the threshold is exceeded.
+     * The path to which output will be directed if the threshold is exceeded.
      */
-    private File outputFile = null;
+    private Path outputPath = null;
 
     /**
      * The temporary file prefix.
@@ -62,58 +63,30 @@ class OffloadingOutputStream extends ThresholdingOutputStream
      */
     private final String suffix;
 
-    /**
-     * The directory to use for temporary files.
-     */
-    private final File directory;
-
-    /**
-     * True when close() has been called successfully.
-     */
-    private boolean closed = false;
-
     // ----------------------------------------------------------- Constructors
 
     /**
      * Constructs an instance of this class which will trigger an event at the
      * specified threshold, and save data to a temporary file beyond that point.
      *
      * @param threshold The number of bytes at which to trigger an event.
-     * @param prefix Prefix to use for the temporary file.
-     * @param suffix Suffix to use for the temporary file.
-     * @param directory Temporary file directory.
-     *
+     * @param prefix    Prefix to use for the temporary file.
+     * @param suffix    Suffix to use for the temporary file.
      * @since 1.4
      */
-    public OffloadingOutputStream( int threshold, String prefix, String suffix, File directory )
+    public OffloadingOutputStream( int threshold, String prefix, String suffix )
     {
-        this( threshold, null, prefix, suffix, directory );
+        super( threshold );
+
         if ( prefix == null )
         {
             throw new IllegalArgumentException( "Temporary file prefix is missing" );
         }
-    }
-
-    /**
-     * Constructs an instance of this class which will trigger an event at the
-     * specified threshold, and save data either to a file beyond that point.
-     *
-     * @param threshold The number of bytes at which to trigger an event.
-     * @param outputFile The file to which data is saved beyond the threshold.
-     * @param prefix Prefix to use for the temporary file.
-     * @param suffix Suffix to use for the temporary file.
-     * @param directory Temporary file directory.
-     */
-    private OffloadingOutputStream( int threshold, File outputFile, String prefix, String suffix, File directory )
-    {
-        super( threshold );
-        this.outputFile = outputFile;
 
         memoryOutputStream = new ByteArrayOutputStream( threshold / 10 );
         currentOutputStream = memoryOutputStream;
         this.prefix = prefix;
         this.suffix = suffix;
-        this.directory = directory;
     }
 
     // --------------------------------------- ThresholdingOutputStream methods
@@ -123,8 +96,7 @@ private OffloadingOutputStream( int threshold, File outputFile, String prefix, S
      * based, depending on the current state with respect to the threshold.
      *
      * @return The underlying output stream.
-     *
-     * @exception java.io.IOException if an error occurs.
+     * @throws java.io.IOException if an error occurs.
      */
     @Override
     protected OutputStream getStream() throws IOException
@@ -138,27 +110,24 @@ protected OutputStream getStream() throws IOException
      * much data is being written to keep in memory, so we elect to switch to
      * disk-based storage.
      *
-     * @exception java.io.IOException if an error occurs.
+     * @throws java.io.IOException if an error occurs.
      */
     @Override
     protected void thresholdReached() throws IOException
     {
-        if ( prefix != null )
-        {
-            outputFile = File.createTempFile( prefix, suffix, directory );
-        }
-        currentOutputStream = Files.newOutputStream( outputFile.toPath() );
+        outputPath = Files.createTempFile( prefix, suffix );
+        currentOutputStream = Files.newOutputStream( outputPath );
     }
 
     public InputStream getInputStream() throws IOException
     {
 
         InputStream memoryAsInput = memoryOutputStream.toInputStream();
-        if ( outputFile == null )
+        if ( outputPath == null )
         {
             return memoryAsInput;
         }
-        return new SequenceInputStream( memoryAsInput, Files.newInputStream( outputFile.toPath() ) );
+        return new SequenceInputStream( memoryAsInput, Files.newInputStream( outputPath ) );
     }
 
     // --------------------------------------------------------- Public methods
@@ -196,20 +165,18 @@ public byte[] getData()
      */
     public File getFile()
     {
-        return outputFile;
+        return outputPath != null ? outputPath.toFile() : null;
     }
 
     /**
-     * Closes underlying output stream, and mark this as closed
+     * Closes underlying output stream.
      *
-     * @exception java.io.IOException if an error occurs.
+     * @throws java.io.IOException if an error occurs.
      */
     @Override
     public void close() throws IOException
     {
         super.close();
-        closed = true;
         currentOutputStream.close();
     }
-
 }

src/test/java/org/codehaus/plexus/archiver/BasePlexusArchiverTest.java

@@ -51,8 +51,8 @@ protected void waitUntilNewTimestamp( File outputFile, long timestampReference )
         throws IOException
     {
         long startTime = System.currentTimeMillis();
-        File tmpFile = File.createTempFile(
-            "BasePlexusArchiverTest.waitUntilNewTimestamp", null );
+        File tmpFile = Files.createTempFile(
+            "BasePlexusArchiverTest.waitUntilNewTimestamp", null ).toFile();
         long newTimestamp;
 
         // We could easily just set the last modified time using

src/test/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStreamTest.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright The Plexus developers.
+ *
+ * Licensed 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.codehaus.plexus.archiver.zip;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+class OffloadingOutputStreamTest
+{
+
+    @Test
+    void temporaryFileShouldBeCreated() throws IOException
+    {
+        File streamFile = null;
+        try ( OffloadingOutputStream stream = new OffloadingOutputStream( 100, "test", "test" ) )
+        {
+            stream.write( new byte[256] );
+            stream.close();
+            streamFile = stream.getFile();
+            assertThat( streamFile )
+                .isFile()
+                .hasSize( 256 );
+        }
+        finally
+        {
+            if ( streamFile != null )
+            {
+                Files.delete( streamFile.toPath() );
+            }
+        }
+    }
+}