dovecot-2.2: quota: Fixed quota_transaction_is_over() to handle ...

dovecot at dovecot.org dovecot at dovecot.org
Wed Jul 2 17:38:35 UTC 2014


details:   http://hg.dovecot.org/dovecot-2.2/rev/76d573ec5045
changeset: 17566:76d573ec5045
user:      Timo Sirainen <tss at iki.fi>
date:      Wed Jul 02 20:36:49 2014 +0300
description:
quota: Fixed quota_transaction_is_over() to handle "user is already over quota" case.
If size=0 we didn't return failure. This change also fixes various potential
integer overflows in the check. Added unit test for the function.

diffstat:

 src/plugins/quota/Makefile.am       |  20 ++++++++
 src/plugins/quota/quota-util.c      |  54 ++++++++++++++++++--
 src/plugins/quota/test-quota-util.c |  92 +++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+), 7 deletions(-)

diffs (192 lines):

diff -r 0d4de84a54f0 -r 76d573ec5045 src/plugins/quota/Makefile.am
--- a/src/plugins/quota/Makefile.am	Wed Jul 02 20:34:43 2014 +0300
+++ b/src/plugins/quota/Makefile.am	Wed Jul 02 20:36:49 2014 +0300
@@ -5,6 +5,7 @@
 
 AM_CPPFLAGS = \
 	-I$(top_srcdir)/src/lib \
+	-I$(top_srcdir)/src/lib-test \
 	-I$(top_srcdir)/src/lib-master \
 	-I$(top_srcdir)/src/lib-dict \
 	-I$(top_srcdir)/src/lib-index \
@@ -105,3 +106,22 @@
 	  rm -f $(top_builddir)/src/plugins/quota/rquota.x; \
 	fi; \
 	rm -f rquota_xdr.c rquota.h
+
+test_programs = \
+	test-quota-util
+noinst_PROGRAMS = $(test_programs)
+
+test_libs = \
+	../../lib-test/libtest.la \
+	../../lib/liblib.la
+test_deps = $(noinst_LTLIBRARIES) $(test_libs)
+
+test_quota_util_SOURCES = test-quota-util.c
+test_quota_util_LDADD = quota-util.o $(test_libs)
+test_quota_util_DEPENDENCIES = quota-util.o $(test_deps)
+
+check: check-am check-test
+check-test: all-am
+	for bin in $(test_programs); do \
+	  if ! $(RUN_TEST) ./$$bin; then exit 1; fi; \
+	done
diff -r 0d4de84a54f0 -r 76d573ec5045 src/plugins/quota/quota-util.c
--- a/src/plugins/quota/quota-util.c	Wed Jul 02 20:34:43 2014 +0300
+++ b/src/plugins/quota/quota-util.c	Wed Jul 02 20:36:49 2014 +0300
@@ -391,11 +391,51 @@
 bool quota_transaction_is_over(struct quota_transaction_context *ctx,
 			       uoff_t size)
 {
-	if ((ctx->count_used < 0 ||
-	     (uint64_t)ctx->count_used + 1 <= ctx->count_ceil) &&
-	    ((ctx->bytes_used < 0 && size <= ctx->bytes_ceil) ||
-	     (uint64_t)ctx->bytes_used + size <= ctx->bytes_ceil))
-		return FALSE;
-	else
-		return TRUE;
+	if (ctx->count_used < 0) {
+		/* we've deleted some messages. we should be ok, unless we
+		   were already over quota and still are after these
+		   deletions. */
+		if (ctx->count_over > 0) {
+			if (ctx->count_over > (uint64_t)-ctx->count_used + 1)
+				return TRUE;
+		} else {
+			return TRUE;
+		}
+	} else {
+		if (ctx->count_ceil < 1 ||
+		    ctx->count_ceil - 1 < (uint64_t)ctx->count_used) {
+			/* count limit reached */
+			return TRUE;
+		}
+	}
+
+	if (ctx->bytes_used < 0) {
+		const uint64_t bytes_deleted = (uint64_t)-ctx->bytes_used;
+
+		/* we've deleted some messages. same logic as above. */
+		if (ctx->bytes_over > 0) {
+			if (ctx->bytes_over > bytes_deleted) {
+				/* even after deletions we're over quota */
+				return TRUE;
+			}
+			if (size > bytes_deleted - ctx->bytes_over)
+				return TRUE;
+		} else {
+			if (size > bytes_deleted &&
+			    size - bytes_deleted < ctx->bytes_ceil)
+				return TRUE;
+		}
+	} else if (size == 0) {
+		/* we need to explicitly test this case, since the generic
+		   check would fail if user is already over quota */
+		if (ctx->bytes_over > 0)
+			return TRUE;
+	} else {
+		if (ctx->bytes_ceil < size ||
+		    ctx->bytes_ceil - size < (uint64_t)ctx->bytes_used) {
+			/* bytes limit reached */
+			return TRUE;
+		}
+	}
+	return FALSE;
 }
