Closed Bug 280419 Opened 15 years ago Closed 12 years ago
When a temporary file is created from the download manager it should be readonly
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 When a download is requested and the user clicks 'Open' the file is saved in the temporary folder. For word documents this allows users to edit and save the file back to disk. When mozilla closes it will delete the temporary file, loosing the changes. The file should be marked readonly so the user can't change the file. This is a particular problem with webmail. Reproducible: Always Steps to Reproduce: 1.Open a word document from the web by clicking the 'Open' button on the download prompt. 2.Edit the document and select 'Save' from the 'File' menu in word. 3.Exit word. 4.Exit mozilla. Actual Results: The file was deleted including the changes. Expected Results: The temporary file should have been saved as readonly - so the user could not modify the file. Ideally this would be done by removing the write permissions for the file.
The webmail package I tried this with was 'horde'. The bug was produced by clicking on the link on the left - not the disk icon - when trying to save an attachment.
*** Bug 280420 has been marked as a duplicate of this bug. ***
15 years ago
Assignee: download-manager → file-handling
Component: Download Manager → File Handling
Product: Mozilla Application Suite → Core
QA Contact: ian
Version: unspecified → Trunk
This is easy to do if we decide it's desirable, fwiw; just change all occurrences of 0600 to 0400 in http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp
bz, that won't work on windows, will it?
Not sure, actually. I thought we set some sort of bits on at least WinXP... But yes, in general we don't really have a way to mark files readonly on Windows that I know of.
related problem for attachments: Bug 220808
This is an automated message, with ID "auto-resolve01". This bug has had no comments for a long time. Statistically, we have found that bug reports that have not been confirmed by a second user after three months are highly unlikely to be the source of a fix to the code. While your input is very important to us, our resources are limited and so we are asking for your help in focussing our efforts. If you can still reproduce this problem in the latest version of the product (see below for how to obtain a copy) or, for feature requests, if it's not present in the latest version and you still believe we should implement it, please visit the URL of this bug (given at the top of this mail) and add a comment to that effect, giving more reproduction information if you have it. If it is not a problem any longer, you need take no action. If this bug is not changed in any way in the next two weeks, it will be automatically resolved. Thank you for your help in this matter. The latest beta releases can be obtained from: Firefox: http://www.mozilla.org/projects/firefox/ Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html Seamonkey: http://www.mozilla.org/projects/seamonkey/
This bug has been automatically resolved after a period of inactivity (see above comment). If anyone thinks this is incorrect, they should feel free to reopen it.
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → EXPIRED
This is a real issue.
Status: RESOLVED → UNCONFIRMED
Resolution: EXPIRED → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
bug 107088 fixed this on non-windows platforms
OS: All → Windows 95
Hardware: All → PC
Not really. The files are still 0600; this bug is asking for them to be 0400.
OS: Windows 95 → All
Hardware: PC → All
I can't see any harm in doing this, as long as it won't muck with delete-on-exit on poorly-behaved filesystems (its been a while, but I remember oddness with deleting readonly things on older Windows, of course none that we're supporting on trunk!)
For what it's worth, I just tried to implement BZ's suggestion from comment #3. It did not work on Windows XP, files were still read/write.
I'm pretty sure the nsIFile file permissions have never worked on Windows. Someone would need to make them work...
This should fix it for others besides windows, I tested it under linux. Filed bug 369108 about the remaining windows issue.
Assignee: file-handling → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #253852 - Flags: review?(cbiesinger)
Comment on attachment 253852 [details] [diff] [review] proposed fix doesn't this also mark normal downloads as readonly? (also, you can't assert that SetPermissions succeeded. if it fails, that's not a programming error.)
No, save to disk is in another if-clause (where FixFilePermissions is also called). A minor caveat is that this patch doesn't affect files handled by plugins (e.g. in one profile I had mozplugger set to handle .doc).
no need for the "rv = " ...
Christian: ping on the review request...
please use something like -pu9 as diff options next time I'd rather you didn't rely on FixFilePermissions to fix up the permissions... it's only implemented on unix, while this code is cross-platform. It seems like this would actually make a difference on Mac and BeOS.
also, sorry for the delay. I was quite busy with a lot of stuff lately.
Hmm, so maybe we should fix FixFilePermissions? The mac implementation would be copy-paste from unix, no? http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/unix/nsOSHelperAppService.cpp#1677 Have no idea about BeOS though.
Ok, maybe we're better off just setting readonly before opening.
Comment on attachment 267284 [details] [diff] [review] proposed fix, v4 So, how about this? At least it's simple:)
12 years ago
Attachment #267284 - Flags: review?(cbiesinger) → review+
Attachment #267284 - Flags: superreview?(dmose)
Comment on attachment 267284 [details] [diff] [review] proposed fix, v4 >+ // make the tmp file readonly so users won't edit it and loose the changes >+ mFinalFileDestination->SetPermissions(0400); >+ Spelling nit: s/loose/lose/ sr=dmose with that fixed.
Attachment #267284 - Flags: superreview?(dmose) → superreview+
Checking in uriloader/exthandler/nsExternalHelperAppService.cpp; /cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v <-- nsExternalHelperAppService.cpp new revision: 1.344; previous revision: 1.343 done I can only confirm this fixes it for linux. Can someone report back for other platforms? Given the approach taken, bug 369108 is not so relevant anymore... ->FIXED, if it doesn't work on your platform, SetPermissions for that platform needs to get fixed to take readonly into account
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 12 years ago
No longer depends on: 369108
Resolution: --- → FIXED
Or... i guess bug 369108 could still be relevant. Depends on how widely you interpret it.
Comment on attachment 267284 [details] [diff] [review] proposed fix, v4 dmose: is this safe for the 1.8 branch?
If it lands on 1.8 bug 410794 should also land there. (And probably bug 401316, but that hasn't been approved for trunk, yet.)
Comment on attachment 267284 [details] [diff] [review] proposed fix, v4 clearing branch approval request until the other bugs mentioned above are resolved one way or another.
This used to work on Windows but now (Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0) the files seem not to be set as read only before opening. Can anyone confirm the regression ?
Sadly yes, filed bug 1062524.
You need to log in before you can comment on or make changes to this bug.