zstd icon indicating copy to clipboard operation
zstd copied to clipboard

compressed file ownership inconsistent with that of uncompressed file

Open Kawashima-Azumi opened this issue 3 years ago • 1 comments

Using the zstd command (version 1.5.1) to compress a file, and the compressed file does not have the same owner:group as that of the input file. File permissions are consistent, though.

In comparison, gzip, xz, and zstd-1.4.4 retain both permission and ownership.

After looking at https://github.com/facebook/zstd/issues/2739 it seem that the problem of losing file ownership was also introduced by https://github.com/facebook/zstd/pull/2525

A casual look at https://github.com/facebook/zstd/pull/2525 reveals multiple other issues, such as no error checking for fdopen and unnecessary use of _open and fdopen because fopen with mode "wb" is sufficient.

I'd suggest to revert https://github.com/facebook/zstd/pull/2525 for now to make things work, then refine it if necessary.

A closer look shows that https://github.com/facebook/zstd/pull/2525 is trying to solve an invalid bug report at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=981404#28 Being world readable is not a bug but a feature. Processes often run at umask 022 in a secure directory, and so world readable files cannot be actually read by others. If this is not the case, then the file is either readable by others intentionally, or the problem can be avoided by changing umask to e.g. 077 . open+fdopen does not resolve the problem (if it is a problem at all). umask does. The time window being one flaw. Another one might be:

Suppose a file owned by user1:group1, permission 0660, with open+fdopen it can get ownership user1:group2, permission 0660. It's better to umask 0777 then call the zstd command. The umask command need not be called by the zstd program. It can be issued by the user before running zstd: $ (umask 0777; zstd file_to_be_compressed.txt)

$ ls -l foo
-rw-r--r-- 1 10001 20010 0 Dec 22 15:14 foo
$ xz -k foo
$ gzip -k foo
$ (umask 0777; zstd foo)
foo                  :1300.00%   (     0 =>     13 bytes, foo.zst)             
$ ls -l foo*
-rw-r--r-- 1 10001 20010  0 Dec 22 15:14 foo
-rw-r--r-- 1 10001 20010 24 Dec 22 15:14 foo.gz
-rw-r--r-- 1 10001 20010 32 Dec 22 15:14 foo.xz
-rw-r--r-- 1 10001 20010 13 Dec 22 15:14 foo.zst
$ zstd --version
*** zstd command line interface 64-bits v1.4.4, by Yann Collet ***
$ 

Kawashima-Azumi avatar Dec 22 '21 07:12 Kawashima-Azumi

This issue was opened almost a year ago, 1.5.2 was released but, as far as I see, current git still fails to preserve owner, group and permission (when run as root). Any chance that could be fixed before 1.5.3?

leapfog avatar Dec 14 '22 14:12 leapfog

@Kawashima-Azumi, I am about to put up a PR that addresses this issue, and switches zstd's behavior back to copying the source file's attributes to the output file. Your review will be very welcome!

I wanted to discuss some of the criticisms you have of the current implementation, so that we can either agree that they are non-issues or agree on a solution for them:

A casual look at https://github.com/facebook/zstd/pull/2525 reveals multiple other issues, such as no error checking for fdopen and unnecessary use of _open and fdopen because fopen with mode "wb" is sufficient.

open() + fdopen() is necessary because (unlike fopen()) it allows us to specify the mode bits during creation.

We do error check the return of fdopen() right here.

Being world readable is not a bug but a feature.

One thing that has become clear dealing with this topic is that no behavior makes everyone happy. I think at this point the least objectionable thing we can do is mimic gzip's behavior. If you object, please let me know.

felixhandte avatar Jan 18 '23 22:01 felixhandte

Thanks @felixhandte, I checked out and compiled your fix-perms tree. It perfectly solves my issue.

While testing, I just observed that there still is a difference to gzip when a file has any of the suid, sgid or sticky bit set: gzip (at least v1.12-r4 on Gentoo) refuses to compress these files while zstd does compress them, but does not mirror these special bits. I have no opinion what is better.

leapfog avatar Jan 19 '23 09:01 leapfog