-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Bug 961080 asks for the new download manager to "support group and
world-writable umasks for downloaded files".  It is stalled on a design
disagreement between me and the OS.File maintainers (well, it is also
stalled because my development box corrupts its filesystem when I do 'hg
pull -u', but that's not important right now) which I would like to
raise for discussion here.

In case you are unfamiliar, the 'umask' is how Unix-derived systems
(including Mac) control whether newly created files are accessible to
anyone beyond the creating user account.  It's a process-global bit
mask.  When you create a file ...

   fd = open(path, O_RDWR|O_CREAT|O_EXCL, 0666);

... that third octal argument would normally cause the file to be
created readable and writable by the current user, the current user's
primary group, and everyone else (i.e. by everyone).  But bits that are
*set* in the umask are *cleared* from the permission bits passed to
open().  So if the umask is 0027, the above open() will create a file
with permission bits set to 0640 -- readable and writable by the current
user, readable but not writable by the user's primary group, and
inaccessible to anyone else.

The new download manager is currently doing the moral equivalent of

   fd = open(path, O_RDWR|O_CREAT|O_EXCL, 0600);
                                          ^^^^

which causes all downloaded files to be inaccessible to group and other,
*regardless* of the umask setting.  (The umask can only clear permission
bits, it cannot set them.)  The bug requests that we make it "honor the
umask", i.e. go back to creating files as-if by open(,,0666). The module
owners have decided that it is not safe to just change how we call
open(); instead we need to reset the files' permissions manually *after*
they are fully downloaded.  This requires augmenting the OS.File API
somehow, and this is what I added (quoting bug 961080 comment 15):

| setPerms([path,] mode) - set the Unix file access permissions to a
|    specific numeric mode. Throws an exception if called on Windows.
|
|  makeAccessibleByOthers([path,] exec) - Adjust file access permissions
|     to make the file accessible by others, honoring any relevant
|     system configuration about that.  If 'exec' is true, make the
|     file executable, otherwise don't. On Unix, that's
|
|         chmod(path, (exec ? 0777 : 0666) & ~umask);
|
|     On Windows, currently does nothing.  I suspect there is
|     something useful it could do on Windows, but I don't know
|     what it is.

Both Paolo Amadini and David R. Teller objected to this API; their
preferred API looks something like

| unixSetPermissions([path,] mode [, options]) - set the Unix file
|    access permissions to 'mode & ~umask' by default; if 'options'
|    is '{ honorUmask: false }', then set them to exactly 'mode'
|    instead.  Not present on Windows.

I think their proposal is not a good API, and here's why.  First, I
don't think an operation named setPermissions() should honor the umask
by default, because *most* set-permissions operations won't want to,
and it's inconsistent with the C-level API (chmod()).  "I have already
created this file, now correct its permissions to what they would have
been if the umask had been allowed to determine them" is an unusual
scenario that should be easily greppable for; it deserves its own name.

Second, if you are in a situation where you *do* want to honor the
umask, then there are only two correct choices for the permissions to
combine with the umask: 0666 if the file shouldn't be executable, 0777
if it should (or if it's a directory).  It is better to boil this down
to a boolean ('exec') than to make all callers get it right.

Third, there is probably something analogous that we should be doing
on Windows; I am not qualified to know what it is, but I am 100%
certain that this, too, should *not* be something that all callers
have to get right, or even know about.  Therefore I think that
whatever new API is added should be defined in cross-platform terms
(such as "make accessible by [appropriate] others").

Against these (particularly the third) was the claim that this API is
only useful to the download manager.  I don't believe that.  I suspect
we have failure-to-honor-umask-or-Windows-analogue bugs in every place
where we create a file outside the profile directory, and that
therefore my proposed API is at least potentially relevant --
everywhere we create files.  And I believe it is correctly designed
for the general case and not just the download manager.

zw
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Icedove - http://www.enigmail.net/

iQIcBAEBCAAGBQJTWqSvAAoJEJH8wytnaapksg8P/jYu3oAAr5u8+devUCWN/Uos
THhjyHq0DQKO5JzHWhAOVbQpUeD6TLkJdp5HPmCsodx3J416G0sG9D+4DqJ5tpQU
tGahDaPwJcnjEaq/KhzhxEq6s6E7z0n01Pu5YxgC84MMTWsvIcuVEsoYZBKDNo6y
Y34qASTB57OhjO164aqNBLblm9nZPCPdMqLg8TeFA2EeO6pmy4enaxj+XEcED7yr
CMziKu0Ild2gVXDzUNaiTALNlkItDsP+aFi1EV3PLoX2EgJObzIokGQ99uySX15q
SNUG1lF5X4k1USICnkfqDTTi2Jr0kAjGJp7bZs+v+fNs8i18w1XK0ERU+7G2t8jF
uF91ame9QunfJAr0kO+ezEcMKB6fGQOkpk0TvaBNNQIaLeTVRRjTO19WQfQkej8y
ddmdy5Pi+LjoWUqL9vjRYp6Zk/bdi4ovC1V8tFDVEDg9l5uzsdnnctZI7GajCAqw
mjZdkDwvNA+8DaAzAWX0CtC9ivmXW7Wq+U0c42Gf5LgqnorO544xbfXu0KFzG9l4
nYv7Jfc2p/TU2EdN8xxPk7v4UaG/b6HF+N4oLDUuPsv2JNuKFcv754OzyEMDDJ8n
bU3OmZMoymRuRqSoPiDFt96vQSLl3ZxmMvPVB5mCIqMAFkrC8xm94l7nijamMAa8
UPtBM/LBuwnorGQfn9TF
=DFbE
-----END PGP SIGNATURE-----
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to