From: Timo Sirainen Date: Sun, 18 May 2003 16:02:46 +0000 (+0300) Subject: More robust error handling for mbox. X-Git-Tag: 1.1.alpha1~4623 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=53b32ab10cd22d7e762db4d813298ff72bff4b3d;p=thirdparty%2Fdovecot%2Fcore.git More robust error handling for mbox. --HG-- branch : HEAD --- diff --git a/src/lib-storage/index/mbox/mbox-list.c b/src/lib-storage/index/mbox/mbox-list.c index 2d2db376df..0e88bbcb97 100644 --- a/src/lib-storage/index/mbox/mbox-list.c +++ b/src/lib-storage/index/mbox/mbox-list.c @@ -72,7 +72,7 @@ static int list_opendir(struct mail_storage *storage, if (*dirp != NULL) return 1; - if (errno == ENOENT || errno == ENOTDIR) { + if (ENOTFOUND(errno)) { /* root) user gave invalid hiearchy, ignore sub) probably just race condition with other client deleting the mailbox. */ @@ -201,7 +201,7 @@ static int list_file(struct mailbox_list_context *ctx, const char *fname) DIR *dirp; size_t len; enum imap_match_result match, match2; - int ret; + int ret, noselect; /* skip all hidden files */ if (fname[0] == '.') @@ -225,15 +225,19 @@ static int list_file(struct mailbox_list_context *ctx, const char *fname) /* see if it's a directory */ real_path = t_strconcat(ctx->dir->real_path, "/", fname, NULL); - if (stat(real_path, &st) < 0) { - if (errno == ENOENT) - return 0; /* just deleted, ignore */ + if (stat(real_path, &st) == 0) + noselect = FALSE; + else if (errno == EACCES || errno == ELOOP) + noselect = TRUE; + else { + if (ENOTFOUND(errno)) + return 0; mail_storage_set_critical(ctx->storage, "stat(%s) failed: %m", real_path); return -1; } - if (S_ISDIR(st.st_mode)) { + if (!noselect && S_ISDIR(st.st_mode)) { /* subdirectory. scan inside it. */ path = t_strconcat(list_path, "/", NULL); match2 = imap_match(ctx->glob, path); @@ -265,7 +269,8 @@ static int list_file(struct mailbox_list_context *ctx, const char *fname) /* don't match any INBOX here, it's added separately. we might also have ~/mail/inbox, ~/mail/Inbox etc. Just ignore them for now. */ - ctx->list.flags = MAILBOX_NOINFERIORS | STAT_GET_MARKED(st); + ctx->list.flags = noselect ? MAILBOX_NOSELECT : + (MAILBOX_NOINFERIORS | STAT_GET_MARKED(st)); ctx->list.name = p_strdup(ctx->list_pool, list_path); return 1; } diff --git a/src/lib-storage/index/mbox/mbox-storage.c b/src/lib-storage/index/mbox/mbox-storage.c index 81cd577de0..1e24736235 100644 --- a/src/lib-storage/index/mbox/mbox-storage.c +++ b/src/lib-storage/index/mbox/mbox-storage.c @@ -2,6 +2,7 @@ #include "lib.h" #include "home-expand.h" +#include "mkdir-parents.h" #include "unlink-directory.h" #include "subscription-file/subscription-file.h" #include "mail-custom-flags.h" @@ -20,39 +21,17 @@ extern struct mail_storage mbox_storage; extern struct mailbox mbox_mailbox; -static int mbox_permission_denied(struct mail_storage *storage) +static int mbox_handle_errors(struct mail_storage *storage) { - mail_storage_set_error(storage, "Permission denied"); - return FALSE; -} - -static int mkdir_parents(const char *path) -{ - const char *p, *dir; - - p = path; - if (*p == '/') p++; - - do { - t_push(); - - p = strchr(p, '/'); - if (p == NULL) - dir = path; - else { - dir = t_strdup_until(path, p); - p++; - } - - if (mkdir(dir, CREATE_MODE) < 0 && errno != EEXIST) { - t_pop(); - return -1; - } - - t_pop(); - } while (p != NULL); - - return 0; + if (ENOACCESS(errno)) + mail_storage_set_error(storage, "Permission denied"); + else if (ENOSPACE(errno)) + mail_storage_set_error(storage, "Not enough disk space"); + else if (ENOTFOUND(errno)) + mail_storage_set_error(storage, "Directory structure is broken"); + else + return FALSE; + return TRUE; } static int mbox_autodetect(const char *data) @@ -139,7 +118,7 @@ static const char *create_root_dir(void) } path = t_strconcat(home, "/mail", NULL); - if (mkdir(path, CREATE_MODE) < 0) { + if (mkdir_parents(path, CREATE_MODE) < 0) { i_error("mbox: Can't create root IMAP folder %s: %m", path); return NULL; } @@ -295,25 +274,24 @@ static const char *mbox_get_index_dir(struct mail_storage *storage, } static int create_mbox_index_dirs(struct mail_storage *storage, - const char *name, int verify) + const char *name) { - const char *index_dir, *imap_dir; + const char *index_dir; index_dir = mbox_get_index_dir(storage, name); if (index_dir == NULL) return TRUE; - imap_dir = t_strdup_until(index_dir, strstr(index_dir, ".imap/") + 5); - - if (mkdir(imap_dir, CREATE_MODE) == -1 && errno != EEXIST) - return FALSE; - if (mkdir(index_dir, CREATE_MODE) == -1 && (errno != EEXIST || !verify)) + if (mkdir_parents(index_dir, CREATE_MODE) < 0) { + mail_storage_set_critical(storage, + "mkdir_parents(%s) failed: %m", index_dir); return FALSE; + } return TRUE; } -static void verify_inbox(struct mail_storage *storage) +static int verify_inbox(struct mail_storage *storage) { int fd; @@ -323,7 +301,10 @@ static void verify_inbox(struct mail_storage *storage) (void)close(fd); /* make sure the index directories exist */ - (void)create_mbox_index_dirs(storage, "INBOX", TRUE); + if (!create_mbox_index_dirs(storage, "INBOX")) + return FALSE; + + return TRUE; } static const char *mbox_get_path(struct mail_storage *storage, const char *name) @@ -381,7 +362,8 @@ mbox_open_mailbox(struct mail_storage *storage, /* INBOX is always case-insensitive */ if (strcasecmp(name, "INBOX") == 0) { /* make sure inbox exists */ - verify_inbox(storage); + if (!verify_inbox(storage)) + return FALSE; return mbox_open(storage, "INBOX", readonly, fast); } @@ -399,18 +381,19 @@ mbox_open_mailbox(struct mail_storage *storage, } /* exists - make sure the required directories are also there */ - (void)create_mbox_index_dirs(storage, name, TRUE); + if (!create_mbox_index_dirs(storage, name)) + return NULL; return mbox_open(storage, name, readonly, fast); - } else if (errno == ENOENT || errno == ENOTDIR) { + } + + if (ENOTFOUND(errno)) { mail_storage_set_error(storage, "Mailbox doesn't exist: %s", name); - return NULL; - } else { - mail_storage_set_critical(storage, "Can't open mailbox %s: %m", - name); - return NULL; - } + } else if (!mbox_handle_errors(storage)) + mail_storage_set_critical(storage, "stat(%s) failed: %m", path); + + return NULL; } static int mbox_create_mailbox(struct mail_storage *storage, const char *name, @@ -437,27 +420,27 @@ static int mbox_create_mailbox(struct mail_storage *storage, const char *name, return FALSE; } - if (errno == ENOTDIR) { - mail_storage_set_error(storage, - "Mailbox doesn't allow inferior mailboxes"); - return FALSE; - } - - if (errno != ENOENT) { - mail_storage_set_critical(storage, - "stat() failed for mbox file %s: %m", path); + if (errno != ENOENT && errno != ELOOP && errno != EACCES) { + if (errno == ENOTDIR) { + mail_storage_set_error(storage, + "Mailbox doesn't allow inferior mailboxes"); + } else { + mail_storage_set_critical(storage, + "stat() failed for mbox file %s: %m", path); + } return FALSE; } /* create the hierarchy if needed */ p = only_hierarchy ? path + strlen(path) : strrchr(path, '/'); if (p != NULL) { - if (mkdir_parents(t_strdup_until(path, p)) < 0) { - if (errno == EACCES) - return mbox_permission_denied(storage); + p = t_strdup_until(path, p); + if (mkdir_parents(p, CREATE_MODE) < 0) { + if (mbox_handle_errors(storage)) + return FALSE; + mail_storage_set_critical(storage, - "mkdir_parents() failed for mbox path %s: %m", - path); + "mkdir_parents(%s) failed: %m", p); return FALSE; } @@ -472,17 +455,16 @@ static int mbox_create_mailbox(struct mail_storage *storage, const char *name, if (fd != -1) { (void)close(fd); return TRUE; - } else if (errno == EEXIST) { + } + + if (errno == EEXIST) { /* mailbox was just created between stat() and open() call.. */ mail_storage_set_error(storage, "Mailbox already exists"); - return FALSE; - } else if (errno == EACCES) { - return mbox_permission_denied(storage); - } else { - mail_storage_set_critical(storage, "Can't create mailbox " - "%s: %m", name); - return FALSE; + } else if (!mbox_handle_errors(storage)) { + mail_storage_set_critical(storage, + "Can't create mailbox %s: %m", name); } + return FALSE; } static int mbox_delete_mailbox(struct mail_storage *storage, const char *name) @@ -504,11 +486,10 @@ static int mbox_delete_mailbox(struct mail_storage *storage, const char *name) path = mbox_get_path(storage, name); if (lstat(path, &st) < 0) { - if (errno == ENOENT || errno == ENOTDIR) { + if (ENOTFOUND(errno)) { mail_storage_set_error(storage, - "Mailbox doesn't exist: %s", - name); - } else { + "Mailbox doesn't exist: %s", name); + } else if (!mbox_handle_errors(storage)) { mail_storage_set_critical(storage, "lstat() failed for " "%s: %m", path); } @@ -520,13 +501,14 @@ static int mbox_delete_mailbox(struct mail_storage *storage, const char *name) if (rmdir(path) == 0) return TRUE; - if (errno == ENOTEMPTY) { + if (ENOTFOUND(errno)) { + mail_storage_set_error(storage, + "Mailbox doesn't exist: %s", name); + } else if (errno == ENOTEMPTY) { mail_storage_set_error(storage, "Folder %s isn't empty, can't delete it.", name); - } else if (errno == EACCES) { - mbox_permission_denied(storage); - } else { + } else if (!mbox_handle_errors(storage)) { mail_storage_set_critical(storage, "rmdir() failed for %s: %m", path); } @@ -535,16 +517,12 @@ static int mbox_delete_mailbox(struct mail_storage *storage, const char *name) /* first unlink the mbox file */ if (unlink(path) < 0) { - if (errno == ENOENT || errno == ENOTDIR) { + if (ENOTFOUND(errno)) { mail_storage_set_error(storage, - "Mailbox doesn't exist: %s", - name); - } else if (errno == EACCES) { - mbox_permission_denied(storage); - } else { + "Mailbox doesn't exist: %s", name); + } else if (!mbox_handle_errors(storage)) { mail_storage_set_critical(storage, - "unlink() failed for %s: %m", - path); + "unlink() failed for %s: %m", path); } return FALSE; } @@ -553,8 +531,8 @@ static int mbox_delete_mailbox(struct mail_storage *storage, const char *name) index_dir = mbox_get_index_dir(storage, name); if (index_dir != NULL && unlink_directory(index_dir, TRUE) < 0 && errno != ENOENT) { - mail_storage_set_critical(storage, "unlink_directory(%s) " - "failed: %m", index_dir); + mail_storage_set_critical(storage, + "unlink_directory(%s) failed: %m", index_dir); /* mailbox itself is deleted, so return success anyway */ } return TRUE; @@ -564,6 +542,7 @@ static int mbox_rename_mailbox(struct mail_storage *storage, const char *oldname, const char *newname) { const char *oldpath, *newpath, *old_indexdir, *new_indexdir, *p; + struct stat st; mail_storage_clear_error(storage); @@ -582,32 +561,41 @@ static int mbox_rename_mailbox(struct mail_storage *storage, /* create the hierarchy */ p = strrchr(newpath, '/'); if (p != NULL) { - if (mkdir_parents(t_strdup_until(newpath, p)) < 0) { + p = t_strdup_until(newpath, p); + if (mkdir_parents(p, CREATE_MODE) < 0) { + if (mbox_handle_errors(storage)) + return FALSE; + mail_storage_set_critical(storage, - "mkdir_parents() failed for mbox path %s: %m", - newpath); + "mkdir_parents(%s) failed: %m", p); return FALSE; } } - /* NOTE: renaming INBOX works just fine with us, it's simply created - the next time it's needed. FIXME: it's not atomic, should we use - rename() instead? That might overwrite files.. */ - if (link(oldpath, newpath) == 0) - (void)unlink(oldpath); - else if (errno == EEXIST) { + /* first check that the destination mailbox doesn't exist. + this is racy, but we need to be atomic and there's hardly any + possibility that someone actually tries to rename two mailboxes + to same new one */ + if (lstat(newpath, &st) == 0) { mail_storage_set_error(storage, "Target mailbox already exists"); return FALSE; - } else if (errno == ENOENT || errno == ENOTDIR) { - mail_storage_set_error(storage, "Mailbox doesn't exist: %s", - oldname); + } else if (!ENOTFOUND(errno) && errno != EACCES) { + mail_storage_set_critical(storage, "lstat(%s) failed: %m", + newpath); return FALSE; - } else if (errno == EACCES) { - return mbox_permission_denied(storage); - } else { - mail_storage_set_critical(storage, "link(%s, %s) failed: %m", - oldpath, newpath); + } + + /* NOTE: renaming INBOX works just fine with us, it's simply recreated + the next time it's needed. */ + if (rename(oldpath, newpath) < 0) { + if (ENOTFOUND(errno)) { + mail_storage_set_error(storage, + "Mailbox doesn't exist: %s", oldname); + } else if (!mbox_handle_errors(storage)) { + mail_storage_set_critical(storage, + "rename(%s, %s) failed: %m", oldpath, newpath); + } return FALSE; } @@ -653,7 +641,7 @@ static int mbox_get_mailbox_name_status(struct mail_storage *storage, return TRUE; } - if (errno == ENOENT) { + if (ENOTFOUND(errno) || errno == EACCES) { *status = MAILBOX_NAME_VALID; return TRUE; } else if (errno == ENOTDIR) { diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index 57734bc1e0..366bc52ee5 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -32,6 +32,7 @@ liblib_a_SOURCES = \ mempool-alloconly.c \ mempool-datastack.c \ mempool-system.c \ + mkdir-parents.c \ mmap-anon.c \ mmap-util.c \ module-dir.c \ @@ -85,6 +86,7 @@ noinst_HEADERS = \ macros.h \ md5.h \ mempool.h \ + mkdir-parents.h \ mmap-util.h \ module-dir.h \ network.h \ diff --git a/src/lib/compat.h b/src/lib/compat.h index 486398dd85..be531f66cc 100644 --- a/src/lib/compat.h +++ b/src/lib/compat.h @@ -122,4 +122,15 @@ ssize_t my_writev(int fd, const struct iovec *iov, int iov_len); # define ENOSPACE(errno) ((errno) == ENOSPC) #endif +/* EPERM is returned sometimes if device doesn't support such modification */ +#ifdef EROFS +# define ENOACCESS(errno) \ + ((errno) == EACCES || (errno) == EROFS || (errno) == EPERM) +#else +# define ENOACCESS(errno) ((errno) == EACCES || (errno) == EPERM) +#endif + +#define ENOTFOUND(errno) \ + ((errno) == ENOENT || (errno) == ENOTDIR || (errno) == ELOOP) + #endif diff --git a/src/lib/mkdir-parents.c b/src/lib/mkdir-parents.c new file mode 100644 index 0000000000..4f018a1740 --- /dev/null +++ b/src/lib/mkdir-parents.c @@ -0,0 +1,33 @@ +/* Copyright (c) 2003 Timo Sirainen */ + +#include "lib.h" +#include "mkdir-parents.h" + +#include + +int mkdir_parents(const char *path, mode_t mode) +{ + const char *p; + + if (mkdir(path, mode) < 0 && errno != EEXIST) { + if (errno != ENOENT) + return -1; + + p = strrchr(path, '/'); + if (p == NULL || p == path) + return -1; /* shouldn't happen */ + + t_push(); + if (mkdir_parents(t_strdup_until(path, p), mode) < 0) { + t_pop(); + return -1; + } + t_pop(); + + /* should work now */ + if (mkdir(path, mode) < 0 && errno != EEXIST) + return -1; + } + + return 0; +} diff --git a/src/lib/mkdir-parents.h b/src/lib/mkdir-parents.h new file mode 100644 index 0000000000..0efbf12fcf --- /dev/null +++ b/src/lib/mkdir-parents.h @@ -0,0 +1,8 @@ +#ifndef __MKDIR_PARENTS_H +#define __MKDIR_PARENTS_H + +/* Create path and all the directories under it if needed. + Returns 0 if ok, or if path already exists (not necessarily as directory). */ +int mkdir_parents(const char *path, mode_t mode); + +#endif