145557 - cookperm.txt / hostperm.1 truncated when disk full

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect, P4)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

This bug is split of 86501. The reporter in that bug cites the file
'cookieperm.txt' being truncated when shutting down after the disk is full. Not
sure if this affects any other cookie related files. The reporter cites the
files as having 0-length after this shut down. In out-of-disk circumstances it
seems that we don't want to write out the file, preferring to lose any settings
from the current session over losing *all* settings.

Comment 1

[Peter Trudelle]

Isn't it cookperm.txt?  If this is dataloss, please set severity to critical,
and nominate for MachV.

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

Oops, you're right. Was just copying out of 86501. cheers. 

Comment 3

[Peter Trudelle]

Nav triage team needs info: is this really losing any data, and if so what is
the tangible impact on the end user?

Comment 4

[Stephen P. Morse]

Although I haven't tested with a full disk, I can well believe that this file 
will get lost.  And the cookies.txt file should suffer the same fate as well.

The impact on the end user is that all the sites for which he has given 
permanent YES or permanent NO for cookie accpetance will be forgotten, and he'll 
have to reply to the cookie-warning-box for those sites all over again.  If he 
has quite a lot of sites stored there so that his surfing has become a pleasant 
experience, he'll once again start being bombarded with annoying warning boxes.  
(Image permissions are kept in the same file, so all those will be lost as 
well.)

And if the cookie file has the same problem (which I'm pretty sure it must), 
then all the preference and state information that has been saved in cookies 
will be lost.

However, on the flip side, this problem is not that serious because few users 
will encounter it.  Even if they are running low on disk space, the fact that 
they will hit the zero limit just when the cookie files are being saved is a 
very unlikely event.

Comment 5

[Samir Gehani]

Nav triage team: nsbeta1+, adt2 rtm

Comment 6

[Jaime Rodriguez, Jr.]

Mass removing adt1.0.0, and adding adt1.0.1 because, we are now on 1.0.1.

Comment 7

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 8

[Jaime Rodriguez, Jr.]

adt1.0.1-, and removing [adt2 RTM], as this has been nsbeta1- by nav triage.

Comment 9

[Darin Fisher]

seems like we might want to have some sort of generic code for "safe" saving. 
other modules must suffer from the same problem.

Comment 10

[Brant Gurganus]

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.

Comment 11

[Sander]

also see bug 169220 - which sees this for mail filters.

Comment 12

[timeless]

.

Comment 13

[Darin Fisher]

hmm... cookperm.txt no longer exists.  now we have hostperm.1 -- and perhaps
this bug still applies?  mvl?

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

Yeah, the code that does the writing is basically the same. And afaik it is the
same for cookies.txt too. And likely other places. Would it be feasible to have
some service to take care making sure not to overwrite until there is a complete
new file?

Comment 15

[Darin Fisher]

maybe a special nsIOutputStream implementation?

Comment 16

[Michiel van Leeuwen (email: mvl+moz@)]

yeah, sounds good to me.
It just needs and extra method to indicate that writing was finshed and that the
old file can be removed (or backupped?) and the new file moved in place.
It only needs to work for local files ofcourse.

Comment 17

[Darin Fisher]

adding to my 1.7b task list.  i'll try to hash out an interface for this.  maybe
it can be a necko component.

Comment 18

[Mike Connor [:mconnor]]

darin, did you ever get anywhere on this, and is it too late for 1.7?

Comment 19

[Darin Fisher]

i haven't made any progress on this unfortunately :(

i'm going to keep this on my 1.7 radar in hopes of finding time to work on it.

Comment 20

[Michiel van Leeuwen (email: mvl+moz@)]

In prefs, there is nsSafeSaveFile.cpp/.h. What if we move that into a more
general place (maybe inside network) and use that from other places that write
out profile information? That way, they won't overwrite old files anymore.
Maybe we can also add the symlink stuff
http://lxr.mozilla.org/seamonkey/source/xpfe/components/bookmarks/src/nsBookmarksService.cpp#5441
has. If bookmarks need special treatment to follow symlinks, why not other files?

Comment 21

[Brendan Eich [:brendan]]

Would like to get traction on this -- darin, can you give guidance to helpers?

/be

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

see bug 246675, which this one depends on.

Comment 23

[Darin Fisher]

brendan: mvl is all over this.  his patch for bug 246675 is exactly what we
want. after that lands, we just need to use nsISafeFileInputStream in more places.

Comment 24

[Brendan Eich [:brendan]]

blocking-aviary1.0, then -- go, mvl!

/be

Comment 25

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: use nsISafeFileOutputStream

Patch makes permission code use the new nsISafeFileOutputStream code. Cookies
already do that.

Comment 26

[[e-mail]]

Comment on attachment 152539 [details] [diff] [review]
use nsISafeFileOutputStream

yeah, go mvl! r=me

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 152539 [details] [diff] [review]
use nsISafeFileOutputStream

a=asa (on behalf of drivers) for checkin to 1.8a2

Comment 28

[Michiel van Leeuwen (email: mvl+moz@)]

checked in