1941646 - Disable automatic repairing for corruption in local folders

Status: RESOLVED FIXED

(MailNews Core :: General, defect)

Description

[Kai Engert [:KaiE:]]

In bug 1935331 we implemented automatic repairing for folders that cannot be compacted, because there is unexpected data at the beginning of the message file.

Now, I believe that fix has a risk for dataloss, see the explanation in bug 1935331 comment 57.

I'd like to test if my theory is correct, and if it is, restrict that code to IMAP folders (disable for local folders).

Comment 1

[Ben Campbell]

I don't think there's a risk for dataloss, simply because if the compaction fails, the local repair will also fail.
They both use the same mbox scanning code, and in the case we're talking about, it's an invalid mbox.

So yes, the auto-repair option only works for IMAP.

That said, I'm not sure it's such an issue for local folders.
The problem seems to be that the IMAP folder code in older versions of thunderbird wrote out invalid mbox files (with no "From " separators!).
During normal operation that wasn't a problem, because the message offsets and sizes were used from the database and there was no parsing of the mbox structure: a message was just a raw chunk inside a larger file.
And IMAP repair still worked, because that doesn't involve scanning the mbox file (instead it rebuilds the database from the server).

But if local folders had suffered this problem, then "repair folder" would never have worked, as that does parse the mbox structure.
So I'm pretty confident that particular case is an IMAP-only thing.

Comment 2

[Ben Campbell]

Attachment: Bug 1941646 - Restrict auto-repair-upon-failed-compaction to IMAP. r=#thunderbird-back-end-reviewers

Comment 3

[Kai Engert [:KaiE:]]

I've interactively tested the attached patch, I confirm it prevents automatic repair when compacting a corrupted local folder and shows an error instead. I confirm automatic repair of corrupted storage IMAP folder still works.

Comment 4

[Kai Engert [:KaiE:]]

(In reply to Ben Campbell from comment #1)

I don't think there's a risk for dataloss, simply because if the compaction fails, the local repair will also fail.

The local repair will reset the msf file in my testing, which means the folder will have an empty message list.
Which appears to the user as dataloss.
If the user was previously able to access some messages of that folder, that's no longer possible.

The mbox file on disk is still present.

Thanks for your explanations. Let's hope that users don't experience the kind of corrupted local folders that I used in my testing, but it's still good to have safe handling for that potential scenario.

Comment 5

[Kai Engert [:KaiE:]]

Comment on attachment 9459498 [details]
Bug 1941646 - Restrict auto-repair-upon-failed-compaction to IMAP. r=#thunderbird-back-end-reviewers

[Approval Request Comment]
Regression caused by (bug #): 1935331
User impact if declined: potentially treatment of corrupted local folders could result in less accessible data
Testing completed (on c-c, etc.): manually
Risk to taking this patch (and alternatives if risky): low, it limits the code from bug 1935331 to the IMAP scenario, restoring the local folder scenario to the previous logic

Comment 6

[Kai Engert [:KaiE:]]

Attachment: 1941646-esr128-include.patch

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1d991e485305
Restrict auto-repair-upon-failed-compaction to IMAP. r=kaie

Comment 8

[Corey Bryant]

Comment on attachment 9459498 [details]
Bug 1941646 - Restrict auto-repair-upon-failed-compaction to IMAP. r=#thunderbird-back-end-reviewers

[Triage Comment]
Approved for beta

Comment 9

[Daniel Darnell [:dandarnell]]

Thunderbird 135.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/bc8e23b6cc55

Comment 10

[Kai Engert [:KaiE:]]

Comment on attachment 9459498 [details]
Bug 1941646 - Restrict auto-repair-upon-failed-compaction to IMAP. r=#thunderbird-back-end-reviewers

[Approval Request Comment]
Regression caused by (bug #): 1935331
User impact if declined: Required to complete the intended behavior of bug 1935331
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): Required when landing 1935331

Comment 11

[Corey Bryant]

Comment on attachment 9459498 [details]
Bug 1941646 - Restrict auto-repair-upon-failed-compaction to IMAP. r=#thunderbird-back-end-reviewers

[Triage Comment]
Approved for esr128

Comment 12

[Corey Bryant]

Comment on attachment 9459612 [details] [diff] [review]
1941646-esr128-include.patch

[Triage Comment]
Approved for esr128

Comment 13

[Daniel Darnell [:dandarnell]]

Thunderbird 128.7.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/254216cd29b3