Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message list too short after delete/move, old messages missing #4245

Closed
rcubetrac opened this issue Jun 18, 2013 · 24 comments
Closed

Message list too short after delete/move, old messages missing #4245

rcubetrac opened this issue Jun 18, 2013 · 24 comments

Comments

@rcubetrac
Copy link

Reported by cepheid on 18 Jun 2013 05:37 UTC as Trac ticket #1489184

I have RC set to display 50 rows per screen, i.e. 50 messages at a time in the message list. On the very first page, I've then moved/deleted a bunch of messages, so there are fewer than 50 messages in the list. When I refresh the list, there are STILL fewer than 50 messages... in fact, the list is the same sequence of messages it was before (minus the deleted ones).

Even if I compact the mailbox, logout, and log back in, the list for page 1 claims to be showing messages 1-50, but is actually showing far fewer messages. See attached screenshot #1.

The TOTAL message count is correct; the number it claims to be displaying is totally wrong.

Also, a (very probably) related problem: messages on later screens are also now misnumbered. For example, on the "final" page (see second screenshot), only one message is displayed even though it claims to be displaying 13 of them. Moreover, the message displayed is NOT #7801... it is actually #6951. On the "next to final" page, only 4 messages are displayed... they are correct in sequence (#6946-6950), but definitely NOT the messages #7751-7800 that RC claims to be showing.

Logging in with pine or Thunderbird confirms the above -- the messages are displayed in proper sequence but the message list is misbehaving. Likely due to the second bug, any messages older than the one displayed on the last page are also absolutely invisible to RC... pine/Thunderbird show that there are 863 older messages in the mailbox, but RC can't seem to pull them up.

The first error (message list too short) is mostly an interface issue... confusing and wrong, but no data loss. The second error is critical -- I can't access old mail that clearly exists in the mailbox, is visible to other IMAP clients, but is not visible to RC.

Please let me know what kind of IMAP logs you need to diagnose this (as in, after enabling the log, what sequence of steps/clicks you want to see in the log).

Migrated-From: http://trac.roundcube.net/ticket/1489184

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 06:25 UTC

Enable imap_debug and provide the log starting from the moment when you move/delete messages. Provide your config.

@rcubetrac
Copy link
Author

Milestone changed by @alecpl on 18 Jun 2013 06:25 UTC

later => 1.0-beta

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 06:33 UTC

I don't know that this has to do with moving/deleting specifically... it's just that I know I moved some stuff, and now the message list appears shorter. One important thing to note, however, is that even if I delete the RC sqlite DB and start from scratch, the message view list remains too short, i.e. this problem does not appear to be resolved by delete the sqlite DB. (Deleting cookies also had no effect.)

Config file and log coming up.

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 07:04 UTC

IMAP debug log has been attached, along with config files. The IMAP debug log begins when I log into RC, into page 1 of the message list. I don't have caching enabled (at least, I don't think I do) but the log does not show any messages being loaded... this makes me thing RC is caching something. On the other hand, deleting the SQlite DB and starting from scratch doesn't change anything, so if anything is getting cached, I have no idea where.

After logging in, I select a message; it is displayed in the preview pane. I delete the message and wait for the confirmation. The message list is now shorter by one message (and still well under 50 messages). Just in case, I now reload the message list (by clicking on the "Mail" icon in the top right). Message headers are fetched, but the message list still shows the same (reduced) number of messages... nothing close to 50.

Anything pop out as incorrect?

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 07:13 UTC

BTW, I've attached a second piece to the debug log... after the --- LAST PAGE --- line.

Here, I clicked the >| button to get to the last page of the message list, i.e. what SHOULD show the very oldest messages in my INBOX. According to the message list, RC claims to display messages 7701 to 7715 (of 7715). The actual message list shows only three messages, and as you can see from the log, those are NOT messages #1-15, they are messages #868-870. Looking at the mailbox with pine confirms this is the case -- the three displayed messages are indeed 868-870 (date ascending order), so RC is very clearly missing a whole bunch of messages.

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 07:14 UTC

This is wrong:

C: A0005 UID SEARCH RETURN (ALL) ALL
S: * ESEARCH (TAG "A0005") UID ALL 2392076:2403622
S: A0005 OK UID SEARCH completed

You could fix this, if your server allows capabilities list modification, by disabling ESEARCH.

@rcubetrac
Copy link
Author

Status changed by @alecpl on 18 Jun 2013 07:14 UTC

new => closed

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 07:26 UTC

I'll look into disabling this. Could you explain why it's wrong, and is there any other way to fix this or work around this within RC if I cannot disable ESEARCH? I hate to bring up SQmail again since I know RC isn't SQmail (which is exactly why I want to install RC =D), but somehow SQmail is not affected by this issue... does it not use ESEARCH or how else does it work around this?

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 07:30 UTC

There's no option, but you can modify rcube_imap_generic class code to not use ESEARCH capability features.

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 07:34 UTC

Will that reduce or otherwise affect performance or usability?

Could you also please explain why the ESEARCH response is wrong? As in, what is wrong about it? If we can file a bug report with uw-imap then it might be patchable.

Thanks!

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 07:36 UTC

