Closed
Bug 115819
Opened 23 years ago
Closed 22 years ago
You have chosen to download a file of type: "#1" [#2] from
Categories
(Core Graveyard :: File Handling, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.1alpha
People
(Reporter: colin, Assigned: law)
References
()
Details
(Keywords: helpwanted, Whiteboard: se-radar)
Attachments
(3 files, 1 obsolete file)
There appears to be a race condition that I'm seeing on OpenVMS which is
preventing helper apps from getting created.
nsExternalAppHandler::OnStopRequest calls ExecuteDesiredAction,
ExecuteDesiredAction calls OpenWithApplication, and its OpenWithApplication
which (eventually) results in the helper app getting fired up.
But, ExecuteDesiredAction does NOT call OpenWithApplication unless
mProgressWindowCreated is set, and stepping through with the debugger I am
getting into ExecuteDesiredAction without mProgressWindowCreated being set.
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperApp
Service.cpp#1071
I can't find anywhere else in the code where ExecuteDesiredAction
or OpenWithApplication is called.
So is this a timing bug that's only hitting OpenVMS, or is
mProgressWindowCreated ALWAYS meant to be set?
If I manually force mProgressWindowCreated to be set, then the helper app DOES
get created (and I also get a "Save As" dialog box; this is either another
problem I have to track down or caused by my jamming mProgressWindowCreated).
Reporter | ||
Comment 1•23 years ago
|
||
[ok, i give up, how do I put long URLs into bugzilla WITHOUT getting them
wrapped???]
Comment 2•23 years ago
|
||
i'm able to open helper apps, at least with 12/17 verif bits on linux.
Keywords: qawanted
either use the url field, or use a browser that is semi ignorant of the evil
word wrap features.
Reporter | ||
Comment 4•23 years ago
|
||
Bill, if you are not the best person to help me debug this, can you re-assign to
someone who would be? I'd like to try and get to the bottom of this problem, but
need some help. Thanks, Colin.
Colin, probably myself or Scott MacGregor can help you with this.
I see ExecuteDesiredAction getting called from SetWebProgressListener. Scott
has comments there that indicate that code may be dealing with your situation,
or a related one. It looks like the code there
(http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAp
pService.cpp#686) is trying to handle the case where you've gone through
OnStopRequest before the progress dialog has come up.
I think the strategy is:
2. In OnStartRequest, initiate the download (to temporary file; actually, this
is already underway). Unless the user has said "don't ask", open helper app
dialog and ask user whether to save or open.
3. If user said "don't ask", then OnStartRequest calls either SaveToDisk or
LaunchWithApp directly.
4. Helper app dialog says one of:
a. Cancel (no problem)
Note: Step 3 falls through to one of these next two:
b. Save (calls SaveToDisk, which opens progress dialog);
c. Open (calls LaunchWithApplication, which opens progress dialog).
5. If progress dialog was opened, calls SetWebProgressListener. If
OnStopRequest has already occurred, that calls ExecuteDesiredAction.
6. Otherwise, when OnStopRequest comes in, ExecuteDesiredAction.
I'm not sure I can see where this breaks down. Do you? I'll have to think
about it more carefully. Maybe you can insert breakpoints to determine what's
going on?
Reporter | ||
Comment 6•23 years ago
|
||
I put some print statements into the code. They should be self-explanatory.
Here's what I got.
CRB: Entered ExecuteDesiredAction
CRB: mProgressWindowCreated=0
CRB: mCanceled=0
CRB: Leaving ExecuteDesiredAction
Save to dialog appears. I click on SAVE and then:
CRB: Entered SetWebProgressListener
CRB: mStopRequestIssued=1
CRB: aWebProgressListener=63699488
CRB: About to call ExecuteDesiredAction
CRB: Entered ExecuteDesiredAction
CRB: mProgressWindowCreated=1
CRB: mCanceled=0
CRB: Inside first if statement
CRB: action=0 (saveToDisk)
CRB: after GetPreferredAction, action=0
CRB: About to call MoveFile
CRB: Leaving ExecuteDesiredAction
So it looks like the "problem" is that the action is saveToDisk, even though
I have defined a helper app for the MIME type I'm downloading (PDF):
If I stop in ExecuteDesiredAction
DBG> ex *this
*nsExternalAppHandler::ExecuteDesiredAction::this: class nsExternalAppHandler
inherit nsIStreamListener
inherit nsIRequestObserver
inherit nsISupports
__vptr: 16480920
__vptr: 16480920
__vptr: 16480920
inherit nsIHelperAppLauncher
inherit nsISupports
__vptr: 16088408
__vptr: 16088408
inherit nsIURIContentListener
inherit nsISupports
__vptr: 16096648
__vptr: 16096648
inherit nsIInterfaceRequestor
inherit nsISupports
__vptr: 16094472
__vptr: 16094472
mRefCnt: 3
_mOwningThread: 73671520
mTempFile: class nsCOMPtr<nsIFile>
mRawPtr: 92355696
mSourceUrl: class nsCOMPtr<nsIURI>
mRawPtr: 91394112
mTempFileExtension: class nsCString
inherit nsAFlatCString
inherit nsASingleFragmentCString
inherit nsACString
__vptr: 66530552
__vptr: 66530552
__vptr: 66530552
inherit nsStr
mLength: 4
mCapacity: 4
union
[Displaying union member number 1]
mStr: 92368832
mCharSize: 0
mOwnsBuffer: 1
__vptr: 66530552
mMimeInfo: class nsCOMPtr<nsIMIMEInfo>
mRawPtr: 92354200
mOutStream: class nsCOMPtr<nsIOutputStream>
mRawPtr: 0
mWindowContext: class nsCOMPtr<nsISupports> (has no non-static data
members)
inherit nsCOMPtr_base
mRawPtr: 89878792
mSuggestedFileName: class nsString
inherit nsAFlatString
inherit nsASingleFragmentString
inherit nsAString
__vptr: 66530200
__vptr: 66530200
__vptr: 66530200
inherit nsStr
mLength: 42
mCapacity: 42
union
[Displaying union member number 1]
mStr: 92352744
mCharSize: 1
mOwnsBuffer: 1
__vptr: 66530200
mCanceled: 0
mReceivedDispostionInfo: 0
mStopRequestIssued: 1
mProgressWindowCreated: 0
mTimeDownloadStarted: 1010503283078830
mFinalFileDestination: class nsCOMPtr<nsIFile>
mRawPtr: 0
mDataBuffer: 84394184
mLoadCookie: class nsCOMPtr<nsISupports> (has no non-static data members)
inherit nsCOMPtr_base
mRawPtr: 92348992
mWebProgressListener: class nsCOMPtr<nsIWebProgressListener>
mRawPtr: 0
mOriginalChannel: class nsCOMPtr<nsIChannel>
mRawPtr: 92340680
mDialog: class nsCOMPtr<nsIHelperAppLauncherDialog>
mRawPtr: 92471864
__vptr: 16480920
DBG>
And inside GetPreferredAction:
DBG> ex *this
*nsMIMEInfoImpl::GetPreferredAction::this: class nsMIMEInfoImpl
inherit nsIMIMEInfo
inherit nsISupports
__vptr: 16343912
__vptr: 16343912
mRefCnt: 1
_mOwningThread: 73671520
mExtensions: class nsCStringArray
inherit nsVoidArray
mImpl: 92367048
__vptr: 66517144
__vptr: 66517144
mDescription: class nsAutoString
inherit nsString
inherit nsAFlatString
inherit nsASingleFragmentString
inherit nsAString
__vptr: 66515240
__vptr: 66515240
__vptr: 66515240
inherit nsStr
mLength: 3
mCapacity: 63
union
[Displaying union member number 1]
mStr: 92354240
mCharSize: 1
mOwnsBuffer: 0
__vptr: 66515240
mBuffer: "p"
__vptr: 66515240
mURI: class nsCOMPtr<nsIURI>
mRawPtr: 0
mMacType: 1415071060
mMacCreator: 1954051188
mMIMEType: class nsCString
inherit nsAFlatCString
inherit nsASingleFragmentCString
inherit nsACString
__vptr: 66530552
__vptr: 66530552
__vptr: 66530552
inherit nsStr
mLength: 15
mCapacity: 15
union
[Displaying union member number 1]
mStr: 92368480
mCharSize: 0
mOwnsBuffer: 1
__vptr: 66530552
mPreferredApplication: class nsCOMPtr<nsIFile>
mRawPtr: 92354776
mPreferredAction: 0
mPreferredAppDescription: class nsString
inherit nsAFlatString
inherit nsASingleFragmentString
inherit nsAString
__vptr: 66530200
__vptr: 66530200
__vptr: 66530200
inherit nsStr
mLength: 4
mCapacity: 4
union
[Displaying union member number 1]
mStr: 92368544
mCharSize: 1
mOwnsBuffer: 1
__vptr: 66530200
__vptr: 16343912
DBG>
I can see an access() call to the file I've defined as my helper app, so I know
the MIME stuff is set up correctly.
I have a lunch appointment now, but this afternoon I'll try to track down why I
don't have an action defined.
Reporter | ||
Comment 7•23 years ago
|
||
I did a bit more debugging, starting from the access() call to verify that my
helper app exists. I stepped through the code and everything seemed to get set
up correctly for the helper app.
BUT, remember I said that ExecuteDesiredAction is entered twice (the first time
before the "save as" dialog appears, and the second time after I dismiss the
"save as"). Well, the first time in GetPreferredAction is 2 (I put an extra call
in to get this value), but the second time in its 0.
So why, after setting up the application correctly, is it getting lost?
Here's the tracing again, with some extra info:
CRB: Entered ExecuteDesiredAction with this=63065888
CRB: mMimeInfo=63182120
CRB: mMimeInfo->GetPreferredAction=2
CRB: mProgressWindowCreated=0
CRB: mCanceled=0
CRB: Leaving ExecuteDesiredAction
"save as" dialog appears now and is dismissed
CRB: Entered SetWebProgressListener
CRB: mStopRequestIssued=1
CRB: aWebProgressListener=63630648
CRB: About to call ExecuteDesiredAction
CRB: Entered ExecuteDesiredAction with this=63065888
CRB: mMimeInfo=63182120
CRB: mMimeInfo->GetPreferredAction=0
CRB: mProgressWindowCreated=1
CRB: mCanceled=0
CRB: Inside first if statement
CRB: action=0 (saveToDisk)
CRB: after GetPreferredAction, action=0
CRB: About to call MoveFile
CRB: Leaving ExecuteDesiredAction
If I do this in the second call to ExecuteDesiredAction,
mMimeInfo->SetPreferredAction(2)
it works (my PDF viewer gets created).
Reporter | ||
Comment 8•23 years ago
|
||
Maybe I'm doing something wrong. I just set up a simple helper app on my Linux
system for pdf files. Its just a shell script that writes the time to a file.
Anyway, its not getting invoked.
When I set up the helper app I specified a file type of "pdf" and a MIME type of
"application/pdf". That's right, right?
Yes (assuming you meant "file extension" rather than "file type").
It looks like the nsIMIMEInfo's "preferredAction" attribute is changing in
between the two calls to ExecuteDesiredAction.
Might the curious code at
http://lxr.mozilla.org/seamonkey/source/embedding/components/ui/helperAppDlg/nsHelperAppDlg.js#358
somehow be involved?
Reporter | ||
Comment 10•23 years ago
|
||
I'll check that out tomorrow, Bill.
Are you able to get a simple helper app working on Linux? Or is it just me?
Reporter | ||
Comment 11•23 years ago
|
||
Bill, this is really sounding like a dup of bug 98084 now. That's good, because
I hate OpenVMS specific problems!
Assignee | ||
Comment 12•23 years ago
|
||
Colin, my Linux box is out of commision co I can't test this myself. I suspect
there is some sort of timing issue because I gotta believe helper apps are
working on Linux for most people. But maybe its one of those things where
people just don't expect it to work so they don't complain when it doesn't...
How does this bug manifest itself, exactly? You're clicking on a link to the
application/pdf content, presumably. Do you see the "download dialog" (asking
whether to open with application versus save to disk)? If so, is your helper
app displayed in that dialog?
Do you see a progress dialog? Do you get a file-picker dialog?
Reporter | ||
Comment 13•23 years ago
|
||
Bill, I'm not at all sure helper apps are working for a lot of Linux people.
See bug 98084 for other people's tales of woe. With all due respect, I think you
need to get your Linux system up and running.
> How does this bug manifest itself, exactly? You're clicking on a link to
> the application/pdf content, presumably.
Yes.
> Do you see the "download dialog" (asking whether to open with
> application versus save to disk)?
No. All I get is the "save to disk" dialog.
> Do you see a progress dialog?
Yes.
> Do you get a file-picker dialog?
Yes, that's the first thing to appear after I click on the PDF link. This is
what I'm referring to as the "save to disk' dialog.
The second thing to appear, after I dismiss the file picker dialog, is the
progress dialog.
Those two are the only dialogs I see.
Assignee | ||
Comment 14•23 years ago
|
||
>Bill, I'm not at all sure helper apps are working for a lot of Linux people.
>See bug 98084 for other people's tales of woe. With all due respect, I think you
>need to get your Linux system up and running.
Either that, or reassign this bug :-).
My Linux machine is newly refreshed with Redhat 7.2 (thanks to Sarah) and I'll
be building and maybe testing there soon.
But I have one more question: have you unchecked the "always ask me" box for
your helper app entry for this mime type?
It appears you have (otherwise, you'd see the "Downloading..." dialog asking
whether to open with the helper app or save to disk. Do you get different
behavior if you check that box?
After examining this code once again, it looks OK. We go through
OnStartRequest, which calls LaunchWithApplication. That routine sees we haven't
opened the progress dialog so it calls ShowProgressDialog and returns.
Next, the OnStopRequest for the pdf file comes in. That triggers a call to
ExecuteDesiredAction, which doesn't do anything because the progress dialog
hasn't come to life yet.
Then, the progress dialog appears, and (immediately before that), calls
SetWebProgressListener. That code detects that the OnStopRequest has already
fired, so it sends out a OnStateChange call to the dialog and then calls
ExecuteDesiredAction.
I don't see anywhere that the mMIMEInfo.preferredAction attribute can change.
The first thing to do is put a breakpoint in SetPreferredAction to see if that
is getting called. If it isn't, then somebody's stomping on the object and we
need to set a watch breakpoint on the preferredAction field.
But wait a minute...onStateChange() in nsProgressDlg will call
processEndOfDownload() which likely calls closeWindow() which likeliy calls
window.close() which might trigger release of some xpconnect objects, including
the nsHelperAppLauncher and its member nsMIMEInfo. *That* could be a problem;
I'm not sure what other references there are to the helperAppLauncher.
How about adding a printf to the nsHelperAppLauncher and/or nsMIMEInfo dtor?
Assignee | ||
Comment 15•23 years ago
|
||
Another idea: Comment out the call to OnStateChange in SetWebProgressListener.
That will leave the progress dialog hanging, but if your helper app gets called,
then that's a sign that what's happening downstream from there is the cause of
our problem.
Reporter | ||
Comment 16•23 years ago
|
||
> Another idea: Comment out the call to OnStateChange in SetWebProgressListener.
Made no difference. That is, the helper app still does NOT get invoked.
Assignee | ||
Comment 17•23 years ago
|
||
Is SetPreferredAction getting called to reset that field, and who's calling it
if so?
Reporter | ||
Comment 18•23 years ago
|
||
SetPreferredAction is called twice, the first time with a 2, and the
second time with a zero. Both calls occur real early on, right as its
starting the download.
Attached are the call frames from the second call (when its passed zero).
Hopefully this will shed some light.
Reporter | ||
Comment 19•23 years ago
|
||
initDialog in nsHelperAppDlg.js is what's calling saveToDisk with the zero
value (and this is getting called AFTER first call to SetPreferredAction).
Remember, I'm NEVER seeing the "do you want to open it or save it to disk"
dialog.
Reporter | ||
Comment 20•23 years ago
|
||
It seems that the "what to do with the file" (what gets stored in
mPreferredAction in nsMIMEInfoImpl) is decided before nsHelperAppDlg's
initDialog is called. Yet when initDialog is called is calls saveToDisk(0)
which overwrites the information already stored in mPreferredAction.
Is this just a timing issue?
Are the two calls to SetPreferredAction occuring on different threads and its a
race condition?
To prove to myself that this is indeed what's going on I changed
SetPreferredAction so that it will only write into mPreferredAction if the
contents are zero (the ctor sets this to zero so we should be safe).
Now things are kind of working for me. I click on a PDF link, and I get
the "save to" dialog, I click on OK and I get the progress dialog, and
when the download is complete my helper app is fired up. The only part
I'm not sure about is that I never see the "do you want to open it or
save it" dialog. And yes, I went and clicked on the "reset" button in
the Helper Applications prefs panel.
Here's some tracing. I put a printf in the two nsMIMEInfoImpl ctors and
also in SetPreferredAction. Gee, we create more nsMIMEInfoImpl's than I
expected, and there was a ton of them at Mozilla startup time. Is this
normal?
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl noargs ctor (setting to 0)
CRB SetPreferredAction called with 2
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB SetPreferredAction called with 0
CRB Already set to 2 so NOT setting to 0
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
CRB nsMIMEInfoImpl with args ctor (setting to 0)
Assignee | ||
Comment 21•23 years ago
|
||
This is making my head hurt.
I can't understand why you don't see the "Download..." dialog asking whether to
open with the helper app vs. save to disk. The only explanation is that the
file appears to be an executable.
It looks like nsLocalFileUnix simply tests for the executable bit in the file
mode, which is always set. That seems like it would be a problem on all unixes,
though. Oh, I know. I'll bet that implementation returns false if the file
doesn't exist yet, and that's what's different about your environment.
But why is this dialog even being shown? If you have a helper app registered,
and you've unchecked the "always ask me" box, then it won't. Maybe you didn't
uncheck that box?
I think that's what's happening. Something is different on your system so that
the download happens fast enough that the file is created and seems to be
executable. So we force it to save-to-disk. Maybe we need a new
"WillBeExecutedDirectlyOrIndirectlyAndIsCapableOfDoingHarmWhenWeCallLaunch"
method (which is what we're really testing for by using IsExecutable).
I'm not sure what to do about this. Anybody have ideas? We could just avoid
the IsExecutable check on Unix.
BTW, lots of mime infos are created early on to fill the "cache" for default
types like text/html, xul, images, etc.
Reporter | ||
Comment 22•23 years ago
|
||
> I think that's what's happening. Something is different on your system
> so that the download happens fast enough that the file is created and
> seems to be executable. So we force it to save-to-disk.
BINGO!!
access() is getting called with the temporary PDF file name and X_OK. OpenVMS
returns 0 (success) and so the launcher code thinks the file is an executable
and so forces a "save to disk". I hacked my access so that it would return -1
for the temporary PDF file and then I saw the "do you want to open it or save
it" box. **
On OpenVMS access(foo,X_OK) means "do I have execute access to foo?" and most
certainly does NOT mean "is foo an executable file?". On UNIX does X_OK mean
that the file is an executable? I think this test in the applauncher code is
WRONG.
** the "open it or save it" box had #1 and #2 all over the place, instead of PDF
and the name of the helper app. BUT, after I clicked on ADVANCED and came back
to the dialog, most of the #1 and #2 strings were replaced with the correct
information. Sounds like another timing problem to me!
Reporter | ||
Comment 23•23 years ago
|
||
This is how my "save it or open it" box looked.
Comment 24•23 years ago
|
||
> On UNIX does X_OK mean that the file is an executable?
No, it means "do I have execute permissions", just like on VMS....
The code in question is
http://lxr.mozilla.org/seamonkey/source/embedding/components/ui/helperAppDlg/nsHelperAppDlg.js#185
This calls .isExecutable() on the nsIFile.
Now the IsExecutable impls do the following:
Windows/OS2 -- check whether the file extension is in a list of "executable"
extensions
MacOS -- check with LaunchServices on OSX or check the file type is in a list of
executable file types ('APPL' and the like) on Classic
Unix (includes VMS) -- "access(mPath.get(), X_OK) == 0"
So this sounds like a bug in nsLocalFileUnix to me...
Comment 25•23 years ago
|
||
It's not a bug. I've encountered this misconception on unix before.
If you don't own the file, then you can't get access information on it.
If you own the file, you *can* get access info on it.
--pete
Comment 26•23 years ago
|
||
BTW: if you are running as root, every file will return X_OK = 0.
Thus causing ::IsExecutable to return true.
--pete
Pete: On what Unix do you see this?
access(2) returns -EACCESS if you don't have the access permissions you're
asking about, but it has nothing to do with file ownership:
: trunk; id
uid=500(shaver) gid=500(shaver) groups=500(shaver)
: trunk; strace -etrace=access access -w /etc/passwd
access("/etc/passwd", W_OK) = -1 EACCES (Permission denied)
: trunk; strace -etrace=access access -x /etc/passwd
access("/etc/passwd", X_OK) = -1 EACCES (Permission denied)
Boris: if there's a bug here on nsLocalFileUnix, I don't know how you expect to
fix it. I don't know of any portable way to test whether a file would execute
if exec(2)d, without actually trying it. Why are we not allowing "open from
location" on the other platforms, for that matter? IE does, and as an
occasional IE user I find it handy at times.
Pete: wrong again, I fear:
[root@split root]# id
uid=0(root) gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel)
[root@split root]# strace -etrace=access access -x /etc/passwd
access("/etc/passwd", X_OK) = -1 EACCES (Permission denied)
Are there Unix variants that have different access(2) semantics? I'd be
surprised, I'll admit.
Comment 29•23 years ago
|
||
Shaver, I see it on RH Linux and FreeBSD.
access will return X_OK=0 on a file w/ permissions of 644 if you are root.
Try it. I shit you not . . .
--pete
Comment 30•23 years ago
|
||
> I don't know of any portable way to test whether a file would execute
> if exec(2)d, without actually trying it.
Me neither.... otherwise I would have suggested it.
> Why are we not allowing "open from location"
See bug 68279 (that added the code in question). There is a bug to get rid of
or change this check -- bug 106094.
The question remains. What exactly _should_ the semantics of isExecutable() be?
Comment 31•23 years ago
|
||
#include <stdio.h>
#include <unistd.h>
int
main() {
char *file = "foo";
return printf("access = %d\n", access(file, X_OK));
}
Running a.out as root:
Thu Jan 24
root@tigris
~/CC $ touch foo
Thu Jan 24
root@tigris
~/CC $ ls -l foo
-rw-r--r-- 1 root root 0 Jan 24 14:45 foo
Thu Jan 24
root@tigris
~/CC $ gcc -g -Wall access.c
Thu Jan 24
root@tigris
~/CC $ ./a.out
access = 0
Comment 32•23 years ago
|
||
same file as non-root user.
$ su petejc
[petejc@tigris CC]$ ./a.out
access = -1
[petejc@tigris CC]$
Reporter | ||
Comment 33•23 years ago
|
||
The X_OR access test is a test for execute PERMISSION. Where does it say in any
spec that its a test of whether of not you can exec() it?
> Shaver, I see it on RH Linux and FreeBSD.
>
> access will return X_OK=0 on a file w/ permissions of 644 if you are root.
>
> Try it. I shit you not . . .
I _did_ try it, above, but I'll repeat here:
[root@split root]# strace -etrace=access access -x /etc/passwd
access("/etc/passwd", X_OK) = -1 EACCES (Permission denied)
[root@split root]# ls -l /etc/passwd
-rw-r--r-- 1 root root 1283 Jan 7 09:04 /etc/passwd
What do you get from that sequence, as root? Ownership of the file doesn't seem
to matter, as well it should not:
[root@split root]# strace -etrace=access access -x /tmp/shaverfile
access("/tmp/shaverfile", X_OK) = -1 EACCES (Permission denied)
[root@split root]# ls -l /tmp/shaverfile
-rw-r--r-- 1 shaver shaver 0 Jan 24 14:41 /tmp/shaverfile
[root@split root]#
OK, now things are getting wierd:
[root@split tmp]# ./acc
access = -1
[root@split tmp]# su shaver
: tmp; ./acc
access = -1
: tmp; ls -l foo
-rw-r--r-- 1 shaver shaver 0 Jan 24 14:44 foo
What does strace (or the FreeBSD equivalent) report for you when you run your
little program?
Comment 36•23 years ago
|
||
This is on Linux bro . . .
Thu Jan 24
root@tigris
~/CC $ strace -etrace=access access -x /etc/passwd
access("/etc/passwd", X_OK) = 0
Thu Jan 24
root@tigris
~/CC $
Thu Jan 24
root@tigris
~/CC $ ls -l /etc/passwd
-rw-r--r-- 1 root root 1817 Jan 12 16:25 /etc/passwd
Thu Jan 24
root@tigris
~/CC $ su petejc
[petejc@tigris CC]$ strace -etrace=access access -x /etc/passwd
access("/etc/passwd", X_OK) = -1 EACCES (Permission denied)
[petejc@tigris CC]$ glib-config --version
1.2.6
Ah, damn. I forgot about the difference between su and su -. I get the "0"
behaviour as root too, though of course the file isn't executable (bash: ./foo:
permission denied).
I can picture the exception in the syscall code, too. Blah. Sorry about that.
Anyway, I'm still pretty sure that it has nothing to do with file ownership.
access(2) is clearly fraught with peril, but I don't think there's anything else
appropriate.
Reporter | ||
Comment 38•23 years ago
|
||
You can have execute permission on a jpg file, but that doesn't mean you can
exec() it.
ITS THE WRONG TEST!!!!
Yes, Colin, we know. But you might well be able to execute a JPEG file, if the
system is configured "correctly". (My laptop currently is, for example.)
What replacement test do you propose, other than actually attempting to execute it?
Comment 40•23 years ago
|
||
Why not change the permissions of the temp file to 644 in the js logic?
Then this bug will only happen is you are running mozilla as su. ;-)
Attempting to execute the file is not a good thing. Imagine testing
isExecutable() from a loop that is reading recursively all the dir entries. Ouch!
--pete
Reporter | ||
Comment 41•23 years ago
|
||
Why are we even doing this test (beside the fact that it would appear to be
impossible to test correctly)?
The server's told us the mime type, and we know we have a helper app to handle
that mime type. So popping up the "open it or save to disk" dialog makes sense.
Why would we ever want to "force to disk" just because we think we can exec()
the file? I just don't understand what the problem is that we're trying to solve
here.
Comment 42•23 years ago
|
||
> Why are we even doing this test
Bug 68279. We need to warn the user about running possibly-malicious code.
> Why not change the permissions of the temp file to 644
The file is not created in the js and it's created as 644 (or 600 by now, I
think). This is why this is _not_ a problem on Unix builds...
Reporter | ||
Comment 43•23 years ago
|
||
> Bug 68279. We need to warn the user about running possibly-malicious code.
But we're not going to RUN it. We're going to feed it in to the helper app as
data. So what if the file is "executable" (whatever that means)?
Maybe my helper app is hexedit and I want to feed something that's "executable"
into it?
Maybe my helper app is a virus checker...
Comment 44•23 years ago
|
||
bug 122422 is a very similar problem on BSDI....
Comment 45•23 years ago
|
||
This patch fixes the problem for root attachments on BSDI. The nice thing is
that it applies to the javascript source and does not require a recompile.
Comment 46•23 years ago
|
||
I just submitted a javascript patch for attachments by root on BSDI.
As for the current isExecutable code that uses access(), it seems access() is
the wrong function call for this. access() tests permissions more than file
flags. stat() with S_IXUSR seems the proper test to use.
Assignee | ||
Comment 47•23 years ago
|
||
Comment on attachment 67203 [details] [diff] [review]
Fixes attachment problem for root on BSDI
This patch isn't right. If I apply this on windows, and click on a .exe file,
I get the helper app dialog. If I choose to "open with" my virus checker, then
clicking on OK doesn't run the virus checker, it runs the downloaded .exe.
That's no good.
Attachment #67203 -
Flags: needs-work+
Assignee | ||
Comment 48•23 years ago
|
||
Plan is to make the "IsExecutable()" check here (and a similar spot elsewhere)
only on non-Unix platforms.
Target Milestone: --- → mozilla1.0
Assignee | ||
Comment 49•23 years ago
|
||
*** Bug 122422 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 50•23 years ago
|
||
IsExecutable() is only part of the problem. I still think there's a timing
problem here. Here's what I did.
I hacked access() so that it would always return -1 when passed a mode of X_OK.
Now my helper app gets created, but NEVER THE FIRST TIME. The first time (of
each Mozilla session) I always get the dialog box that I posted in attachment
66276 [details]. But second and subsequent attempts are fine.
Comment 52•23 years ago
|
||
Attachment #67203 -
Attachment is obsolete: true
Assignee | ||
Comment 53•23 years ago
|
||
What about Mac?
I think we should do the isExecutable test if both these conditions are met:
1. The platform implements isExecutable() based on the file extension (or
content type, maybe); that is, it can be done before the file is created.
2. Launching the file is potentially harmful (i.e., will execute directly and is
capable of destroying the user's data).
I'm not sure what the deal is on the Mac. A cursory examiniation of
nsLocalFileMac::IsExecutable and ::Launch (at
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileMac.cpp) isn't
enough for me to answer those questions.
Comment 54•23 years ago
|
||
At least on MacOS Classic, the file needs to actually exist (IsExecutable()
calls GetFileType() which will throw NS_ERROR_FILE_NOT_FOUND if the file is not
found). I can't make sense of the OSX code without knowing more about the
mac-specific functions it uses.
Comment 55•23 years ago
|
||
See also bug 116938 for a possible alternate solution to this problem.... We
should decide on exactly what we're doing and that bug highlights some security
issues the current code has even on Windows.
Assignee | ||
Comment 56•23 years ago
|
||
And the Mac code returns "false," then? Which is a lie, for some files. So if
::Launch can run the file, then there might still be a problem in this case.
Does the IsExecutable() check ever pass on Mac?
I'm tempted to OK this patch, since it keeps the same behavior on Windows, is
safe on Linux (because Launch() will never launch the problem directly), and is
safe on Mac, because IsExecutable() always fails, anyway.
Does that sound right?
Comment 57•23 years ago
|
||
That sums up the current state of things, yes.
Assignee | ||
Comment 58•23 years ago
|
||
Comment on attachment 69967 [details] [diff] [review]
Patch to do the test only on Windows
r=law
My new motto: It doesn't have to be perfect, it just has to be better.
Attachment #69967 -
Flags: review+
Reporter | ||
Comment 59•23 years ago
|
||
Even in M0.9.9 where most of this is now fixed, the first time (of each Mozilla
session) I always get the dialog box that I posted in attachment 66276 [details]. But
second and subsequent attempts are fine.
Comment 60•23 years ago
|
||
Mass-moving all Navigator team 1.0 nsbeta1- bugs to 1.1
Target Milestone: mozilla1.0 → mozilla1.1
Comment 61•23 years ago
|
||
I can report this bug is fixed for root users of BSD/OS 4.2 with release 0.9.9.
Comment 62•23 years ago
|
||
Mozilla 0.9.9
Mozilla/5.0 (X11; U; OpenVMS COMPAQ_AlphaServer_DS10_466_MHz; en-US; rv:0.9.9)
Gecko/20020309
I can not get the Adobe PDF helper application to work. The first time I try, I
get the broken dialog.
On subsequent tries, the actual file is downloaded to my default login
directory. and then an error message appears sometimes, sometimes just a blank
screen.
The error message popup is: "/tmp/OVMS_73_DEC_TPU_REF.PDF could not be opened,
because an unknown error occured.
Sorry about that, Try saving to disk first and then opening the file."
I have verified that the PDF helper application is registered and that when run
from the DCL command line it will display the PDF file.
I put some diagnostics in the command file, and it appears that it is not being
run at all.
This is also outputted on the terminal session that Mozilla is being run from:
*access("/tmp/rkpi3gli.PDF",1) = -1 (xok fix)
*access("/tmp/c52vc9an.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/uf45ank9.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/o52zc5an.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/kpab49ur.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/9ab05yrk.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/4tinspyv.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/6749incx.pdf",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/ezchybol.PDF",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/gtu3k96v.PDF",1) = -1 (xok fix)
*access("/tmp/0hyf4dqj.PDF",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/bw12bsd6.PDF",1) = -1 (xok fix)
*access("/tmp",0) = 0
*access("/tmp/zwp6fcpm.PDF",1) = -1 (xok fix)
*access("/tmp",0) = 0
Reporter | ||
Comment 63•23 years ago
|
||
What is the name of the command file you are using as your helper app? Can you
search for that in the file MIMETYPES.RDF (in the _MOZILLA tree in your
SYS$LOGIN). Also search for "pdf". I'd like to see what you have.
Reporter | ||
Comment 64•23 years ago
|
||
The remaining problem in this report is that SOMETIMES the downloading dialog
box doesn't appear correctly - see attachment 66276 [details] - here's the URL in case
http://bugzilla.mozilla.org/attachment.cgi?id=66276&action=view
There is another report of this problem in bug 81190. That bug has been marked
as a duplicate of bug 77850, but I don't think it is.
I'm convinced there is a real timing problem here.
Reporter | ||
Comment 65•23 years ago
|
||
Bug 136023 is reporting the same problem, that sometimes the download dialog box
doesn't appear correctly, and when this happens, the file is not renamed from
its temp name.
The x_ok problem was just one problem. There's another (timing, I think) related
problem still lurking.
Reporter | ||
Comment 66•23 years ago
|
||
*** Bug 136023 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 67•23 years ago
|
||
Changing the summary from "Helper apps don't start up" to "You have chosen to
download a file of type: "#1" [#2] from" since it more accurately describes the
remaining problem here which still needs fixing.
Attachment 66276 [details] (Dialog box with missing information) shows how this bug
presents itself to the user.
Summary: Helper apps don't start up → You have chosen to download a file of type: "#1" [#2] from
Comment 68•23 years ago
|
||
Please, when someone fix this bug, take a look at bug 95828 . It is essentially
the some bug as this one, but on Linux platform.
Reporter | ||
Comment 69•22 years ago
|
||
I think that bug 150145 is another instance of this problem. I will confirm once
I know for sure.
As a workaround, if all you want to do is save a file to disk, shift-click on
the file and this should immediately bring up the "save as" dialog.
Reporter | ||
Comment 70•22 years ago
|
||
I don't know if this means anything, but I thought I'd mention it in case it
does mean something to someone. When the infamous 'You have chosen to download a
file of type: "#1" [#2]' dialog appears, its always in the very top left of the
screen, whereas when the correct downloading box appears its in the center of
the screen.
Reporter | ||
Comment 71•22 years ago
|
||
*** Bug 150145 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 72•22 years ago
|
||
Wahoo. I've finally found this damn bug. Its in the OpenVMS "look like UNIX"
code, and not in the Mozilla code itself. For the record, when access() was
called to make the X_OK check, we were returning -1 to say that the file is not
executable, but NOT setting errno to EACCES. isExecutable was then returning
NS_ERROR_FILE_TARGET_DOES_NOT_EXIST to nshelperappdlg.js, and things went
downhill from there.
If anyone wants a fixed image for Mozilla or CSWB V1.0 let me know via mail.
I'll also see if I can get this fixed image up on the OpenVMS web site.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•22 years ago
|
QA Contact: sairuh → nobody
Whiteboard: se-radar
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•