Closed Bug 725597 Opened 13 years ago Closed 12 years ago

SafeBrowsing fails to update persistent PrefixSet on Windows

Categories

(Toolkit :: Safe Browsing, defect)

13 Branch
x86_64
Windows Vista
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 17

People

(Reporter: gcp, Assigned: gcp)

Details

Attachments

(2 files)

What seems to be happening is that upon the first update, all files are created normally. Upon the second update, saving the sbtore files succeeds but saving the pset files doesn't. The data in memory *is* updated, though. When the user restarts the browser, our corruption detection will see that the sbstore data is not in sync with the pset data, and blow away the database. Bugs are: a) prefixSet->WriteFile(); in Classifier.cpp doesn't have its return value checked b) Windows doesn't allow us to rename into the file (SafeOutputStream) while we have an open read handle to it. This is a known problem that was fixed in HashStore.cpp but missed in LookupCache.cpp c) I think its possible to make a testcase for this, so we should probably do that.
Assignee: nobody → gpascutto
Attached patch Patch. Fix WriteFile on Windows (deleted) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=0ad1f7d95261 Adding a test for this specific failure seems tricky. I'd need to shut down the entire classifier system, afaik. Not sure how to achieve that.
Attachment #595825 - Flags: review?(dcamp)
Attachment #595825 - Flags: review?(dcamp) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Attached patch Patch. Backout (deleted) — Splinter Review
[Approval Request Comment] Backout due to bug 744993: a01cf079ee0b Bug 730247 1a6d008acb4f Bug 729928 f8bf3795b851 Bug 729640 35bf0d62cc30 Bug 726002 a010dcf1a973 Bug 726002 e9291f227d63 Bug 725597 db52b4916cde Bug 673470 173f90d397a8 Bug 673470
Attachment #616610 - Flags: approval-mozilla-central?
Attachment #616610 - Flags: approval-mozilla-aurora?
Attachment #616610 - Flags: approval-mozilla-central? → approval-mozilla-central+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #616610 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Comment on attachment 616610 [details] [diff] [review] Patch. Backout Sorry - thought this bug was reopened because the backout was backed out. My mistake. Approved for Aurora 13 (or Beta 13 if the merge occurs before we land).
Attachment #616610 - Flags: approval-mozilla-aurora- → approval-mozilla-aurora+
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 13 → Firefox 17
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: