# HG changeset patch
# User Timo Sirainen <timo.sirainen@dovecot.fi>
# Date 1503317490 -10800
# Node ID 4b1e95fa17e88409cccb6bcea3dd2fa83ba68d05
# Parent  01f841c0febba0cba08d62f13ac73cc06c678539
lib-storage: Fix error logging after mail_storage_set_internal_error()

The function doesn't actually set the last_internal_error and it also
doesn't update last_error_is_internal, so calling
mail_storage_get_last_internal_error() afterwards could be returning an
error for some different older error.

Since there's no parameter to set the internal error string, just reset
last_error_is_internal=FALSE.

diff -r 01f841c0febb -r 4b1e95fa17e8 src/lib-storage/mail-storage.c
--- a/src/lib-storage/mail-storage.c	Fri Sep 08 12:20:21 2017 +0300
+++ b/src/lib-storage/mail-storage.c	Mon Aug 21 15:11:30 2017 +0300
@@ -529,6 +529,10 @@
 	i_free(storage->error_string);
 	storage->error_string = i_strdup(str);
 	storage->error = MAIL_ERROR_TEMP;
+
+	/* this function doesn't set last_internal_error, so
+	   last_error_is_internal can't be TRUE. */
+	storage->last_error_is_internal = FALSE;
 }
 
 void mail_storage_set_critical(struct mail_storage *storage,
@@ -537,6 +541,12 @@
 	char *old_error = storage->last_internal_error;
 	va_list va;
 
+	storage->last_internal_error = NULL;
+	/* critical errors may contain sensitive data, so let user
+	   see only "Internal error" with a timestamp to make it
+	   easier to look from log files the actual error message. */
+	mail_storage_set_internal_error(storage);
+
 	va_start(va, fmt);
 	storage->last_internal_error = i_strdup_vprintf(fmt, va);
 	va_end(va);
@@ -546,11 +556,6 @@
 	/* free the old_error only after the new error is generated, because
 	   the old_error may be one of the parameters. */
 	i_free(old_error);
-
-	/* critical errors may contain sensitive data, so let user
-	   see only "Internal error" with a timestamp to make it
-	   easier to look from log files the actual error message. */
-	mail_storage_set_internal_error(storage);
 }
 
 const char *mail_storage_get_last_internal_error(struct mail_storage *storage,
diff -r 01f841c0febb -r 4b1e95fa17e8 src/lib-storage/test-mail-storage.c
--- a/src/lib-storage/test-mail-storage.c	Fri Sep 08 12:20:21 2017 +0300
+++ b/src/lib-storage/test-mail-storage.c	Mon Aug 21 15:11:30 2017 +0300
@@ -38,7 +38,18 @@
 	test_assert(mail_error == MAIL_ERROR_TEMP);
 	test_assert(!storage.last_error_is_internal);
 
-	/* internal error without specifying what it is */
+	/* set internal error in preparation for the next test */
+	test_expect_error_string("critical0");
+	mail_storage_set_critical(&storage, "critical0");
+	test_expect_no_more_errors();
+	test_assert(strstr(mail_storage_get_last_error(&storage, &mail_error), MAIL_ERRSTR_CRITICAL_MSG) != NULL);
+	test_assert(mail_error == MAIL_ERROR_TEMP);
+	test_assert(strcmp(mail_storage_get_last_internal_error(&storage, &mail_error), "critical0") == 0);
+	test_assert(mail_error == MAIL_ERROR_TEMP);
+	test_assert(storage.last_error_is_internal);
+
+	/* internal error without specifying what it is. this needs to clear
+	   the previous internal error. */
 	mail_storage_set_internal_error(&storage);
 	test_assert(strstr(mail_storage_get_last_error(&storage, &mail_error), MAIL_ERRSTR_CRITICAL_MSG) != NULL);
 	test_assert(mail_error == MAIL_ERROR_TEMP);