Closed Bug 235781 Opened 20 years ago Closed 20 years ago

XPInstall ignores user's umask when installing files

Categories

(Core Graveyard :: Installer: XPInstall Engine, defect)

All
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ajschult784, Assigned: dbaron)

Details

(Keywords: fixed1.7)

Attachments

(1 file, 1 obsolete file)

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.
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
Attached patch dbaron's patch minus a chmod (obsolete) — Splinter Review
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.
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.
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.
Attachment #142404 - Attachment is obsolete: true
Attachment #142413 - Flags: review?(bsmedberg)
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?
Attachment #142413 - Flags: review?(bsmedberg) → review?(leaf)
How does this work around the problem for standalone?  Anyway, I attached a
patch that does this the cheap way to bug 231803.
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.
(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.
But I'll attach a slightly improved version of this patch to bug 231083 tomorrow.
(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.
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.
Assignee: xpi-engine → dbaron
Fixed by checkin of patch on bug 231083 to trunk, 2004-08-16 17:12 -0700.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Fixed by checkin of bug 231083 to MOZILLA_1_7_BRANCH, 2004-08-17 13:24 -0700.
Keywords: fixed1.7
Comment on attachment 142413 [details] [diff] [review]
add PR_TRUNCATE and workaround for standalone

so this review request is obsolete, right?
Attachment #142413 - Flags: review?(leaf)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: