garydgregory commented on code in PR #473:
URL:
https://github.com/apache/commons-fileupload/pull/473#discussion_r3340744773
##########
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:
Hello @alhudz
Thank you for your update.
Now the behavior is different b/w file system WRT overwriting files. IMO,
the`StandardOpenOption.TRUNCATE_EXISTING` flag should be used in both paths or
not at all.
The question for both code paths is whether using
`StandardOpenOption.TRUNCATE_EXISTING` breaks with the past behavior enough to
be considered a bug. WDYT? Or, is it the case, that throwing if the file
exists, is in fact a bug?
If `StandardOpenOption.TRUNCATE_EXISTING` is going to be used, a tests
verify that an overwrite is allowed to avoid a regression if the flag is later
removed.
TY!
--
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]
