Closed
Bug 115819
Opened 23 years ago
Closed 23 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!
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 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
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.
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?
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 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•23 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•23 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 72•23 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: 23 years ago
Resolution: --- → FIXED
Updated•23 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
•