Bug in quota_get_status
Hi,
the configuration option
lmtp_rcpt_check_quota = yes
didn't work, so I traced down the problem:
quota_get_status (quota_storage.c:89) calls quota_test_alloc (quota.c:1352) with size = 0 bytes, which leads always to a FALSE result in quota_is_over (quota.c:1305).
I've fixed the function quota_is_over by considering ctx->bytes_over and ctx->count_over. See the included patch.
Kind regards,
Franz
-- Franz Knipp, +43 664 3980169 qnipp GmbH, Hauptstraße 54, 7064 Oslip, Österreich http://qnipp.com http://qnipp.com/qnipp.vcf
On 26.06.2014, at 10:28, Franz Knipp
the configuration option
lmtp_rcpt_check_quota = yes
I noticed that too, and my quick&dirty fix was to make quota_get_status() call quota_test_alloc() with size = 1, which fixes the problem as well. See patch below [1].
didn't work, so I traced down the problem:
quota_get_status (quota_storage.c:89) calls quota_test_alloc (quota.c:1352) with size = 0 bytes, which leads always to a FALSE result in quota_is_over (quota.c:1305).
I've fixed the function quota_is_over by considering ctx->bytes_over and ctx->count_over. See the included patch.
I keep wondering why quota_is_over() does not just check ctx->*_over in the first place instead of doing math with ctx->*_used and ctx->*_ceil. It would seem so much easier. So either ctx->*over was added after quota_is_over() had been written, or this is an oversight, or there’s a specific reason the author did not use/trust ctx->*_over and preferred doing it in a more complicated way. Grepping trough the file, I see much more places the the ctx->*_used and ctx->*_ceil get updated compared to ctx->*_over, so that might indicate that the latter is only updated in specific cases, and cannot be trusted under all circumstances. Then again, I just took a short look at the quota code, so this hunch might me completely wrong. Markus [1] --- src/plugins/quota/quota-storage.c.orig 2014-05-24 17:06:44.822308741 +0200 +++ src/plugins/quota/quota-storage.c 2014-05-24 17:06:55.340307810 +0200 @@ -86,7 +86,7 @@ if ((items & STATUS_CHECK_OVER_QUOTA) != 0) { qt = quota_transaction_begin(box); - if ((ret = quota_test_alloc(qt, 0, &too_large)) == 0) { + if ((ret = quota_test_alloc(qt, 1, &too_large)) == 0) { mail_storage_set_error(box->storage, MAIL_ERROR_NOSPACE, qt->quota->set->quota_exceeded_msg); ret = -1;
On 26.6.2014, at 18.10, Markus Gebert markus.gebert@hostpoint.ch wrote:
On 26.06.2014, at 10:28, Franz Knipp franz@qnipp.com wrote:
the configuration option
lmtp_rcpt_check_quota = yes
I noticed that too, and my quick&dirty fix was to make quota_get_status() call quota_test_alloc() with size = 1, which fixes the problem as well. See patch below [1].
This should fix it properly: http://hg.dovecot.org/dovecot-2.2/rev/76d573ec5045 (Requires http://hg.dovecot.org/dovecot-2.2/rev/0d4de84a54f0)
I keep wondering why quota_is_over() does not just check ctx->*_over in the first place instead of doing math with ctx->*_used and ctx->*_ceil. It would seem so much easier. So either ctx->*over was added after quota_is_over() had been written, or this is an oversight, or there’s a specific reason the author did not use/trust ctx->*_over and preferred doing it in a more complicated way. Grepping trough the file, I see much more places the the ctx->*_used and ctx->*_ceil get updated compared to ctx->*_over, so that might indicate that the latter is only updated in specific cases, and cannot be trusted under all circumstances. Then again, I just took a short look at the quota code, so this hunch might me completely wrong.
The problem is that within the same transaction it's possible to add/remove multiple mails. The *_ceil and *_over are set only once at the beginning of the transaction.
Am 2014-07-02 19:40, schrieb Timo Sirainen:
This should fix it properly: http://hg.dovecot.org/dovecot-2.2/rev/76d573ec5045 (Requires http://hg.dovecot.org/dovecot-2.2/rev/0d4de84a54f0)
Ok. Thanks.
When do you plan to release the next stable version (containing these fixes)?
The problem is that within the same transaction it's possible to add/remove multiple mails. The *_ceil and *_over are set only once at the beginning of the transaction.
Thanks for the explanation.
In the case of LMTP, this doesn't matter :-)
-- Franz Knipp, +43 664 3980169 qnipp GmbH, Hauptstraße 54, 7064 Oslip, Österreich http://qnipp.com http://qnipp.com/qnipp.vcf
participants (3)
-
Franz Knipp
-
Markus Gebert
-
Timo Sirainen