Closed Bug 590843 (cookiemonster3) Opened 14 years ago Closed 14 years ago

Investigate cookie loss on trunk


(Core :: Networking: Cookies, defect)

Not set



Tracking Status
blocking2.0 --- final+


(Reporter: dwitte, Assigned: dwitte)



(2 files)

There has been anecdotal evidence of a cookie problem recently -- people getting logged out of services. Happens again after clearing cookies. Figuring out how recently this started happening, how often it happens, and how it manifests (mass signout all at once? A trickle?) would be a good start.

There is speculation that this could be Sync related. Turning it off for a while would be a good way to test that theory.
Sync doesn't touch cookies in any way.

I don't know the basis for this theory, but I really don't know how turning off Sync would prove anything.
Sync just handles cookie prefs? I thought it sunc cookies as well.

Seems much less likely as a culprit, then.

beltzer, are restarts (i.e. sessionstore) a part of your routine? Assuming the answer is yes, can you reproduce without restarting?
Alias: cookiemonster3
mconnor: sorry for the red herring; are there any things Sync does synchronize (passwords? history?) which might affect how session cookies work?

dwitte: I restart every day or so for updates, yeah, and know that formerly those were considered part of the same "session" for the purposes of cookies. Did we change that? I know that Lucas and some of the other security folks were indeed agitating to do so.
We didn't change it. I'm thinking it could be something to do with how sessionstore adds session cookies back on restart. It'd be helpful to eliminate that possibility, but it's really just a strawclutch until we get more data...
(Actually, adding zpao to cc, just in case he knows of any recent sessionstore changes that could affect this.)
(In reply to comment #5)
> (Actually, adding zpao to cc, just in case he knows of any recent sessionstore
> changes that could affect this.)

You and Shawn were last to touch the cookie stuff in sessionstore, so you'd probably be able to answer that better than me! We're not doing anything fancy, so it should be pretty obvious if something is wrong there.
I'd like to get a blocking+ for investigation -- this might take a bit of time to figure out, so I'd like to make it a priority, and I think we need an answer here by release time.
blocking2.0: --- → ?
I know someone that has this happening in b4. Updating to b5 didn't help. I'm sure she would be willing to help. FYI, she isn't using sync.
Cool. Put her in touch?
CC'd her. I'll also ping her offline.
I was seeing this happening in 4.0b4, but when I ugraded to 4.0b5 it seemed to solve the issue. However after about a day, started seeing the issue again across all new tabs I opened and in already open tabs. Esp prevalent in Google Apps, where I would notice some of my previously open spreadsheets or docs would have redirected to the login page. Also, my work has a single sign on process for all internal pages, yet I have had to re-log on for every new tab I open. Clearing cookies didn't solve the issue in 4.0b4, but I will try it again now and see if I get a different result.
OK. Getting a log here would be a good first step. Instructions are at ... before you do that, clear all your cookies, then turn on logging and browse around. Once you notice some cookie loss, note what site it was on, then send me the log. (You probably don't want to attach it here, since it might contain personal info.)

I see missing cookies from washingonpost pretty regularly on my home machine, so I will try to provide a cookie log when I get back there. It might take a couple days.
blocking2.0: ? → final+
I've also noticed loss on one or two sites. I've kicked off a cookie log, but it'll probably be a few days before I see anything.
Is anyone here using PB mode, by chance? And even better, has noticed that this occurs after or around a PB mode switch, or starting the browser into PB?
Bug 591447 *might* fix this. Once that lands I'd like to see what happens and mark this as appropriate.
So, fwiw, this seemed to happen to me today. I say seemed, because I woke up and had to log back in to yelp, wikimo, flickr, and gmail, none of whom should have hit legitimate expiry thresholds (assuming the sites refresh them on activity).

Having said that - looking at my list of cookies reveals plenty, including ones I'm sure I haven't refreshed, like airchina cookies from 2 weeks ago. So perhaps the cookies aren't expunged, but are just not being presented? Dunno - wish I had more information to offer. This was after a weekend suspended, and a month boundary rollover (November 1st today) neither of which may have anything to do with anything.
That's actually quite helpful. It indicates to me that it's a problem related to purging, somehow -- since it affects multiple sites at once, and you haven't lost *all* your cookies, which would be a symptom of a corrupt database or somesuch.

(This *could* still be corruption, but that is... somewhat unlikely.)

Since this appears quite sporadic, I'll just go inspect the purge code and see what I can see...
Attached patch patchSplinter Review
I have no idea if this will fix what people are seeing, but I found two bugs in purge code. The test changes make them fairly self-explanatory.
Assignee: nobody → dwitte
Attachment #488124 - Flags: review?(sdwilsh)
Comment on attachment 488124 [details] [diff] [review]

Attachment #488124 - Flags: review?(sdwilsh) → review+
Attached patch part 2Splinter Review
Thanks to Neil for catching the warning on MSVC that fingered the problem. (According to C spec, integer literals are required to be 'int' or larger IIRC; my 64-bit gcc happily promotes them to 64-bit and thus doesn't warn, but other compilers may well not. Like 32-bit mac gcc.)
Attachment #489967 - Flags: review?(sdwilsh)
Comment on attachment 489967 [details] [diff] [review]
part 2

Attachment #489967 - Flags: review?(sdwilsh) → review+
This almost certainly fixes this bug. Marking as such!
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.