changeset 13:bb294faf7379 HEAD

"Critical errors" aren't displayed to users anymore, ie. anything that is not a predefined human readable error message is written into log file and user gets only "Internal error [timestamp]".
author Timo Sirainen <tss@iki.fi>
date Thu, 22 Aug 2002 17:50:16 +0300
parents a4423c83b2b0
children af70462f5ca1
files TODO src/lib-storage/index/index-storage.c src/lib-storage/index/maildir/maildir-storage.c src/lib-storage/index/mbox/mbox-save.c src/lib-storage/index/mbox/mbox-storage.c src/lib-storage/mail-storage.c src/lib-storage/mail-storage.h
diffstat 7 files changed, 42 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/TODO	Thu Aug 22 17:26:17 2002 +0300
+++ b/TODO	Thu Aug 22 17:50:16 2002 +0300
@@ -104,7 +104,7 @@
  - ulimit / setrlimit() should be set somewhere
  - create indexer binary
  - SEARCH CHARSET support, iconv()?
- - Fix the blocking SSL handshake
+ - Fix the blocking SSL handshake (req. gnutls 0.5.2)
  - SRP authentication support?
  - Digest-MD5: support integrity protection, and maybe crypting. Do it
    through imap-login like SSL is done?
@@ -113,13 +113,8 @@
    should be something like once/sec.
  - support executing each login in it's own process, so if an exploit is ever
    found from it, the attacker can't see other users' passwords
- - the error messages given in command replies can sometimes be quite
-   specific, eg. rename(/full/path, /full/new/path) failed: xxx. These
-   probably shouldn't be shown to user, instead just print some "internal
-   error" with a timestamp and the real error would be written into syslog.
-   all errors from lib-index should be done this, and maybe some/all
-   lib-storage errors as well (there's separate error vs. critical)
- - Make sure messages of size INT_MAX..UINT_MAX work correctly
+ - Make sure messages of size INT_MAX..UINT_MAX (and more) work correctly
+   virtual_size can also overflow making it less than physical_size
  - allocating readwrite pools now just uses system_pool .. so pool_unref()
    can't free memory used by it .. what to do about it? at least count the
    malloc/free calls and make complain if at the exit they don't match
@@ -134,7 +129,7 @@
  - check that (off_t) castings are safe
  - IOBuffer is a bit confusing and weird. especially the offset-parameter
    works strangely.. And maybe split it into IBuffer and OBuffer?
- - optimize imap_message_send() by using a buffer. most mails don't have long
+ - optimize message_send() by using a buffer. most mails don't have long
    lines and without a buffer it creates tons of write() calls.
 
 optional optimizations:
--- a/src/lib-storage/index/index-storage.c	Thu Aug 22 17:26:17 2002 +0300
+++ b/src/lib-storage/index/index-storage.c	Thu Aug 22 17:50:16 2002 +0300
@@ -10,17 +10,13 @@
 {
 	IndexMailbox *ibox;
 	FlagsFile *flagsfile;
-	const char *path, *error;
+	const char *path;
 
 	i_assert(name != NULL);
 
 	/* open the index first */
 	if (!index->open_or_create(index, !readonly)) {
-		error = index->get_last_error(index);
-		if (error == NULL)
-			error = "(maildir_open)";
-		mail_storage_set_error(storage, "%s", error);
-
+		mail_storage_set_internal_error(storage);
 		index->free(index);
 		return NULL;
 	}
@@ -60,15 +56,9 @@
 
 int mail_storage_set_index_error(IndexMailbox *ibox)
 {
-	const char *error;
-
-	error = ibox->index->get_last_error(ibox->index);
-	if (error == NULL)
-		error = "(no error message)";
-
 	ibox->box.inconsistent =
 		ibox->index->is_inconsistency_error(ibox->index);
-	mail_storage_set_error(ibox->box.storage, "%s", error);
+	mail_storage_set_internal_error(ibox->box.storage);
 	return FALSE;
 }
 
--- a/src/lib-storage/index/maildir/maildir-storage.c	Thu Aug 22 17:26:17 2002 +0300
+++ b/src/lib-storage/index/maildir/maildir-storage.c	Thu Aug 22 17:50:16 2002 +0300
@@ -255,8 +255,9 @@
 		i_snprintf(newpath, sizeof(newpath), "%s/%s", newdir, *tmp);
 
 		if (unlink(newpath) == -1 && errno != EEXIST) {
-			mail_storage_set_error(storage, "unlink(%s) failed: "
-					       "%m", newpath);
+			mail_storage_set_critical(storage,
+						  "unlink(%s) failed: %m",
+						  newpath);
 			return FALSE;
 		}
 	}
--- a/src/lib-storage/index/mbox/mbox-save.c	Thu Aug 22 17:26:17 2002 +0300
+++ b/src/lib-storage/index/mbox/mbox-save.c	Thu Aug 22 17:50:16 2002 +0300
@@ -29,8 +29,8 @@
 	/* append the data into mbox file */
 	fd = open(ibox->index->mbox_path, O_RDWR | O_CREAT);
 	if (fd == -1) {
-		mail_storage_set_error(box->storage, "Can't open mbox file "
-				       "%s: %m", ibox->index->mbox_path);
+		mail_storage_set_critical(box->storage, "Can't open mbox file "
+					  "%s: %m", ibox->index->mbox_path);
 		return FALSE;
 	}
 
@@ -43,8 +43,9 @@
 
 	pos = lseek(fd, 0, SEEK_END);
 	if (pos == -1) {
-		mail_storage_set_error(box->storage, "lseek() failed for mbox "
-				       "file %s: %m", ibox->index->mbox_path);
+		mail_storage_set_critical(box->storage,
+					  "lseek() failed for mbox file %s: %m",
+					  ibox->index->mbox_path);
 		failed = TRUE;
 	}
 
--- a/src/lib-storage/index/mbox/mbox-storage.c	Thu Aug 22 17:26:17 2002 +0300
+++ b/src/lib-storage/index/mbox/mbox-storage.c	Thu Aug 22 17:50:16 2002 +0300
@@ -245,8 +245,8 @@
 			mail_storage_set_error(storage,
 					       "Mailbox doesn't exist.");
 		} else {
-			mail_storage_set_error(storage, "Can't delete mbox "
-					       "file %s: %m", path);
+			mail_storage_set_critical(storage, "Can't delete mbox "
+						  "file %s: %m", path);
 		}
 		return FALSE;
 	}
--- a/src/lib-storage/mail-storage.c	Thu Aug 22 17:26:17 2002 +0300
+++ b/src/lib-storage/mail-storage.c	Thu Aug 22 17:50:16 2002 +0300
@@ -1,10 +1,15 @@
 /* Copyright (C) 2002 Timo Sirainen */
 
 #include "lib.h"
+#include "ioloop.h"
 #include "mail-storage.h"
 
+#include <time.h>
 #include <ctype.h>
 
+/* Message to show to users when critical error occurs */
+#define CRITICAL_MSG "Internal error [%Y-%m-%d %H:%M:%S]"
+
 typedef struct _MailStorageList MailStorageList;
 
 struct _MailStorageList {
@@ -138,20 +143,34 @@
 	}
 }
 
+void mail_storage_set_internal_error(MailStorage *storage)
+{
+	struct tm *tm;
+	char *str;
+
+	tm = localtime(&ioloop_time);
+	str = t_buffer_get(256);
+
+	storage->error = strftime(str, 256, CRITICAL_MSG, tm) > 0 ?
+		i_strdup(str) : i_strdup("Internal error");
+}
+
 void mail_storage_set_critical(MailStorage *storage, const char *fmt, ...)
 {
 	va_list va;
 
 	i_free(storage->error);
-
 	if (fmt == NULL)
 		storage->error = NULL;
 	else {
 		va_start(va, fmt);
-		storage->error = i_strdup_vprintf(fmt, va);
+		i_error("%s", t_strdup_vprintf(fmt, va));
 		va_end(va);
 
-		i_error("%s", storage->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);
 	}
 }
 
--- a/src/lib-storage/mail-storage.h	Thu Aug 22 17:26:17 2002 +0300
+++ b/src/lib-storage/mail-storage.h	Thu Aug 22 17:50:16 2002 +0300
@@ -229,12 +229,14 @@
 MailStorage *mail_storage_create_default(void);
 MailStorage *mail_storage_create_with_data(const char *data);
 
-/* Set error message in storage. Critical errors are logged with syslog() */
+/* Set error message in storage. Critical errors are logged with i_error(),
+   but user sees only "internal error" message. */
 void mail_storage_clear_error(MailStorage *storage);
 void mail_storage_set_error(MailStorage *storage, const char *fmt, ...)
 	__attr_format__(2, 3);
 void mail_storage_set_critical(MailStorage *storage, const char *fmt, ...)
 	__attr_format__(2, 3);
+void mail_storage_set_internal_error(MailStorage *storage);
 
 const char *mail_storage_get_last_error(MailStorage *storage);
 int mail_storage_is_inconsistency_error(Mailbox *box);