Timo Sirainen [Fri, 17 Feb 2017 16:32:05 +0000 (18:32 +0200)]
imap: If SEARCH/SORT fails but returns some results, send them to client.
The previous error handling fixes cause SEARCH/SORT to now fail if there
are any problems reading mails. This change makes the commands still
return the best known results, so the IMAP client can still use them,
even though they may not be entirely correct.
Timo Sirainen [Fri, 17 Feb 2017 16:30:51 +0000 (18:30 +0200)]
lib-storage: Fix error handling when searching mails
Only expunge errors and failures caused by lookup_abort should be ignored.
The rest of the mail errors mean that the search result might not be
correct. We'll still run the search as fully as possible, but we just
return an error at the end.
If they're not already cached, the mail is parsed twice: once to get the
message_parts and again to perform the actual search. The searching can
however do the message_parts parsing internally as well.
Timo Sirainen [Fri, 17 Feb 2017 16:19:46 +0000 (18:19 +0200)]
lib-storage: Fix error handling when sorting mails.
All errors were treated the same as if message had been expunged. This
caused potentially wrong results to be sent to the client without any
indication that they're wrong.
Timo Sirainen [Sat, 18 Feb 2017 23:44:21 +0000 (01:44 +0200)]
dsync: Don't assert-crash if duplicate attributes are seen
Just ignore the duplicates. Normally this shouldn't happen, but due to
some bugs for example a Sieve script could be returned once by doveadm_sieve
plugin and another time from mail_attribute_dict.
Timo Sirainen [Sun, 19 Feb 2017 13:47:48 +0000 (15:47 +0200)]
imap: Fix running time in tagged command replies.
The timing information was updated only after command_exec() returned.
Most of the commands were handled within a single command_exec() though,
so at the time when tagline was sent the running_usecs was still zero.
The msecs in ioloop timing was correct though, because it relied only on
the command start timing info.
This mainly prevents losing hooks that were registered by doveadm plugins.
Otherwise what happens is:
- mail_plugins are unloaded and they unregister their hooks
- doveadm plugins (e.g. doveadm_sieve) are NOT unloaded
- mail_storage_deinit() frees all the registered hooks
- next mail_storage_init() initializes all new hooks
- All mail_plugins are loaded and they register again their hooks
- doveadm plugins are NOT re-loaded or re-initialized, so their existing
hooks were lost.
Timo Sirainen [Sun, 19 Feb 2017 12:34:45 +0000 (14:34 +0200)]
imap: Fix sending UID only when necessary on broken FETCHes.
b748f91d0677fffaa2208b39ebb6db3aeb2e937b changed UID to be sent for most
FETCH replies. There was also some existing code that attempted to do this,
but didn't fully work.
So now:
1) If there are no non-buffered replies, the entire FETCH response isn't
sent.
2) If the buffer was already flushed and nothing else was sent, add UID to
reply. The code paths for handling this are differently for
imap_fetch_failure = disconnect-immediately vs others (depending on
imap_fetch_cur_failed() return value).
Timo Sirainen [Tue, 14 Feb 2017 16:48:51 +0000 (18:48 +0200)]
lib-storage: Update vsize header after sync only if sizes are cached.
The result isn't needed yet in that case, so if it's slow to get the sizes
it might as well be delayd until later. This is especially useful when
indexer-worker triggers FTS indexing. The vsizes can be added to index
after the mail is already read for FTS. Without this change the vsize
update would first open all the mails and then the FTS indexing would
open all the mails a second time.
If folder vsize calculation requires opening more than this many mails from
disk (i.e. mail sizes aren't in cache already), return failure and finish
the calculation via indexer process.
Timo Sirainen [Tue, 7 Feb 2017 11:53:52 +0000 (13:53 +0200)]
lib-storage: Don't stop vsize calculation on expunged mails.
I don't know why I added such logic there in the first place. If we just
skip the expunged mails, the end result should still be correct and
usable when cached.
If user received a mail every day, the day_first_uid wasn't being updated.
This caused wrong caching decisions to be made in dovecot.index.cache:
- Accessing >1 week old emails should have changed caching decision from
"tmp" to "yes". This might not have happened, although as long as
day_first_uid[7] pointed to an existing mail and email client accessed
all the mails, this wouldn't have changed anything.
- Cache compression is supposed to drop >1 week old mails when caching
decision is "tmp". Not enough mails were being dropped because
day_first_uid[7] pointed to a much older than 1 week old mails.
Timo Sirainen [Wed, 15 Feb 2017 16:18:46 +0000 (18:18 +0200)]
lib-storage: Don't sync mailbox after undeleting it.
This is useful only when deleting it. With undeletion the syncing isn't
useful and might actually be harmful with mailbox formats that didn't
fully open the mailbox while it was undeleted.
Timo Sirainen [Wed, 15 Feb 2017 21:32:52 +0000 (23:32 +0200)]
mail-log: Add mail_log_cached_only setting.
If enabled, everything except "save" event will log only the fields that can
be looked up from cache. This improves performance if some of the fields
aren't cached and it's not a strict requirement to log them.
It can never be NULL after the previous change: "lib-storage: Always create
mail_save_context.dest_mail".
The code removal in maildir_transaction_save_commit_pre() seemed
potentially dangerous, but I don't think such code path is possible
anymore. Also even if it is, it's probably fine since the mail_free()
is called even earlier than before (although that itself might have
been a problem).
This also removes last traces of code that made it possible to save mails to
mbox without assigning UID to the mail. The previous commit already caused
this, so this is just removing dead code.
This allows removing similar mail_alloc() from storage backends and plugins
that need it.
As a side effect, this changes mbox code to always assign UIDs to saved
mails. This shouldn't be much of a problem, since it happened practically
always already.
These don't check that they're entirely correct as required by HTTP
specifications. They're mainly there as a quick check that if the caller
didn't validate the key/value in any way, we'll crash instead of creating
a potential security hole. (Because with line feeds the attacker could
add extra headers or even entirely new HTTP requests.)