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
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
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.
er, bug 231083.
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.
Fixed by checkin of patch on bug 231083 to trunk, 2004-08-16 17:12 -0700.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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)
You need to log in before you can comment on or make changes to this bug.