Mercurial > dovecot > core-2.2
changeset 22469:4b1e95fa17e8
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.
| author | Timo Sirainen <timo.sirainen@dovecot.fi> |
|---|---|
| date | Mon, 21 Aug 2017 15:11:30 +0300 |
| parents | 01f841c0febb |
| children | 038c69311cb5 |
| files | src/lib-storage/mail-storage.c src/lib-storage/test-mail-storage.c |
| diffstat | 2 files changed, 22 insertions(+), 6 deletions(-) [+] |
line wrap: on
line diff
--- 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,
--- 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);