Skip to content

Commit

Permalink
[MRESOLVER-313] Wrong FS permissions on cached artifacts. (#234)
Browse files Browse the repository at this point in the history
Collocated temp file should NOT use Files.createTempFile as it uses 0600 perms and not relies implicitly on umask. For "randomized" name using very same technique as Files.createTempFile does under the hood.

Remove one unjustified use of collocated temp file.

---

https://issues.apache.org/jira/browse/MRESOLVER-313
  • Loading branch information
cstamas committed Jan 11, 2023
1 parent 71ac861 commit e665213
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private boolean validateExternalChecksums( Map<String, ?> actualChecksums )
continue;
}
File checksumFile = getChecksumFile( checksumLocation.getChecksumAlgorithmFactory() );
try ( FileUtils.TempFile tempFile = FileUtils.newTempFile( checksumFile.toPath() ) )
try ( FileUtils.TempFile tempFile = FileUtils.newTempFile() )
{
File tmp = tempFile.getPath().toFile();
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.concurrent.ThreadLocalRandom;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -62,8 +63,12 @@ public interface CollocatedTempFile extends TempFile
}

/**
* Creates a {@link TempFile}. It will be in the default temporary-file directory. Returned instance should be
* handled in try-with-resource construct and created temp file is removed on close, if exists.
* Creates a {@link TempFile} instance and backing temporary file on file system. It will be located in the default
* temporary-file directory. Returned instance should be handled in try-with-resource construct and created
* temp file is removed (if exists) when returned instance is closed.
* <p>
* This method uses {@link Files#createTempFile(String, String, java.nio.file.attribute.FileAttribute[])} to create
* the temporary file on file system.
*/
public static TempFile newTempFile() throws IOException
{
Expand All @@ -85,17 +90,23 @@ public void close() throws IOException
}

/**
* Creates a {@link TempFile} for given file. It will be in same directory where given file is, and will reuse its
* name for generated name. Returned instance should be handled in try-with-resource construct and created temp
* file once ready can be moved to passed in {@code file} parameter place.
* Creates a {@link CollocatedTempFile} instance for given file without backing file. The path will be located in
* same directory where given file is, and will reuse its name for generated (randomized) name. Returned instance
* should be handled in try-with-resource and created temp path is removed (if exists) when returned instance is
* closed. The {@link CollocatedTempFile#move()} makes possible to atomically replace passed in file with the
* processed content written into a file backing the {@link CollocatedTempFile} instance.
* <p>
* The {@code file} nor it's parent directories have to exist. The parent directories are created if needed.
* <p>
* This method uses {@link Path#resolve(String)} to create the temporary file path in passed in file parent
* directory, but it does NOT create backing file on file system.
*/
public static CollocatedTempFile newTempFile( Path file ) throws IOException
{
Path parent = requireNonNull( file.getParent(), "file must have parent" );
Files.createDirectories( parent );
Path tempFile = Files.createTempFile( parent, file.getFileName().toString(), "tmp" );
Path tempFile = parent.resolve( file.getFileName() + "."
+ Long.toUnsignedString( ThreadLocalRandom.current().nextLong() ) + ".tmp" );
return new CollocatedTempFile()
{
@Override
Expand Down

0 comments on commit e665213

Please sign in to comment.