470578 - corrupt cookies.sqlite prevents db opening

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[[e-mail]]

zzxc sent me a corrupt cookies.sqlite from a user, which i tested against the cookieservice. when we first open the db connection, we check if it returned NS_ERROR_FILE_CORRUPTED and nuke the db; however, in this case it gets through that part but fails later on, in Read(), when we execute a statement to purge the db of expired cookies. (this makes sense, since it's at that point where sqlite actually traverses the whole db.)

this is easy to fix, and should help users who're getting corrupt db problems. it'll be a low-risk fix, so we should probably take it on trunk, 1.9.1, and 1.9.0 branches. patch forthcoming...

Comment 1

[[e-mail]]

Attachment: patch v1

if anything fails with NS_ERROR_FILE_CORRUPT along the way from opening the db to reading in the data, nuke it and create from scratch. (also tweaks the error propagation of ImportCookies(), since a non-existent cookies.txt shouldn't log an error.)

we could be more picky about where we're interested in NS_ERROR_FILE_CORRUPT (e.g. only on db open or Read()), but this is pretty simple and works...

Comment 2

[[:Cww]]

At least 15 cases of locked/corrupt cookies a week leading to weird login issues/error messages or redirect loop messages.

Comment 3

[[e-mail]]

i'm not sure what's causing the locking problem - any ideas? the OS should clean up file locks on app crash, no?

Comment 4

[Shawn Wilsher :sdwilsh]

places has had issues with the database being locked by virus scanners.  Also, since you want an exclusive lock, things could be full of fail...

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 354038 [details] [diff] [review]
patch v1

r=sdwilsh

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 354038 [details] [diff] [review]
patch v1

yeah, looks ok, but the cookieFile reuse makes me twitchy.  I know it's fine, and not in scope anyway, but it feels like some rope that's just looking to hang someone.

Comment 7

[[e-mail]]

Comment on attachment 354038 [details] [diff] [review]
patch v1

thanks mconnor, i'll tweak the var name on checkin.

requesting approval for 1.9.0 and 1.9.1 branches... see comment 2 for upside; this patch basically means that if a user gets a corrupt db for some reason, we'll detect and delete it on startup rather than requiring them to intervene by deleting it themselves. (we can do something to make corruption occur less often too, but that'll be later.)

Comment 8

[[e-mail]]

landed on trunk.

Comment 9

[Daniel Veditz [:dveditz]]

I'd love to fix this common complaint in a 1.9.0 release, but I'd be happier taking it if it's been tested by reasonable numbers of people in a 1.9.1 nightly or, better, beta release.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 354038 [details] [diff] [review]
patch v1

a191=beltzner

Comment 12

[[e-mail]]

landed on 1.9.1 branch. we can let this bake more before taking on 1.9.0.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 354038 [details] [diff] [review]
patch v1

Approved for 1.9.0.8, a=dveditz for release-drivers

Comment 14

[Daniel Veditz [:dveditz]]

Checked in to the 190 branch for dwitte.

Comment 15

[ked2]

I just _started_ to have problems with cookies when this bug was "fixed." Just for the last few days whenever I go to Facebook, Netflix, or any other site in which I previously have always stayed logged in, I am not logged in. I double-checked my preferences and nothing has changed there: it says "accept cookies" and "accept 3rd party cookies."

Comment 16

[[:Cww]]

(In reply to comment #15)
This bug has been "fixed" in the development version of Firefox only.  Unless you're using a nightly build of Firefox (Gran Paradiso or Shiretoko or Minefield), this hasn't been "fixed" in your version of Firefox. The fix will be shipped when the next update ships.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Can we have one of the corrupted cookies.sqlite for QA to test the fix against or some manual method to reproduce the problem since no unit tests were created? I want to verify this for 1.9.0.8.

Comment 18

[Al Billings [:abillings - ex-MoCo]]

Verified that the corrupt db is deleted on startup for 1.9.0.8 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.8pre) Gecko/2009031904 GranParadiso/3.0.8pre and that it isn't for 1.9.0.7.

Comment 19

[Serg Podtynnyi]

Looks like that 3.0.9 broke something in cookies database, after update had problems with cookies storing, only after manual deleting cookies.sqlite file from profile solved this problem. 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.9) Gecko/2009040821 Firefox/3.0.9 (.NET CLR 3.5.30729)

Comment 21

[DD]

Are you sure it's been fixed? I had this problem for some time but recently it just manifested when I first loaded firefox, but if I closed it down and reloaded, it was fine and remembered the login pages correctly. 

However I've just upgraded to 3.0.9 and now it's constantly "forgetting" the login in cookies to several websites. (It remembers some but not others, and it's always the same ones...)

(Windows Vista home premium, AVG 8.0.238, firefox 3.0.9)

Comment 22

[Al Billings [:abillings - ex-MoCo]]

DD, the problem you are describing is not the same issue as this bug.

Comment 23

[Mardeg]

DD, from http://forums.mozillazine.org/viewtopic.php?p=6042015#p6042015
"I went in to AVG, clicked on the resident shield icon and unticked the 'scan tracking cookies' box and lo and behold it fixed the problem immediately."

Comment 24

[Michael McLagan]

Something tells me that this "fix" is what rebroke the problem that I'm having with cookies, specifically https://bugzilla.mozilla.org/show_bug.cgi?id=457321.

I updated from 3.0.8 to 3.0.10 and my problem has re-appeared.  Bugzilla won't let me change the status of this bug, so I am hoping that this gets sent to involved parties anyways.

Comment 25

[[:Cww]]

We're still seeing people with cookie loss but less so than before so I'm going to clear the common-issue flag from this one and file a new bug if needed.