From 53f6d2a73cd9ebad37b2e95da6436a3986d84fb8 Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Wed, 3 May 2023 14:17:51 +0200 Subject: [PATCH] Use NIO Files for creating temporary files File.createTempFile should not be used due to security issues --- .../zip/DeferredScatterOutputStream.java | 2 +- .../archiver/zip/OffloadingOutputStream.java | 69 +++++-------------- .../archiver/BasePlexusArchiverTest.java | 4 +- .../zip/OffloadingOutputStreamTest.java | 52 ++++++++++++++ 4 files changed, 73 insertions(+), 54 deletions(-) create mode 100644 src/test/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStreamTest.java diff --git a/src/main/java/org/codehaus/plexus/archiver/zip/DeferredScatterOutputStream.java b/src/main/java/org/codehaus/plexus/archiver/zip/DeferredScatterOutputStream.java index 36df563a7..730fa6dbe 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/DeferredScatterOutputStream.java +++ b/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 diff --git a/src/main/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStream.java b/src/main/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStream.java index 1a9951fa9..be56fa274 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStream.java +++ b/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,16 +63,6 @@ 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 /** @@ -79,41 +70,23 @@ class OffloadingOutputStream extends ThresholdingOutputStream * 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(); } - } diff --git a/src/test/java/org/codehaus/plexus/archiver/BasePlexusArchiverTest.java b/src/test/java/org/codehaus/plexus/archiver/BasePlexusArchiverTest.java index 647b0f072..f779ced80 100644 --- a/src/test/java/org/codehaus/plexus/archiver/BasePlexusArchiverTest.java +++ b/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 diff --git a/src/test/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStreamTest.java b/src/test/java/org/codehaus/plexus/archiver/zip/OffloadingOutputStreamTest.java new file mode 100644 index 000000000..62ee2fa27 --- /dev/null +++ b/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() ); + } + } + } +}