Last Comment Bug 235781 - XPInstall ignores user's umask when installing files
: XPInstall ignores user's umask when installing files
Status: RESOLVED FIXED
: fixed1.7
Product: Core Graveyard
Classification: Graveyard
Component: Installer: XPInstall Engine (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: ---
Assigned To: David Baron :dbaron: ⌚️UTC-10
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-02-26 19:56 PST by Andrew Schultz
Modified: 2015-12-11 07:21 PST (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
dbaron's patch minus a chmod (5.78 KB, patch)
2004-02-26 20:16 PST, Andrew Schultz
no flags Details | Diff | Splinter Review
add PR_TRUNCATE and workaround for standalone (6.52 KB, patch)
2004-02-26 21:54 PST, Andrew Schultz
no flags Details | Diff | Splinter Review

Description Andrew Schultz 2004-02-26 19:56:05 PST
When files are installed via XPInstall, the user's umask is ignored.  The
XPInstall engine should respect the user's umask setting.  The problem code is here:

nsZipArchive::ExtractFile
http://lxr.mozilla.org/mozilla/source/modules/libjar/nsZipArchive.cpp#641
nsJAR::Extract
http://lxr.mozilla.org/mozilla/source/modules/libjar/nsJAR.cpp#251

both open the file with 0644 perms and then chmod the file to the appropriate
perms, but this sidesteps any umask.

Just to make things more complicated, the nsZipArchive is part of standalone
libjar, where PR_Open is defined as fopen in zipstub.h, so passing the file's
mode to PR_Open wouldn't work in that situation.

this bug contributed to problems in bug 231083.
Comment 1 Andrew Schultz 2004-02-26 20:01:34 PST
also note that (as argued against in bug 111124), directories are created with
appropriately umasked permissions here:
http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsInstallFile.cpp#213
Comment 2 Andrew Schultz 2004-02-26 20:16:09 PST
Created attachment 142404 [details] [diff] [review]
dbaron's patch minus a chmod

this is dbaron's patch from bug 231083 except that this patch also removes the
chmod from nsJAR::Extract.  This would fix things except for the standalone
libjar/PR_Open problem.
Comment 3 David Baron :dbaron: ⌚️UTC-10 2004-02-26 21:28:00 PST
I'd actually wanted to add an nsIFile::Remove / PR_Delete before the PR_Open /
GetANSIFileDesc calls so that it would be creating a new file rather than
reusing an old one that might have different permissions.  Otherwise a XPI
couldn't fix permissions that were wrong in an old buggy one, I think.
Comment 4 Andrew Schultz 2004-02-26 21:54:48 PST
Created attachment 142413 [details] [diff] [review]
add PR_TRUNCATE and workaround for standalone

this adds PR_TRUNCATE to the flags so that permissions are set for existing
files.	

this patch also grabs the umask and uses it to chmod the extracted file when
using standalone libjar.
Comment 5 Benjamin Smedberg [:bsmedberg] 2004-03-01 05:27:47 PST
Comment on attachment 142413 [details] [diff] [review]
add PR_TRUNCATE and workaround for standalone

I don't know enough about *nix file systems and umask sto review this.
Leaf/dbaron/dveditz?
Comment 6 David Baron :dbaron: ⌚️UTC-10 2004-08-02 17:38:30 PDT
How does this work around the problem for standalone?  Anyway, I attached a
patch that does this the cheap way to bug 231803.
Comment 7 David Baron :dbaron: ⌚️UTC-10 2004-08-02 17:39:50 PDT
er, bug 231083.
Comment 8 David Baron :dbaron: ⌚️UTC-10 2004-08-03 02:31:59 PDT
Oh, I do see the workaround for standalone now, but I'd prefer that in a PR_Open.

Did you test that PR_TRUNCATE causes the permissions to be used if the file
exists?  Based on the documentation of open(2), I'd guess that it wouldn't do so.
Comment 9 David Baron :dbaron: ⌚️UTC-10 2004-08-03 02:44:03 PDT
(In reply to comment #8)
> Oh, I do see the workaround for standalone now, but I'd prefer that in a PR_Open.

Actually, never mind, that's hard.
Comment 10 David Baron :dbaron: ⌚️UTC-10 2004-08-03 02:49:14 PDT
But I'll attach a slightly improved version of this patch to bug 231083 tomorrow.
Comment 11 Andrew Schultz 2004-08-03 07:00:20 PDT
(In reply to comment #8)
> Did you test that PR_TRUNCATE causes the permissions to be used if the file
> exists?  Based on the documentation of open(2), I'd guess that it wouldn't do so.

The documentation itself is ambiguous, but it worked ok when I actually tested
it (I can't retest right now).  It might not be portable though.
Comment 12 David Baron :dbaron: ⌚️UTC-10 2004-08-03 12:15:32 PDT
It doesn't seem portable to Fedora Core 2.  I just did the following test.  I
added this to a test program:

PR_Open("/tmp/mozpermtest", PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0700);

ran the test program, and it created /tmp/mozpermtest with 0700 (my umask is 002).

Then I changed the line to this:

PR_Open("/tmp/mozpermtest", PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0666);

and ran the test program again, and the file was still 0700.
Comment 13 David Baron :dbaron: ⌚️UTC-10 2004-08-16 18:44:47 PDT
Fixed by checkin of patch on bug 231083 to trunk, 2004-08-16 17:12 -0700.
Comment 14 David Baron :dbaron: ⌚️UTC-10 2004-08-17 13:26:49 PDT
Fixed by checkin of bug 231083 to MOZILLA_1_7_BRANCH, 2004-08-17 13:24 -0700.
Comment 15 Christian :Biesinger (don't email me, ping me on IRC) 2004-09-07 16:46:17 PDT
Comment on attachment 142413 [details] [diff] [review]
add PR_TRUNCATE and workaround for standalone

so this review request is obsolete, right?

Note You need to log in before you can comment on or make changes to this bug.