Maintenance doesn't properly detect and replace malformed databases

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: mak, Assigned: mak)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox64 wontfix, firefox65 fixed, firefox66 fixed)

Details

Attachments

(1 attachment)

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

Comment 3

3 months ago
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/662d6d626c15
Places maintenance doesn't properly replace malformed databases. r=Standard8

Comment 4

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months 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)
Duplicate of this bug: 1523070
You need to log in before you can comment on or make changes to this bug.