User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021212 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021212 If the temporary directory ($TMP & $TEMP) used by the download manager uses encryption (EFS) then the download fails if the destination directory is not an NTFS folder. This eror results: "xxxfile could not be saved, because an unknown error occurred. Sorry about that. Try saving to a different location." The problem is due to the fact that when you move a file from EFS, Windows requires that the destination folder resides on a file system that supports EFS (i.e. NTFS). Reproducible: Always Steps to Reproduce: 1. Encrypt $TMP folder. It needs to be on NTFS file system. 2. Download any file to FAT16 or FAT32 file system. Download fails because FAT16 and FAT32 don't support EFS 3. Download to NTFS; download succeeds because encryption attributes from temporary file is retained in the destination NTFS. 4. Do not encrypt directory as in 1. above, then problem does not exist. Actual Results: step 2. results in an error: "xxxfile could not be saved, because an unknown error occurred. Sorry about that. Try saving to a different location." Expected Results: Store temporary file in the same directory as destination and rename/move when download is completed.

Related to (and would be fixed by) bug 69938. Not marking as dupe since there might be other ways to fix this too.

This is actually a networking bug, I would say. nsIFile should handle this transparently to consumers....

Only downloading to the destination would lose the performance benefit of downloading while the user is choosing the destination. If Windows' move function doesn't want to work, we could simply do it ourselves: 1. Create file on FAT32. 2. Read and write blocks of data from temp file to destination. 3. Delete temp file.

Hello, I have submitted a patch to fix this issue. CopyFileEx can use the flag COPY_FILE_ALLOW_DECRYPTED_DESTINATION. This means that If windows cannot encrypt the file on the destination volume it will store it on the disk unencrypted(See http://msdn.microsoft.com/en-us/library/aa363852(VS.85).aspx for more detail) I do not have a FAT32 drive to test this patch on. Thanks, Steve

Sorry this is the correct patch, the previous patch contained some changes from a previous defect i was working on.

Comment on attachment 442951 [details] [diff] [review] A patch that uses CopyFileW with the COPY_FILE_ALLOW_DECRYPTED_DESTINATION flag to perform the move. Hey Steve, thanks for the patch. In Mozilla all patches have to be reviewed by a module owner or peer so I've asked Benjamin Smedberg to review this as he is the owner for the xpcom code. I wonder if the patch would be simpler if instead of moving the file differently you just called DecryptFile on it?

Yes you may be correct. Will this function decrypt the file and replace it with the encrypted one? I have not used this function before and there is no parameter you can pass to the function in order to specify where the resultant decrypted file should go. The patch I posted will not change the encryption on the source file, and will keep the file safely encrypted if its copied to another place on the same volume. It will only decrypt the file if the drive does not have EFS.

Comment on attachment 442951 [details] [diff] [review] A patch that uses CopyFileW with the COPY_FILE_ALLOW_DECRYPTED_DESTINATION flag to perform the move. Delegating review to jmathies. We can't mochitest this (since the build slaves use FAT32), but we should probably have a litmus test for this.

Comment on attachment 442951 [details] [diff] [review] A patch that uses CopyFileW with the COPY_FILE_ALLOW_DECRYPTED_DESTINATION flag to perform the move. This looks ok to me Steve. Nits however on formatting - please repost sans tab spacing and match up new code with existing formatting. We have a mozilla code formatting guideline doc here - https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide

Hope this patch is ok

http://hg.mozilla.org/mozilla-central/rev/f4f9d45743a8 Thanks for the patch!

You may remove the flag about encryption, and the CopyFileEx altogether. I have tested MoveFileExW. It works fine between file systems, one of which does support encryption, and the other does not. There was no need for the patch. Revert it. You rather introduced one more, and malicious bug. It triggered another bug, which destroys data. See the report, and attachment: https://bugzilla.mozilla.org/show_bug.cgi?id=674742#c38

You patch has one more bug: in case a "move operation" is not requested, you call CopyFile without the flag about encryption. You only "patched" the case for "move". Maybe Windows has patched their MoveFileEx since then. Maybe that it why MoveFileEx now works, and your patch no longer necessary. A user should update one's OS.

I proposed a patch for that. See my attachment to the dependent bug 674742. The summary of patches is in section "Solution".

When I submitted the patch MoveFileExW didn't work as expected when copying files from an encrypted volume. I only made changes to the code when performing a "move operation" as this was all that was required in-order to fix this particular issue. If we can rely on users keeping there OS up to date then we could remove the code.

Then I ask Bugzilla to remove this patch. Before doing so, check if the bug is really gone in all OS. Or simply remove it without checks. If users encounter the bug again, they may file it here. If they won't, it would be a successful check. How do you remove a patch? Do you file a new patch to replace it? Or do you revert to old code? Your patch was a bug in itself. Very poor job. I described it in bug 674742, and in comment #16 above. In short, you failed to: - apply it to both cases (move, and copy). - check, if CopyFileExW actually accepts your flag. You could check this both through documentation, and through a return value. The progress of your patch proved a big failure, and disregard of rules of Bugzilla, which Bugzilla itself pretends to follow meticulously. See bug 674742. There everyone brought up any bureaucratic hurdle to prevent progress. Here someone filed the bug in 2003. After 7 years Steve simply made a patch. Nobody bothered to reproduce the bug. Not even after the 7 years to see if OS is patched. No "automated tests", no "litmus" tests, no tests on your own computers, or anywhere. All the reviewers, and supervisors simply committed the "patch" blindly, without analyzing the code. This is so contrary to my experience. When I submitted the bug 674742, everybody was asking to reproduce, and not doing anything themselves. A look at your code would surely reveal a bug in it. So, you would make it to start with. A look at the code related to my bug report, would spare me the effort and time. With that look they would have to examine your flawed code of a patch again. In both cases nobody bothered to look at the code. Only I did. In your case they simply waved your patch on without any checks. In my case I had to dig the guilty code by myself. If I were to reproduce the bug, I would waste even more years.

(In reply to Andrew from comment #19) > Then I ask Bugzilla to remove this patch. > Before doing so, check if the bug is really gone in all OS. > Or simply remove it without checks. If users encounter the bug again, they > may file it here. If they won't, it would be a successful check. > > How do you remove a patch? > Do you file a new patch to replace it? > Or do you revert to old code? > > Your patch was a bug in itself. > Very poor job. > I described it in bug 674742, and in comment #16 above. > In short, you failed to: > - apply it to both cases (move, and copy). > - check, if CopyFileExW actually accepts your flag. You could check this > both through documentation, and through a return value. > > The progress of your patch proved a big failure, and disregard of rules of > Bugzilla, which Bugzilla itself pretends to follow meticulously. See bug > 674742. > There everyone brought up any bureaucratic hurdle to prevent progress. > > Here someone filed the bug in 2003. > After 7 years Steve simply made a patch. > Nobody bothered to reproduce the bug. Not even after the 7 years to see if > OS is patched. > No "automated tests", no "litmus" tests, no tests on your own computers, or > anywhere. > All the reviewers, and supervisors simply committed the "patch" blindly, > without analyzing the code. > > This is so contrary to my experience. > When I submitted the bug 674742, everybody was asking to reproduce, and not > doing anything themselves. > > A look at your code would surely reveal a bug in it. So, you would make it > to start with. > A look at the code related to my bug report, would spare me the effort and > time. With that look they would have to examine your flawed code of a patch > again. > > In both cases nobody bothered to look at the code. > Only I did. > In your case they simply waved your patch on without any checks. > In my case I had to dig the guilty code by myself. > If I were to reproduce the bug, I would waste even more years. Ouch. So, is this bug actually fixed, or what?