diff -r 0d4de84a54f0 -r 76d573ec5045 src/plugins/quota/test-quota-util.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/plugins/quota/test-quota-util.c	Wed Jul 02 20:36:49 2014 +0300
@@ -0,0 +1,92 @@
+/* Copyright (c) 2014 Dovecot authors, see the included COPYING file */
+
+#include "lib.h"
+#include "quota-private.h"
+#include "test-common.h"
+
+struct test {
+	uint64_t limit, initial_size;
+	int64_t transaction_diff;
+	uint64_t new_size;
+	bool is_over;
+};
+
+static void test_quota_transaction_is_over(void)
+{
+#define MAXU64 (uint64_t)-1
+#define MAXS64 9223372036854775807LL
+#define MINS64 (-MAXS64 - 1LL)
+	static const struct test tests[] = {
+		/* first test only with new_size=1. these are used for both
+		   count and bytes tests: */
+
+		/* limit,     init,     diff, new */
+		{      1,        0,        0,   1, FALSE },
+		{ MAXU64,   MAXU64,        0,   1, TRUE },
+		{ MAXU64, MAXU64-1,        0,   1, FALSE },
+		{ MAXU64, MAXU64-1,        1,   1, TRUE },
+
+		/* these are for bytes tests: */
+
+		/* limit,   init,   diff,    new */
+		{ MAXU64, MAXU64,      0,      0, FALSE },
+		{ MAXU64, MAXU64-1,    1,      0, FALSE },
+		{ MAXU64-1, MAXU64,    1,      0, TRUE },
+		{ MAXU64-1, MAXU64,    0,      0, TRUE },
+		{ MAXU64-1, MAXU64,   -1,      0, FALSE },
+		{ MAXU64, MAXU64,      0,      1, TRUE },
+		{ MAXU64,      0,      0, MAXU64, FALSE },
+		{ MAXU64,      1,      0, MAXU64, TRUE },
+		{ MAXU64,      0,      1, MAXU64, TRUE },
+		{ MAXU64-1,    0,      0, MAXU64, TRUE },
+		{ MAXU64-1,    0,      0, MAXU64-1, FALSE },
+		{ MAXU64-1,    1,      0, MAXU64-1, TRUE },
+		{ MAXU64-1,    1,     -1, MAXU64-1, FALSE },
+		{ MAXU64, MAXU64,      0, MAXU64, TRUE },
+	};
+	struct quota_transaction_context ctx;
+	unsigned int i;
+
+	test_begin("quota transcation is over (count)");
+	for (i = 0; i < N_ELEMENTS(tests); i++) {
+		if (tests[i].new_size != 1)
+			continue;
+
+		memset(&ctx, 0, sizeof(ctx));
+		ctx.count_used = tests[i].transaction_diff;
+		if (tests[i].initial_size > tests[i].limit)
+			ctx.count_over = tests[i].initial_size - tests[i].limit;
+		else {
+			ctx.count_ceil = tests[i].limit - tests[i].initial_size;
+			i_assert(ctx.count_used < 0 ||
+				 (uint64_t)ctx.count_used <= ctx.count_ceil); /* test is broken otherwise */
+		}
+		test_assert_idx(quota_transaction_is_over(&ctx, 0) == tests[i].is_over, i);
+	}
+	test_end();
+
+	test_begin("quota transcation is over (bytes)");
+	for (i = 0; i < N_ELEMENTS(tests); i++) {
+		memset(&ctx, 0, sizeof(ctx));
+		ctx.count_ceil = 1;
+		ctx.bytes_used = tests[i].transaction_diff;
+		if (tests[i].initial_size > tests[i].limit)
+			ctx.bytes_over = tests[i].initial_size - tests[i].limit;
+		else {
+			ctx.bytes_ceil = tests[i].limit - tests[i].initial_size;
+			i_assert(ctx.bytes_used < 0 ||
+				 (uint64_t)ctx.bytes_used <= ctx.bytes_ceil); /* test is broken otherwise */
+		}
+		test_assert_idx(quota_transaction_is_over(&ctx, tests[i].new_size) == tests[i].is_over, i);
+	}
+	test_end();
+}
+
+int main(void)
+{
+	static void (*test_functions[])(void) = {
+		test_quota_transaction_is_over,
+		NULL
+	};
+	return test_run(test_functions);
+}


More information about the dovecot-cvs mailing list