User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:126.96.36.199) Gecko/20070222 SeaMonkey/1.1.1 Mnenhy/0.7.5.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:188.8.131.52) Gecko/20070222 SeaMonkey/1.1.1 I intentionally set my cookies.txt file to read-only. Sites that set cookies cause the browser to change these to read-write. Reproducible: Always Steps to Reproduce: 1. With the browser closed, locate cookies.txt in a profile. 2. Right-click on the file and select Properties in the pull-down context menu. 3. On the Properties window, select the Read-only checkbox and then select the OK button. 4. Start the browser. 5. Start the Cookie Manager. Count the number of cookies. 6. Go to a Web site that is known to set cookies (e.g., the Bugzilla site cited above). 7. Do a query to locate a bug report. 8. Terminate the browser. 9. On the cookies.txt file, right-click and and select Properties in the pull-down context menu. 10. Restart the browser. 11. Start the Cookie Manager. Count the number of cookies. Actual Results: The read-only checkbox is cleared. Cookies have been written to the file. Expected Results: The read-only checkbox should remain checked. No new cookies should have been written to the file. See bug #257288 regarding why some of us want to lock our cookies.txt to read-only. See especially the following comments in that bug report: 12, 24 (last paragraph), and 27. In any case, changing a user's read-only file to read-write without any warning (let alone asking permission) is quite wrong. What is happening to me is that I am losing my preferred settings (contained in cookies) whenever I request a non-preferred operation on certain Web sites, including the Bugzilla site. I have preferred columns and sorting for query reports and other preferred settings. If I change any of them for a single instance, I must then manually restore them the next time I visit Bugzilla. This is a loss of data -- cookie data that control settings -- and is thus a critical bug.
why is this not a duplicate of bug 257288?
This is NOT the same as bug 257288. This bug reports an improper change in the properties of read-only files, to read-write. This is something new within recent Gecko or UI versions and was not happening as recently as 11 November 2006 (the time of the last comment on this issue in bug 257288). Bug 257288 reports that, when a file is read-only, the Cookie Manager writes a new file with the old file's name and a numeric suffix. However, in that bug (dating back almost three years), the properties of the read-write file were not changed. Note that bug 375778 hides the effect of bug 257288 by changing read-only files to read-write. Since read-only files are not found, the erroneous behavior reported in bug 257288 does not occur. This must NOT be considered a fix for bug 257288 since bug 375778 makes things worse instead of better. Also note that, if no software changes were made and bug 375778 was left as a duplicate of bug 257288, then bug 257288 could be closed. After all, the error reported in bug 375778 now eliminates the symptoms described in bug 257288 (as explained in the paragraph above). Reopening this bug report.
From <https://bugzilla.mozilla.org/show_bug.cgi?id=257288#c38>, it appears that this problem does not affect Mac versions.
Agreed, cookies.txt read-only permissions are disrespected by Firefox 2 on my two Windows XP installations, and it is this bug (<a href="show_bug.cgi?id=375778" title="REOPENED - Read-Only Files Changed to Read-Write">375778</a>) which prevents the conditions that would otherwise trigger the 10,000-cookiefiles bug <a href="show_bug.cgi?id=257288" title="NEW - read-only profile files (cookies, bookmarks, etc) not respected, results in data loss">257288</a>. Meanwhile, this permission-disrespect bug does NOT occur on my Mac and Windows 2000 installations, thereby allowing the 10,000-cookiefiles bug to occur.
Ugh, let me try that again... Agreed, cookies.txt read-only permissions are disrespected by Firefox 2 on my two Windows XP installations, and it is this (bug 375778) which prevents the conditions that would otherwise trigger the 10,000-cookiefiles bug 257288. Meanwhile, this permission-disrespect bug does NOT occur on my Mac and Windows 2000 installations, thereby allowing the 10,000-cookiefiles bug to occur.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:184.108.40.206) Gecko/20070509 Netscape/7.2 (ax) Firefox/220.127.116.11 SeaMonkey/1.1.2 Two problems: cookies.txt stops being Read-only and cookies are not removed at end-of-session. I allow cookies based on privacy settings and accept for current session only, which should delete the cookies when I exit my browser. network.cookie.cookieBehavior is set to 3. network.cookie.lifetimePolicy is set to 2. Additionally, I have my cookies.txt set to Read-only. Yet, after going to a site in SeaMonkey that sets cookies, I find that my cookies.txt is no longer Read-only and it has accepted cookies and retained them. They are not deleted on exit. This was not a problem in Mozilla Suite 1.7.13.
-> 1.8 branch since we now use storage on trunk (1.9) and thus have different file writing code.
Created attachment 269507 [details] [diff] [review] don't proceed with overwriting a file which can't be opened for writing This bug still affects all consumers of nsISafeOutputStream.
Comment on attachment 269507 [details] [diff] [review] don't proceed with overwriting a file which can't be opened for writing I'd like to get a second look at this from jmathies.
Comment on attachment 269507 [details] [diff] [review] don't proceed with overwriting a file which can't be opened for writing Certainly an acceptable way of testing write access. Looks like this will only execute when we're trying to move the file, and MoveFileW below it is going to fail anyway if somebody has the file open, so it doesn't look like it'll impact anything. I'm not as familiar with the backend xpcom io stuff as some other folks though, you might want to run this past somebody like Doug Turner. Related - hasn't cookies.txt been replaced with an sqlite db?
It has, recently - https://bugzilla.mozilla.org/show_bug.cgi?id=230933 So even with this patch, your cookie problem might not go away. We might want to revisit this and see if the database implementation has similar problems. e.g. If it aint broke, don't fix it.
Thanks for the reviews. (In reply to comment #12) > So even with this patch, your cookie problem might not go away. Indeed, although this is not exclusively about the cookie problem (that's rather bug 257288), but also applies e.g. to sessionstore.js (cf bug 351551) and other consumers of nsISafeOutputStream <http://lxr.mozilla.org/mozilla/search?string=nsISafeOutputStream>.
Comment on attachment 269507 [details] [diff] [review] don't proceed with overwriting a file which can't be opened for writing I'd still like Doug to take a look.
Comment on attachment 269507 [details] [diff] [review] don't proceed with overwriting a file which can't be opened for writing maybe we should simply use MoveFileEx now that our windows system requirements have been lifted. I believe the extra flags will allow us to remove most of this code. can you also fix the spelling error in the comment block above this where it says "out of the say so". s/say/way
Created attachment 276092 [details] [diff] [review] use MoveFileEx instead Much simpler now.
Comment on attachment 276092 [details] [diff] [review] use MoveFileEx instead I think we are also going to want to pass MOVEFILE_COPY_ALLOWED as an additional flag so that cross volume operations work. And we may want to consider using MOVEFILE_WRITE_THROUGH so that the operation completes before returning. In case, we are probably going to want to add unit tests at this point. Benjamin can advise regarding this point.
Created attachment 276130 [details] [diff] [review] update to comment #17 (In reply to comment #17) > consider using MOVEFILE_WRITE_THROUGH Yep, especially since this code path is used for nsISafeOutputStream. > In case, we are probably going to want to add unit tests at this point. You'll have to look for a different volunteer for these.
Comment on attachment 276130 [details] [diff] [review] update to comment #17 this looks fine. I am concerned about regressions, so we should land early and test. We should also add a few test cases to xpcom/tests/nsIFileTest.cpp for this case. Sadly, copy and move is only tested on Unix. nit: i would have put |destPath.get(),| on its own line.
Comment on attachment 276130 [details] [diff] [review] update to comment #17 Needs approval before checking-in.
10 years ago
dougt, would this be viable for 1.8.1 branch also, or do we still have old windows system requirements there?
Checking in xpcom/io/nsLocalFileWin.cpp; /cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v <-- nsLocalFileWin.cpp new revision: 1.174; previous revision: 1.173 done