With build 2000102008 (2000-10-20-09-MN6) on US W2K. I tried to save a file from the above URL. It popped up a blank browser window, and the "Downloading" dialog with the radio buttons: [ ] Open using [x] Save to disk When I hit OK, the dialog window goes away and nothing else. I expected to see the "Save As" dialog to pop-up.
Confirmed with mozilla trunk build 102404 on NT4. Over to mscott.
Looks like this is a bug in the file picker code. Sending over to Pavlov. The file in question has a ridiculously long file name. We attempt to invoke the file picker with this as the suggested file name. The file picker croaks when it makes an OS call to GetSaveFileName in nsFilePicker.cpp: result = ::GetSaveFileName(&ofn); The native dialog croaks and returns a cancel code instead of showing the dialog. So we abort the download.
Bill? Any idea here. I'm not a windows expert what-so-ever.
It sounds like Windows doesn't like for the "save to" file name to be an invalid Windows file name. We should probably truncate it somehow when that happens. I can't reproduce the problem at the moment because there doesn't seem to be any files at that URL. Can somebody suggest a way to reproduce it? I'll take this bug but I really need a test case. Would any i-drive site produce similar problems?
This is a nasty bug. The code at: http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsFilePicker.cpp#88 will overrun the fileBuffer if strlen(mDefault.get())>MAX_PATH. Bad stuff happens. Setting target milestone and adding crash keyword. Also, I changed the summary. BTW, I built a test case and will attach it. I think idrive.com is history.
Created attachment 50799 [details] Test case; Watch Out! Trying to save link on this test page will probably crash.
Nominating due to potential security hole. This bug involves a stack buffer overrun and thus permits web content to place arbitrary data (perhaps with restrictions) on the stack. The fix is simple and safe. Note that this fixes the buffer overrun but doesn't fully fix the problem. The file picker passes too long a file name to the OS file picker, which then fails with an "invalid parameter" error. We treat that the same as the user pressing Cancel on the file picker (so nothing happens).
Comment on attachment 50964 [details] [diff] [review] Simple fix sr=mscott Nice catch Bill.
PDT+ pending r=.
Fix checked in on branch. Waiting for trunk to open.
Comment on attachment 50964 [details] [diff] [review] Simple fix r=cmanske
Fixed on trunk.
what i see when i do "save link as" in the attached testcase is a dialog saying "there was an error writing to the target location", and the file is not saved. is this expected behavior? [no crash, though!]
Yes, we ultimate get some other error but the fix only prevents the crash. Perhaps we need another bug to deal with figuring out how to handle this situation?
okay, good to know! however, now when i test this on different platforms i see different results [all commercial builds]: * linux - 2001.10.08.04-0.9.2-branch and 2001.10.10.08-trunk: actually this is the platform i was using wrt my 2001-10-11 12:42 comments. * winNT - 2001.10.08.05-0.9.4-branch and 2001.10.09.09-trunk: after selecting "save link as" from the context menu, no dialog appears. the only thing that happens is that the link turns the active color and gets the dotted focus ring... * mac os 10.1 - 2001.10.11.04-0.9.4-branch and 2001.10.10.20-trunk: i am somehow able to save the link [downloads as a text file]. in any case, no crashes. if this is still expected [in spite of platform diffs], i'll go ahead and vrfy this.
I had only tried Windows (since this was a Win32 file picker problem and the code is Win32 only). What you report is what I saw (and reported above: silent failure). There *should* be a bug for that, too. The behavior on other platforms is an issue with their file pickers. Sounds like Linux may need a bug also (or maybe the message we get is OK?).
I forgot to mention: Great testing! Once again you went above and beyond the call of duty (trying things on all platforms for a Win32 only bug; I didn't even think to see if the other platforms had their own crashers).
Sarah - let's file a new bug as Bill suggests for the items you saw and then mark this one verified. Thanks for the great work.
okay, marking vrfy'd. spun off bug 105151 for the win32 silent failure. spun off bug 105155 for the error encountered on linux. [no problems for mac there. :)]