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)

Product:
Core Graveyard
Component:
File Handling
Platform:
DEC
OpenVMS
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla1.1alpha

People

(Reporter: colin, Assigned: law)

References

(
URL
)

Details

(Keywords: helpwanted, Whiteboard: se-radar)

Attachments

(3 files, 1 obsolete file)

How we got into SetPreferredAction the second time
5.32 KB, text/plain
Details
Dialog box with missing information
21.23 KB, image/jpeg
Details
Patch to do the test only on Windows
855 bytes, patch
law
: review+
Details | Diff | Splinter Review
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).
[ok, i give up, how do I put long URLs into bugzilla WITHOUT getting them wrapped???]
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.
URL: http://lxr.mozilla.org/seamonkey/sour...
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?
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.
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).
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?
I'll check that out tomorrow, Bill. Are you able to get a simple helper app working on Linux? Or is it just me?
Bill, this is really sounding like a dup of bug 98084 now. That's good, because I hate OpenVMS specific problems!
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?
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.
>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?
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.
> Another idea: Comment out the call to OnStateChange in SetWebProgressListener. Made no difference. That is, the helper app still does NOT get invoked.
Is SetPreferredAction getting called to reset that field, and who's calling it if so?
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.
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.
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)
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.
> 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!
This is how my "save it or open it" box looked.
> 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...
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
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.
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
> 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?
#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
same file as non-root user. $ su petejc [petejc@tigris CC]$ ./a.out access = -1 [petejc@tigris CC]$
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?
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.
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?
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
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.
> 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...
> 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...
Blocks: 122422
bug 122422 is a very similar problem on BSDI....
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.
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.
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+
Plan is to make the "IsExecutable()" check here (and a similar spot elsewhere) only on non-Unix platforms.
Target Milestone: --- → mozilla1.0
*** Bug 122422 has been marked as a duplicate of this bug. ***
Keywords: nsbeta1
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.
nsbeta1-, per Nav triage team, helpwanted.
Keywords: nsbeta1 → helpwanted, nsbeta1-
Attachment #67203 - Attachment is obsolete: true
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.
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.
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.
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?
That sums up the current state of things, yes.
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+
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.
Mass-moving all Navigator team 1.0 nsbeta1- bugs to 1.1
Target Milestone: mozilla1.0 → mozilla1.1
I can report this bug is fixed for root users of BSD/OS 4.2 with release 0.9.9.
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
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.
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.
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.
*** Bug 136023 has been marked as a duplicate of this bug. ***
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
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.
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.
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.
*** Bug 150145 has been marked as a duplicate of this bug. ***
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
QA Contact: sairuh → nobody
Whiteboard: se-radar
Keywords: qawanted
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: