alhudz commented on code in PR #473:
URL:
https://github.com/apache/commons-fileupload/pull/473#discussion_r3346135217
##########
commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/core/DeferrableOutputStream.java:
##########
@@ -388,7 +392,16 @@ protected OutputStream persist() throws IOException {
if (dir != null) {
Files.createDirectories(dir);
}
- final OutputStream os = Files.newOutputStream(p);
+ // Restrict the temporary file to its owner where the file system
supports it. The default repository is the shared system temporary directory, so
+ // creating the file with default permissions would expose the
uploaded data to other local users.
+ final OutputStream os;
+ if (p.getFileSystem().supportedFileAttributeViews().contains("posix"))
{
+ os = Channels.newOutputStream(Files.newByteChannel(p,
+ EnumSet.of(StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE),
+
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"))));
+ } else {
+ os = Files.newOutputStream(p);
Review Comment:
Good point. The old newOutputStream call already truncated (no options means
CREATE + TRUNCATE_EXISTING + WRITE), so throwing on an existing file would be
the regression here, not truncating. I pulled the option set into one EnumSet
now used by both branches, so posix and non-posix open the file identically.
Added testPersistedFileOverwritesExisting which pre-creates the target and
checks it gets overwritten, so we'd catch it if TRUNCATE_EXISTING ever gets
dropped.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
