I'm attaching a v2 patch. This refactors setPermissionIfAllowed into FileSystem. Subclasses can call this as needed. I think this is preferable over my earlier version with the method in FileUtil, which is annotated Public.
-1 tests included. The patch doesn't appear to include any new or modified tests.
I don't think we'll be able to add a reliable unit test that works across all dev environments. The only way I know to create this condition is to set up an SMB share and run as a user that doesn't have Full Control access. Not all environments will have the necessary rights to create an SMB share. I have tested this manually, with an SMB share mounted by both a Linux host and a Windows host.
Colin P. McCabe, thank you for reviewing.
It seems wrong to be asked to create a file with a certain permission, but then create it with a different permission and declare success.
While I agree that it can be strange, it's interesting to note that there is existing precedent for handling it this way. This is the same behavior I see in both Linux and Cygwin operating on an SMB share. See below for a sample Cygwin session, where F: is mounted to an SMB share. I suppose this was a pragmatic choice for dealing with odd cases where a user may have rights to create a file but not control its permissions. This could include an SMB share denying WRITE_DAC or a file system using NFSv4 ACLs with a deny ACE on WRITE_ACL.
$ mkdir -m 755 /cygdrive/f/dir2
$ stat -c%a /cygdrive/f/dir2
Isn't there an API on Windows to set the permissions of the file we're creating?
Unfortunately, this is not a matter of using a different syscall. The OS is denying the WRITE_DAC right, so access will be denied on attempts to set the discretionary access control list.
Also, I might be misinterpreting, but this sounded like a suggestion to look for a possible solution in native Windows APIs. Just to clarify, this isn't a Windows-only bug. It's possible for a Linux host to mount an SMB share too. In that case, even if there was a native Windows API that could circumvent the WRITE_DAC denial, the solution wouldn't work for a Linux client.
I haven't searched through all those shiny new JDK7 file APIs, surely it's there?
I believe you're thinking of Files#createDirectory and Files#createFile, with the caller passing PosixFilePermissions as a file attribute. It turns out that doing this will fail fast on Windows with an unchecked exception stating that the file system doesn't implement POSIX permissions. The only reason the Hadoop code is able to do something like this is that we've written native code to translate POSIX permissions to a roughly equivalent discretionary access control list.
If we absolutely, positively can't get this right, then we can have a config option to ignore the permission argument to local file creates... ugh.
This is pretty standard Hadoop code review feedback. As a result, Hadoop now has 762 configuration properties. That's from a grep -c of core-default.xml, hdfs-default.xml, yarn-default.xml and mapred-default.xml, so the count doesn't include undocumented properties. I'm reluctant to keep pushing this higher. Would you consider the possibility that this is just a pragmatic choice for dealing with an edge case, based on the existing precedent shown elsewhere? Also, I've taken care that this patch only triggers this behavior for an EACCES, not general I/O errors or other problems.
If you insist, then let me know, and I'll add a new configuration property.