Closed Bug 1519060 Opened 5 years ago Closed 5 years ago

Maintenance doesn't properly detect and replace malformed databases

Categories

(Toolkit :: Places, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: mak, Assigned: mak)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

here https://searchfox.org/mozilla-central/rev/5053031ba7621fa8f63f42de4c204ab3561e4e59/toolkit/components/places/PlacesDBUtils.jsm#117 we look for Cr.NS_ERROR_FILE_CORRUPTED, and that doesn't seem to work as expected.

There are a few mistakes in our .status/.result usage with these errors, also in Sqlite.jsm where sometimes we return a classic XPCOM Exception, and sometimes we return an Error with a .status field. At this point it's likely better to just use Components.Exception everywhere to avoid the pitfall...

Assignee: nobody → mak77
Status: NEW → ASSIGNED
Priority: -- → P1
Blocks: 1410877
Summary: Maintenance doesn't properly detect malformed disk image → Maintenance doesn't properly detect and replace malformed databases
Blocks: 1516926
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/662d6d626c15
Places maintenance doesn't properly replace malformed databases. r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Comment on attachment 9035648 [details]
Bug 1519060 - Places maintenance doesn't properly replace malformed databases. r=Standard8!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: In some cases, when a Places database is corrupt, we are supposed to replace it on the next Firefox startup. This currently doesn't happen, due to a mistake in error handling, so the user is left with a broken database that may cause various issues: non working bookmarks, address bar or history.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: It's complicate because it requires a specially crafted database, the unit test has one.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This just fixes error handling and has no effect on normal functionality of the product

String changes made/needed:

Attachment #9035648 - Flags: approval-mozilla-beta?

Comment on attachment 9035648 [details]
Bug 1519060 - Places maintenance doesn't properly replace malformed databases. r=Standard8!

[Triage Comment]
Yikes. Approved for 65.0b11.

Attachment #9035648 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Is this something we should take on ESR60 also? Patch would need rebasing if so.

Flags: needinfo?(mak77)

Unfortunately on ESR we lack bug 1432583 that is pretty much needed for this fix to work as expected.

Flags: needinfo?(mak77)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: