Messages moved by filter from POP inbox to local folder reappear in POP inbox after repair
Categories
(MailNews Core :: General, defect)
Tracking
(Not tracked)
People
(Reporter: betterbird.project+18, Unassigned)
References
(Regression)
Details
(Keywords: regression)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/130.0.0.0 Safari/537.36 Edg/130.0.0.0
Steps to reproduce:
Messages moved by filter from POP inbox to local folder reappear in POP inbox after repair.
STR:
Create a POP account and a filter that moves messages with subject test to a local folder called test.
Send yourself a test message. This message gets moved to the test folder.
Repair the inbox.
Actual results:
The test message re-appears.
Expected results:
The message shouldn't reappear.
This is a severe fault since if moved messages reappear, users will be very confused and unhappy.
We noticed this in the context of bug 1926832 comment 10.
Tested in 134.0a1 (2024-11-11).
|
Reporter | |
Comment 1•9 months ago
|
||
Mozregression gives:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=92be7cedf121a3a145a8b412e3c7805b1727f542&tochange=2f215a3f359c07fc0b2d75f764635e2c73075ca4
Actual results:
The test message re-appears.
I can confirm this behavior. At my test with my actual daily-use Thunderbird-Daily. All 422 Mails since 2024.04.24 reappear in my Inbox, but don't disappears in the folders, where my filters originally moved them to.
Elder Mails wasn't affected!
Version=134.0a1 BuildID=20241111104211 Win11 pro 23H2
|
|
Reporter | |
Comment 3•9 months ago
|
||
Thanks for the references. Looks like another facet of the problem has been discovered earlier. In bug 1930273 a correct compaction could likely be triggered by deleting some message from the Inbox. We tested that deleting a message and then compacting in fact removes the messages moved by filter since they have no reference in the MSF.
The main issue seems to be that messages that are moved by filter are incorporated into the Inbox in the first place. That doesn't appear to be correct. Previously, such messages were directly incorporated into the target mailbox. Of course assuming the filter ran before the junk classification.
Also, not that moving a message via filter creates a "corrupt mailbox" event each time, see bug 1926832 comment #10.
Toby, can you please assign a resource to this bug (and friends), we consider this bug to be a blocker.
|
|
Reporter | |
Updated•9 months ago
|
|
||
Comment 4•9 months ago
|
||
I think this is the same as Bug 1890253.
The message moved by a filter does NOT get removed from Inbox, which was a surprise, and
actually made my testbench useless because it counted the messages from outside by parsing Inbox file (MBox format).
The test bench did not trust TB's internal counting to make sure the Inbox and folders contained what they should based on the test scenario and check that.
The bug here makes this test scheme useless.
My external counter encountered these "extra" messages and rang false alarms during local tests.
The root of the problem is mentioned as in https://bugzilla.mozilla.org/show_bug.cgi?id=1890253#c17
in that DiscardNewMessage() is called AFTER FinishNewMessage() is called, which was certainly a surprise. I did know that, and BenC was not aware of it obviously, also.
So now the truncation code is never executed in this case.
I am in the process of creating a patch to change the behavior of finite state machne that either let or block the setting of the file size properly after FinishNewMessage(). Now, it blocks the setting of the file size AFTER FinishNewMessage() is called. But
when a filter move is invoked DiscardNewMessage() is called AFTER FInishNewMessage() has been called,
we have to let the file size setting to occur in that case, too.
I am intending to relax the finite state machine to allow that.
However, as Ben noted, the code may not work well in any case for IMAP folder. I don't use IMAP and so I don't know.
https://bugzilla.mozilla.org/show_bug.cgi?id=1890253#c22
My modification to the finite state machine may be surpassed by Ben's better idea in
https://bugzilla.mozilla.org/show_bug.cgi?id=1890253#c24
(The problem is only for mbox obviously, btw. Maildir format is not affected.)
In any case, a bit of programming is necessary.
I think this is the same as Bug 1890253.
I think that bug 1890253, this bug here, and bug 1930273 are all the same.
The message moved by a filter does NOT get removed from Inbox, which was a surprise
Please note that this is a regression of bug 1857450. A message received to a POP inbox and moved by a filter (before junk classification) should NOT be incorporated into the inbox at all. So there is no need to remove it later. Bug 1857450 has accidentally changed that fundamental design principle. Also note bug 1926832 comment #10, that with the appropriate logging, each filter move now causes a "mbox corruption" error, indicating a more severe underlying issue.
In any case, a bit of programming is necessary.
The regression should be analysed in the first place instead of applying any "work around fixes".
|
|
||
Comment 6•9 months ago
|
||
(In reply to Yury from comment #5)
I think this is the same as Bug 1890253.
I think that bug 1890253, this bug here, and bug 1930273 are all the same.
Thank you for listing related bugs.
I suspect so, too.
The message moved by a filter does NOT get removed from Inbox, which was a surprise
Please note that this is a regression of bug 1857450. A message received to a POP inbox and moved by a filter (before junk classification) should NOT be incorporated into the inbox at all. So there is no need to remove it later.
Well, in an ideal world, it should be like this.
HOWEVER, I think what TB did for many years was like this.
(I am not sure about NNTP issue.).
For POP and MBox folder format: After the download is initiated, the following steps happen.
Repeat the following until all the messages are downloaded.
Download a message
ALWAYS append it into Inbox first. <--- this has been a traditional behavior.
AFTER the append, check the filter rules
If there is a move/copy rule,
copy the message content (with the headers and all) into the target folder.
if the rule was a MOVE, then delete the message from the Inbox,
which was done by updating the meta data
AND TRUNCATE Inbox to its former size before this message was appended.
Since early 2024, "AND TRUNCATE Inbox to its former size before this message was appended." has not been executed due to an oversight in new code.
Not many people realized that this truncation was invoked by DiscardNewMessage() call AFTER FinishNewMessage() committed the
just downloaded message in the Inbox.
I was surprised to learn TB called DiscardNewMessage() to truncate Inbox (to forget about the just downloaded message from Inbox) AFTER FinishNewMessage() has been called. It violates the neat start a new message download and either commit or discard the transaction processing (!).
I was thinking of fixing this part because this interferes by local testbench to check pop download and error processing..
Bug 1857450 has accidentally changed that fundamental design principle.
I am not so sure. TB worked as I described for many years if I am not mistaken.
I only realized the latest issue since Ben's new code to fix "From" processing
keeps the truncation from happening when DiscardNewMessage() is called after "MOVE" filter rule.
Also note bug 1926832 comment #10, that with the appropriate logging, each filter move now causes a "mbox corruption" error, indicating a more severe underlying issue.
In any case, a bit of programming is necessary.
The regression should be analysed in the first place instead of applying any "work around fixes".
I was not aware of any corruption issue until I learned of bug 1926832 comment #10.
It sounds the copying to the target folder is done using a stale storeToken, which is bad. (This is true for both MOVE/COPY since MOVE is basically COPY and discard the just appended message in Inbox.)
But in my mind, I think we can separate the two issues.
- Inbox is left with a moved message (which I am thinking of fixing), AND
- copying to the target folder is done with stale storeToken (which is bad).
I will work on fixing the first part locally and see if I can see any strange code regarding the second point.
ALWAYS append it into Inbox first. <--- this has been a traditional behavior.
Thank you for the clarification. You are correct! Looks like comment #3 and hence my repetition in comment #5 was NOT right after all. I've just run an experiment moving a received message to a folder via a filter, and indeed, the file timestamp of the Inbox changes indicating that the message passes through there. I took the liberty to adjust the summary to remove the erroneous part.
I've looked at bug 1890253. There is a path forward (bug 1890253 comment #22), and having another bug for the same issue doesn't help. Once bug 1890253 is done, we can see whether the alleged issue from bug 1926832 comment #10 persists. I'll close this issue here.
Description
•