74049 - cookperm.txt destroyed

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect)

Description

[Peter Janes]

With today's .tar.gz build 2001032908 on Linux, every time I launch Mozilla my
cookperm.txt is regenerated from scratch, i.e. all of the cookie and image
permissions that were set previously are destroyed.  The file (which I'll
attach) contains several NULL characters that did not appear before.

New entries can be added during a browser session, but they are removed on the
next browser start.

I have confirmation turned on for both cookies and images, and only allow
cookies and images from the originating site.  Marking severity as 'major'; this
combination of features is (apparently) little-used, but has caused data loss of
all of my browsing preferences for the last 6 months.

Comment 1

[Peter Janes]

Attachment: Corrupted cookperm.txt file, note NULLs on line 2

Comment 2

[Peter Janes]

Tried 2001032905 and had the same problem.  2001032808 is okay.

Comment 3

[Stephen P. Morse]

*** Bug 74170 has been marked as a duplicate of this bug. ***

Comment 4

[Stephen P. Morse]

Seeing this on NT as well.  Setting OS to ALL

Comment 5

[Stephen P. Morse]

Oh, what a horrible bug and such an easy mistake to make!  Here's what happened.

The first line of the generated cookperm.txt file used to read:

   # HTTP Cookie Permission File

As part of the cleanup of the cookie module whereby permissions handling was 
completely divorced from cookies (because it is common to both cookies and 
images), that line was changed to the more generic:

   # HTTP Permission File

This was a good thing.  Unfortunately the line of code that generates this line 
was:

   strm.write("# HTTP Cookie Permission File\n", 30); 

and it was changed to:

   strm.write("# HTTP Permission File\n", 30);

But the 30 designates the number of character in the string being written.  Now 
that we shortened the string the number of characters is no longer 30 but rather 
23.

So the simplest fix is simply changing the 30 to 23.  However this is vulnerable 
to repeated problems in the future if someone again changes the string.  So a 
safer fix would be to define the string on a separate line and then refer to the 
name of the defined string:

   #define PERMISSIONFILE_LINE1 "# HTTP Permission File\n"

   strm.write(PERMISSIONFILE_LINE1, PL_strlen(PERMISSIONFILE_LINE1));

and also make similar changes for the remaining header lines written to the 
file.  And to also make the equivalent changes to the code that writes the 
header lines for the cookies.txt file.

Attaching patches for both fixes (simpler fix versus safer fix) with my 
recommendation being to go with the safer fix.

Comment 6

[Stephen P. Morse]

Attachment: simpler fix

Comment 7

[Stephen P. Morse]

Attachment: safer fix

Comment 8

[Judson Valeski]

r=valeski

Comment 9

[Stephen P. Morse]

cc'ing alecf for super-review

Comment 10

[Stephen P. Morse]

*** Bug 74421 has been marked as a duplicate of this bug. ***

Comment 11

[Alec Flett]

sr=alecf

Comment 12

[Stephen P. Morse]

*** Bug 74472 has been marked as a duplicate of this bug. ***

Comment 13

[Cesar Eduardo Barros]

This should be "dataloss, regression" and soon "mostfreq" too =)

Comment 14

[Stephen P. Morse]

Fix checked in yesterday.  Forgot to close this out.

Comment 15

[Jeffrey Baker]

This is still broken on Linux 2001-04-05-05 build.  The first time I get a
cookie in a session, the cookperm.txt gets blown away.  When I quit the browser,
the file looks fine, and has all the cookie permissions in it.  But it gets
wiped out again if I restart the browser and get a cookie.

It's easy to reproduce: turn on cookie prompting, quit the browser, start the
browser, go to any cookie-heavy site (news.com).

Comment 16

[Stephen P. Morse]

Works fine for me.

Even though I couldn't reproduce the symptoms you just described, they are 
different from the original problem.  The original problem was that the 
cookperm.txt file was being written out in a bad format so it could not be read 
in on subsequent runs.  From what you say, the file is saved in a good format 
and read in fine and everything works ok in that second session until you decide 
to accept a new cookie in that session.

So if you are sure you can reproduce the problem you are describing, then open 
a separate bug report on it -- don't morph this one.  Also be more specific in 
your steps to reproduce.  For example, you say

   turn on cookie prompting, quit the browser

Is that it?  You do nothing else during that first session other than turning on 
cookie prompting?  Do you need to collect any cookie prior to this time?

   go to any cookie-heavy site (news.com)

and do what?  What am I supposed to observe at this time?

Comment 17

[Stephen P. Morse]

Oops, I shouldn't have closed this as works-for-me.  This bug report still 
pertains to the original problem (cookperm.txt destroyed) and not baker's 
ammendment.  So the resolution here should be "fixed".  Let me correct that.

Comment 18

[Stephen P. Morse]

Restoring wfm to fixed

Comment 19

[Akkana Peck]

I'm not following Steve's explanation of why this bug shouldn't be reopened, but
I'm certainly still seeing it in the 4/4 build, and it's driving me crazy.  I've
reopened (un-duped) bug 74472, which may or may not be the same bug but which
has a more general summary, so maybe it's okay to reopen that one.

Comment 20

[Stephen P. Morse]

See bug 74858.  There are two separate bugs here (I didn't realize that before) 
and I don't want to morph one bug into the other.

Comment 21

[Peter Janes]

It appears that this behaviour has reappeared in the last couple of builds; it's
certainly in Linux -sea.tar.gz build 2001041821, and I noticed it in one of the
0421 builds from the weekend (no details, as it's on another workstation I don't
have access to).

Comment 22

[Peter Janes]

And this morning's 2001042308. :(

Comment 23

[Jeffrey Baker]

I haven't noticed on 2001-04-22-09 Linux, but I skipped 21 and 20.

Comment 24

[Stephen P. Morse]

Please open this as a separate bug.  This particular bug report pertains to the 
cookperm.txt file that is saved having bad characters in it.  That was indeed 
fixed.  There was a related bug (bug 74472) whereby a the cookperm.txt file was 
overwritten on the next browser session that used it.  That too was fixed.  Both 
of these were not platform specific.

Sounds like you might have something different which is why I am asking you to 
open a new bug report (rather than morphing either of these two which had very 
distinctive symptoms).  Give a detailed scenerio as to what you did that 
demonstrates why you believe there is a problem.

Comment 25

[Tom Everingham]

verified:  6/27/01 branch and trunk, winNT 4, Linux rh6, mac os9