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 --git a/src/lib-storage/mail-storage.c b/src/lib-storage/mail-storage.c
index 1b16c5c872824686b10d0d566606765ea5652142..8f8f1a4957a85f91849073de76050b66fcdbd29d 100644 (file)
--- a/src/lib-storage/mail-storage.c
+++ b/src/lib-storage/mail-storage.c
@@ -533,6 +533,10 @@ void mail_storage_set_internal_error(struct mail_storage *storage)
        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,
@@ -541,6 +545,12 @@ void mail_storage_set_critical(struct mail_storage *storage,
        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);
@@ -550,11 +560,6 @@ void mail_storage_set_critical(struct mail_storage *storage,
        /* 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 --git a/src/lib-storage/test-mail-storage.c b/src/lib-storage/test-mail-storage.c
index 9bea9632b124b4be69e136312d60d93bee5545e6..d9e3ef251b767b2b32c2fb7e3842ecf8ad15c808 100644 (file)
--- a/src/lib-storage/test-mail-storage.c
+++ b/src/lib-storage/test-mail-storage.c
@@ -38,7 +38,18 @@ static void test_mail_storage_errors(void)
        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);