The default bug view has changed. See this FAQ.

nsLocalFileUnix has "access"(system call) problem on Solaris

VERIFIED FIXED

Status

()

Core
XPCOM
VERIFIED FIXED
11 years ago
7 years ago

People

(Reporter: Alfred Peng, Assigned: Alfred Peng)

Tracking

({verified1.8.1.2})

Trunk
Sun
Solaris
verified1.8.1.2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [need testcase])

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
The behavior of system call access on Solaris is different with Linux. Here is a snippet from the manual of access:
If the process has appropriate privileges, an implementation may indicate  success for X_OK even if none of the execute file permission bits are set.

So for a root account, the return value for access on Solaris is different with Linux. Replace access with stat to solve the problem.

Another issue hasn't been addressed: comment 5 for bug 169506:
Why not just replace access with stat universally?
(Assignee)

Comment 1

11 years ago
Created attachment 237500 [details] [diff] [review]
Patch v1
Assignee: nobody → alfred.peng
Status: NEW → ASSIGNED
Attachment #237500 - Flags: superreview?(scc)
Attachment #237500 - Flags: review?(dougt)

Comment 2

11 years ago
brendan, why access() not stat()?
Because access is faster -- no stat buf to copy out of kernel space to userland.

/be
(Assignee)

Comment 4

11 years ago
The difference behaviors influence the file opening dialog for Firefox under Solaris. With root account or account having root privilege, the dialog disables the "open with" option.

Is there any reference for the access system call to make the implementations coherent?
(Assignee)

Comment 5

11 years ago
dougt, to provide the same functionality with Linux, I made this patch. What's your opinion on it?

Besides that, I'm also a little confused by the evaluation for "isexecutable" here:
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in#430

Since we always make a temp file with permission 600 before we really open it:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1610,
is that redundant for us to evaluate the executable permission again?
(Assignee)

Comment 6

11 years ago
dougt, any advice on the patch? It changes the code for Solaris and makes the output of nsLocalFile::Exists consistent with Linux.

Comment 7

11 years ago
Comment on attachment 237500 [details] [diff] [review]
Patch v1

This is fine.  Feel free to request which ever branches you want to land this on.
Attachment #237500 - Flags: superreview?(scc)
Attachment #237500 - Flags: superreview+
Attachment #237500 - Flags: review?(dougt)
Attachment #237500 - Flags: review+
(Assignee)

Comment 8

11 years ago
Thanks.
Checking in xpcom/io/nsLocalFileUnix.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileUnix.cpp,v  <--  nsLocalFileUnix.cpp
new revision: 1.135; previous revision: 1.134
(Assignee)

Comment 9

11 years ago
Comment on attachment 237500 [details] [diff] [review]
Patch v1

This patch makes the behaviour of nsLocalFile::Exists on Solaris consistent with Linux. It only affects the Solaris platform and with low risk. Ask for the approval for 1.8 branch.
Attachment #237500 - Flags: approval1.8.1.2?

Comment 10

10 years ago
Comment on attachment 237500 [details] [diff] [review]
Patch v1

Approved for 1.8 branch, a=jay for drivers.
Attachment #237500 - Flags: approval1.8.1.2? → approval1.8.1.2+
(Assignee)

Comment 11

10 years ago
Checking in xpcom/io/nsLocalFileUnix.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileUnix.cpp,v  <--  nsLocalFileUnix.cpp
new revision: 1.126.8.2; previous revision: 1.126.8.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.1.2
Resolution: --- → FIXED

Comment 12

10 years ago
hi alfred, since this is solaris based fix, could you test it and verify for us on the 1.8 branch?  If the fix is valid, please change the keywords field from "fixed1.8.1.2" to "verified1.8.1.2"

Thanks!

Updated

10 years ago
Whiteboard: [need testcase]
(Assignee)

Comment 13

10 years ago
Mozilla/5.0 (X11; U; SunOS i86pc; en-US; rv:1.8.1.2pre) Gecko/20070131 BonEcho/2.0.0.2pre.

=>Verified.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 14

10 years ago
I just put a binary file and a .xls file on our internal web. It works fine with root user. Do we still need a test case for that?
Keywords: fixed1.8.1.2 → verified1.8.1.2

Updated

7 years ago
Depends on: 542738
You need to log in before you can comment on or make changes to this bug.