Replying to cepheid:

Will that reduce or otherwise affect performance or usability?

Yes, it has impact on performance, but not very big.

Could you also please explain why the ESEARCH response is wrong? As in, what is wrong about it? If we can file a bug report with uw-imap then it might be patchable.

http://tools.ietf.org/html/rfc4731#section-3.1

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 07:45 UTC

Replying to alec:

http://tools.ietf.org/html/rfc4731#section-3.1
So, I read the relevant section here, but I don't understand what's syntactically wrong about the response that uw-imap is providing. It seems to be giving the range of UIDs that match the search (2392076 to 2403622), but the syntax looks OK according to the RFC. I do agree this adds up to 11,546 messages, which is far more than the number actually in the folder (7000+). Is that the issue, i.e. is the server just returning the wrong UIDs, which is confusing RC? (I wonder if SQmail doesn't have this problem because it doesn't use ESEARCH, or if they built specific workarounds...)

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 07:49 UTC

It should not return a range, but message UIDs in compact form.

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 07:51 UTC

Hmm... maybe we could add a workaround. We can check if it returns more messages than EXISTS.

@rcubetrac
Copy link
Author

Status changed by @alecpl on 18 Jun 2013 07:51 UTC

closed => reopened

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 07:59 UTC

Replying to alec:

There's no option, but you can modify rcube_imap_generic class code to not use ESEARCH capability features.
I just tried this, and presto, it resolved this issue completely. For anyone else wishing to implement this workaround prior to an official one, edit $RC_ROOT/program/lib/Roundcube/rcube_imap_generic.php and comment out 1642... place "$esearch = false;" on the next line. (Alec, maybe you do want to make this a config option for the next release?)

Replying to alec:

It should not return a range, but message UIDs in compact form.
Hmm, example #2 in the RFC you listed suggests it could return ranges... that is for a different search but it does have a "2,10:11" ... is that kind of response, using a range, not valid for the ALL search, though?

Replying to alec:

Hmm... maybe we could add a workaround. We can check if it returns more messages than EXISTS.
If true, would you then run a regular search as if esearch was disabled, or would you do something else?

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 08:22 UTC

Replying to cepheid:

Hmm, example #2 in the RFC you listed suggests it could return ranges... that is for a different search but it does have a "2,10:11" ... is that kind of response, using a range, not valid for the ALL search, though?

These must be range(s) without holes.

If true, would you then run a regular search as if esearch was disabled, or would you do something else?

I think my idea might not work, if UW-IMAP also has a wrong response to searches with criteria. Here's my proposed patch:

--- a/program/lib/Roundcube/rcube_imap_generic.php
+++ b/program/lib/Roundcube/rcube_imap_generic.php
@@ -1663,7 +1663,21 @@ class rcube_imap_generic
             $response = null;
         }

-        return new rcube_result_index($mailbox, $response);
+        $index = new rcube_result_index($mailbox, $response);
+
+        // Workaround for invalid ESEARCH implementation in UW-IMAP
+        if ($esearch && $criteria == 'ALL' && $this->data['EXISTS'] != $index->count()) {
+            list($code, $response) = $this->execute($return_uid ? 'UID SEARCH' : 'SEARCH',
+                array($criteria));
+
+            if ($code != self::ERROR_OK) {
+                $response = null;
+            }
+
+            $index = new rcube_result_index($mailbox, $response);
+        }
+
+        return $index;
     }

So, I'm closer to wontfix now.

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 08:30 UTC

Replying to alec:

Replying to cepheid:
I think my idea might not work, if UW-IMAP also has a wrong response to searches with criteria.
I will try to test this patch tomorrow. I don't know if I can test it fully robustly, so I don't know if it'll be a complete workaround even if it seems to solve the problem. On the other hand, as I mentioned, disabling ESEARCH completely did seem to solve this particular problem (but did NOT solve #1489185, unfortunately ... that seems to be a different problem).

When testing, should I apply this patch directly, or is there something else I should do before/during testing? Let me know if you change the code before I test tomorrow. =)

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 08:32 UTC

Test messages search in Roundcube.

@rcubetrac
Copy link
Author

Comment by cepheid on 18 Jun 2013 08:36 UTC

Er... not sure I understand. You mean, use the search box and search for something, e.g. an email address or keyword or some such?

@rcubetrac
Copy link
Author

Comment by @alecpl on 18 Jun 2013 08:40 UTC

Yes.

@rcubetrac
Copy link
Author

Comment by @alecpl on 2 Jul 2013 12:52 UTC

In ed3e51f I implemented a general solution which makes possible to disable any IMAP extension. Set $rcmail_config['imap_disabled_caps'] = array('ESEARCH') to fix this issue.

@rcubetrac
Copy link
Author

Status changed by @alecpl on 2 Jul 2013 12:52 UTC

reopened => closed

@rcubetrac
Copy link
Author

Comment by cepheid on 3 Jul 2013 03:44 UTC

Do you still want me to test that other patch, since it implements a different fix? That is, both methods avoid this issue, but one deactivates ESEARCH entirely, one only if ESEARCH returns invalid results. I can still test the other patch if you want (I apologize for not doing it yet, I got busy).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant