Bug #92401 for File-Slurp: test "t/perms.t" is invalid; umask test broken w/ACLs

BTW, I marked this 'critical', as it prevents File::Slurp from installing (unless done manually).

I'm not sure that I see a way to accurately test the file permissions unless I move this test to the xt/author section so that it's not tested for installs on servers where we can't guarantee there aren't ACLs in place. Do you have any other suggestions? Thanks, Chase

On 10/4/2018 7:01 PM, Chase Whitener via RT wrote: Show quoted text
I'm running this on Win7SP1, which is a desktop/workstation/client machine. I'm not sure how this is different from installs on servers. Not sure what the xt/author section is or how it prevents this problem since I'm not installing on a server in the first place. What does this have to do with servers vs. clients? (sorry, I don't understand)

On 2018-10-05 09:56:52, LAWALSH wrote: Show quoted text
You said: "I'm running this on Win7SP1", so you can hardly blame Chase for being confused and asking for more information.

It seems this entire conversation has been derailed over one word. Forget "server" vs. "client". You're attempting to install the module on a machine that uses ACLs (it doesn't matter if said machine is a server or a client or a workstation or a phone). That attempt to install failed because the test in question tries to set and test permissions on a file. I said "I'm not sure that I see a way to accurately test the file permissions unless I move this test to the xt/author section so that it's not tested for installs on servers where we can't guarantee there aren't ACLs in place. Do you have any other suggestions?" Please replace the word "servers" with "machines" in that. Do you have any suggestions about _how_ we might test to see if the machine in question is using ACLs so that we can skip the tests in such a scenario? If not, I can't think of a better solution than to move that particular test into the xt/authors set of tests (which do not run on end-user installs). Thanks, Chase On Fri Oct 05 15:07:43 2018, LAWALSH wrote: Show quoted text

On 10/5/2018 2:00 PM, Chase Whitener via RT wrote: Show quoted text
Ironically, the fact that you don't support windows has somewhat protected you from running into this problem earlier (not that I'm suggesting you should support native windows or not). Show quoted text
--- The bit of axing the test sounds best in _this_ case only because the permissions on the file are incidental to the prime purpose of the module (IMO) which is to slurp them in/out. "Obviously" (ha), if permissions prevent reading or writing, you won't be able to slurp in or out, but testing mode bits related to a umask setting seems very incidental to your primary objective. Another failure case (not tested, but suspect) would be mounting FAT (and maybe exFAT) file systems on linux. They would not have the same types of permission bits, so depending on how detailed such testing is, I could easily see failure cases there. What you _could_ do, is test for read access before reading and write access before writing. It may be simple, but as near as I can tell, it's all your module would likely need, no? Anything else, be it permission bits, ACL's, or mandatory controls like in SELinux or similar (which comes installed by default on RedHat and Suse flavors of linux) would be OS specific permission controls that seem beyond what you would want to test (I think). Am I missing something? -linda

On 10/5/2018 6:00 PM, Chase Whitener via RT wrote: Show quoted text
---- If at first you don't succeed... look for binaries 'chacl, getfacl setfacl' if on linux. I don't know of an OS independent way to check for acls, but solaris, cygwin linux are likely to have get/set facl. And see if they can be set or listed? For linux, another place to look for OS support would be /proc/config.gz: Show quoted text
CONFIG_EXT4_FS_POSIX_ACL=y CONFIG_XFS_POSIX_ACL=y CONFIG_BTRFS_FS_POSIX_ACL=y CONFIG_FS_POSIX_ACL=y CONFIG_TMPFS_POSIX_ACL=y CONFIG_CIFS_ACL=y --- it shows the running kernel's ACL support Are those the type of things you are looking for to see if ACL's may be present?...unfortunately, linux specific. or something like this (practical test): a directory (named 'file') with an acl: Show quoted text
[u::rwx,g::rwx,g:Trusted\040Local\040Net\040Users:r-x,m::rwx,o::r-x/u::rwx,u:Media:rwx,g::r-x,g:Media:rwx,g:Trusted\040Local\040Net\040Users:r-x,m::rwx,o::r-x] file Show quoted text
[u::rwx,u:Media:rwx,g::r-x,g:Media:rwx,g:Trusted\040Local\040Net\040Users:r-x,m::rwx,o::r-x/u::rwx,u:Media:rwx,g::r-x,g:Media:rwx,g:Trusted\040Local\040Net\040Users:r-x,m::rwx,o::r-x] dir Show quoted text
total 16 drwxrwsr-x 2 Media Media 36 Oct 5 21:24 ./ drwxrwxr-x 13 Media Media 4096 Oct 5 21:25 ../ -rw-rw-r-- 1 law Media 0 Oct 5 21:23 00 -rw-rw-r-- 1 law Media 0 Oct 5 21:24 77 drwxrwsr-x 2 law Media 10 Oct 5 21:32 dir/ file 00 doesn't have 'write' for other (o::r-x) and file 77 still has access for user 'law' and group Media Another gotcha -- if parent has set groupid set, that will propagate to child dirs on xfs by default, though there are ways to change that behavior: Show quoted text
... -rw-r--r-- 1 0 Oct 5 21:35 inherit_noatime -rw-r--r-- 1 0 Oct 5 21:35 inherit_nodefrag -rw-r--r-- 1 0 Oct 5 21:35 inherit_nodump -rw-r--r-- 1 0 Oct 5 21:35 inherit_nosymlinks -rw-r--r-- 1 0 Oct 5 21:35 inherit_sync -rw-r--r-- 1 0 Oct 5 21:35 irix_sgid_inherit -rw-r--r-- 1 0 Oct 5 21:35 irix_symlink_mode ... (specifically irix_sgid_inherit) As you can see, there are ways to change other behaviors to inherit parent settings or not. I think acls are on by default on most file systems EXCEPT with ext[34] they are a mount option. But I don't know of any OS-independent nor FS independent way to detect them. Some versions of 'cp' copy them, as well as some versions of tar and rsync. Show quoted text

On Fri, 5 Oct 2018 02:01:13 GMT, CAPOEIRAB wrote: Show quoted text
Checking for ACLs in a portable fashion is hard, all right. But that doesn’t mean you can’t check whether your test can possibly test something useful, no? Time for some testing 101. What do you do when you are fixing a bug, to be sure that you fixed it? First you write a test that fails. Then you fix the bug. Then you check that the test passes. Why? Because if you fix the bug first and only then write a test, then you don’t know for sure that that particular test would have failed before the bugfix. And so when it passes, you don’t actually know for sure that *the bugfix* is the reason why the test passes. And so you don’t know for sure what your bugfix actually did. Well… here you have a test that only checks for the outcome you expect. But there is no check that umask actually works. Therefore when the test fails, you don’t actually know that this means the code it tests isn’t working. It might have failed because umask failed instead of because your code failed. So… what *do* you do here to check that? You set the umask to 0, create a file, and test whether getmode($file)==0666. If that’s the case, then umask is doing what you thought it does. If not, you have to skip the tests, because something is interfering with your base assumptions about umask. (Further reading: look up “null hypothesis”.) (Note too that you MUST NOT use write_file for the file creation here, because that is the code you want to test in the first place, and you want to test the null hypothesis outside of any possible interference from that code.) So, putting it all together, what does that look like in practice? In copy-pastable terms – you move the `plan tests => 2` line down to the `umask 027` line and put the following block above that: umask 0 ; unlink $file ; { open my $fh, '>', $file or die $! } my $u0_mode = getmode( $file ) ; unlink $file ; plan skip_all => 'umask 0 does not yield 0666 permissions' if $u0_mode != 0666 ; Linda: can you verify that doing this makes the test skip without failing?

On Sun, 7 Oct 2018 19:44:36 GMT, LAWALSH wrote: Show quoted text
That doesn’t matter. The question is whether you can set ACLs in such a way that umask 0 produces 0666 *but* umask 027 does *not* produce 0640. Show quoted text
Those *definitely* don’t matter since they just end up affecting the regular Unix permission bits which compose cleanly with umask. So in a directory with a relevant setgid, the test will be skipped, because umask 0 will not produce a 0666 file. Show quoted text
Eh? How can a test with two possible outcomes neither fail nor pass…? Show quoted text
Depends on how freely you can screw up umask with ACLs, I guess. I suppose I didn’t full apply my own logic – testing what umask 0 does and then expecting that other umasks will have corresponding effects is not a null hypothesis for what is being tested. Instead, the first test could check that umask 027 yields 0640 before testing that write_file under umask 027 also produces an 0640 file, and the second test could check that an 0777 mode yields an 0750 file before testing that that’s what write_file does. (That would also permit running the test on Windows, rather than skipping it on general principle, where it would then just end up skipping both tests individually. Basically trading agent sniffing for feature detection, in webdev terms.) Will suggest a patch later.