From 6b13e24221cc5a21754e63fff19370116b5bfdb1 Mon Sep 17 00:00:00 2001
From: Jared Johnson <jjohnson@efolder.net>
Date: Tue, 24 Feb 2015 11:51:18 -0600
Subject: [PATCH 1/4] Add perms test to Qpsmtpd::DB::File::DBM::dir()

---
 lib/Qpsmtpd/DB/File/DBM.pm | 1 +
 t/qpsmtpd-db-file-dbm.t    | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm
index c038618..6126a1d 100644
--- a/lib/Qpsmtpd/DB/File/DBM.pm
+++ b/lib/Qpsmtpd/DB/File/DBM.pm
@@ -201,6 +201,7 @@ sub validate_dir {
     my ( $self, $d ) = @_;
     return 0 if ! $d;
     return 0 if ! -d $d;
+    return 0 if ! -w $d;
     return 1;
 }
 
diff --git a/t/qpsmtpd-db-file-dbm.t b/t/qpsmtpd-db-file-dbm.t
index 58a94a7..c9e471d 100644
--- a/t/qpsmtpd-db-file-dbm.t
+++ b/t/qpsmtpd-db-file-dbm.t
@@ -125,6 +125,13 @@ sub __validate_dir {
         'validate_dir(): false for non-existent directory' );
     is( $db->validate_dir('t/tmp'), 1,
         'validate_dir(): true for real directory' );
+    mkdir 't/tmp/wtest', 0555;
+    is( $db->validate_dir('t/tmp/wtest'), 0,
+        'validate_dir(): false for non-writeable directory' );
+    chmod 0777, 't/tmp/wtest';
+    is( $db->validate_dir('t/tmp/wtest'), 1,
+        'validate_dir(): true for writeable directory' );
+    rmdir 't/tmp/wtest';
 }
 
 sub __dir {

From 84aa8e2328e2b26527f651b6565a0852fa6096e7 Mon Sep 17 00:00:00 2001
From: Jared Johnson <jjohnson@efolder.net>
Date: Tue, 24 Feb 2015 12:34:13 -0600
Subject: [PATCH 2/4] Die when an invalid dir is specified manually

---
 lib/Qpsmtpd/DB/File/DBM.pm | 10 +++++++---
 plugins/greylisting        | 13 +++++++++----
 t/qpsmtpd-db-file-dbm.t    | 10 ++++++++--
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm
index 6126a1d..7a30c5a 100644
--- a/lib/Qpsmtpd/DB/File/DBM.pm
+++ b/lib/Qpsmtpd/DB/File/DBM.pm
@@ -180,9 +180,13 @@ sub flush {
 }
 
 sub dir {
-    my ( $self, @arg ) = @_;
-    return $self->{dir} if $self->{dir} and ! @arg;
-    for my $d ( $self->candidate_dirs(@arg) ) {
+    my ( $self, $dir ) = @_;
+    if ( $dir ) {
+        die "Cannot use DB directory '$dir'\n" if !$self->validate_dir($dir);
+        return $self->{dir} = $dir;
+    }
+    return $self->{dir} if $self->{dir};
+    for my $d ( $self->candidate_dirs ) {
         next if ! $self->validate_dir($d);
         return $self->{dir} = $d; # first match wins
     }
diff --git a/plugins/greylisting b/plugins/greylisting
index 72b04f2..3c872a3 100644
--- a/plugins/greylisting
+++ b/plugins/greylisting
@@ -266,12 +266,17 @@ sub init_dbm {
         name  => 'greylist',
         class => 'Qpsmtpd::DB::File::DBM'
     );
-    my $cdir = $self->{_args}{db_dir};
-    $cdir = $1 if $cdir and $cdir =~ m{^([-a-zA-Z0-9./_]+)$};
-    # greylisting-specific hints for where to store the greylist DB
-    my $db_dir = $self->db->dir( $cdir, '/var/lib/qpsmtpd/greylisting' );
     $self->db->nfs_locking( $self->{_args}{nfslock} );
 
+    # Add to the default list of possible DB directories
+    $self->db->candidate_dirs('/var/lib/qpsmtpd/greylisting');
+    if ( my $dir_arg = $self->{_args}{db_dir} ) {
+        # user-supplied db dir
+        $dir_arg = $1 if $dir_arg =~ m{^([-a-zA-Z0-9./_]+)$};
+        $self->db->dir($dir_arg);
+    }
+    my $db_dir = $self->db->dir;
+
     # Work around old DBM filename
     my $oldname = 'denysoft_greylist';
     if ( ! -f "$db_dir/greylist.dbm" && -f "$db_dir/$oldname.dbm" ) {
diff --git a/t/qpsmtpd-db-file-dbm.t b/t/qpsmtpd-db-file-dbm.t
index c9e471d..65e1eb0 100644
--- a/t/qpsmtpd-db-file-dbm.t
+++ b/t/qpsmtpd-db-file-dbm.t
@@ -137,11 +137,17 @@ sub __validate_dir {
 sub __dir {
     my $db2 = Qpsmtpd::DB::File::DBM->new( name => 'dirtest' );
     is( $db2->dir(), 't/config', 'default directory' );
-    is( $db2->dir('_invalid','t/Test'), 't/Test', 'skip invalid candidate dirs' );
+    delete $db2->{dir};
+    $db2->candidate_dirs('_invalid','t/Test');
+    is( $db2->dir, 't/Test', 'skip invalid candidate dirs' );
     $db2->{dir} = '_cached';
     is( $db2->dir(), '_cached', 'cached directory' );
     is( $db2->dir('t/Test'), 't/Test', 'passing candidate dirs resets cache' );
-    is( $db2->dir('_invalid'), 't/config', 'invalid candidate dirs reverts to default' );
+    delete $db2->{dir};
+    $db2->candidate_dirs('_invalid');
+    is( $db2->dir, 't/config', 'invalid candidate dirs reverts to default' );
+    eval { $db2->dir('_invalid'); };
+    is( $@, "Cannot use DB directory '_invalid'\n", 'die on invalid dir' );
 }
 
 sub __untie_gotcha {

From 1ae8ed206fda170743e39a5620e0ff481a7eebbf Mon Sep 17 00:00:00 2001
From: Jared Johnson <jjohnson@efolder.net>
Date: Tue, 24 Feb 2015 15:19:47 -0600
Subject: [PATCH 3/4] More useful exception on invalid db_dir

---
 lib/Qpsmtpd/DB/File/DBM.pm | 14 +++++++++-----
 t/qpsmtpd-db-file-dbm.t    | 23 ++++++++++++++++-------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm
index 7a30c5a..a95e867 100644
--- a/lib/Qpsmtpd/DB/File/DBM.pm
+++ b/lib/Qpsmtpd/DB/File/DBM.pm
@@ -182,12 +182,16 @@ sub flush {
 sub dir {
     my ( $self, $dir ) = @_;
     if ( $dir ) {
-        die "Cannot use DB directory '$dir'\n" if !$self->validate_dir($dir);
+        $self->validate_dir($dir);
         return $self->{dir} = $dir;
     }
     return $self->{dir} if $self->{dir};
     for my $d ( $self->candidate_dirs ) {
-        next if ! $self->validate_dir($d);
+        # Ignore invalid directories for static default directories
+        my $is_valid;
+        eval { $is_valid = $self->validate_dir($d); };
+        next if $@;
+        next if !$is_valid;
         return $self->{dir} = $d; # first match wins
     }
 }
@@ -203,9 +207,9 @@ sub candidate_dirs {
 
 sub validate_dir {
     my ( $self, $d ) = @_;
-    return 0 if ! $d;
-    return 0 if ! -d $d;
-    return 0 if ! -w $d;
+    die "Empty DB directory supplied\n"        if ! $d;
+    die "DB directory '$d' does not exist\n"   if ! -d $d;
+    die "DB directory '$d' is not writeable\n" if ! -w $d;
     return 1;
 }
 
diff --git a/t/qpsmtpd-db-file-dbm.t b/t/qpsmtpd-db-file-dbm.t
index 65e1eb0..2217d21 100644
--- a/t/qpsmtpd-db-file-dbm.t
+++ b/t/qpsmtpd-db-file-dbm.t
@@ -119,15 +119,24 @@ sub __candidate_dirs {
 }
 
 sub __validate_dir {
-    is( $db->validate_dir(),      0, 'validate_dir(): false on no input' );
-    is( $db->validate_dir(undef), 0, 'validate_dir(): false on undef' );
-    is( $db->validate_dir('invalid'), 0,
-        'validate_dir(): false for non-existent directory' );
+    eval { $db->validate_dir(); };
+    is( $@, "Empty DB directory supplied\n",
+      'validate_dir(): die on no input' );
+    eval { $db->validate_dir(undef); };
+    is( $@, "Empty DB directory supplied\n",
+      'validate_dir(): die on undef' );
+    eval { $db->validate_dir(''); };
+    is( $@, "Empty DB directory supplied\n",
+      'validate_dir(): die on empty string' );
+    eval { $db->validate_dir('invalid'); };
+    is( $@, "DB directory 'invalid' does not exist\n",
+        'validate_dir(): die on non-existent directory' );
     is( $db->validate_dir('t/tmp'), 1,
         'validate_dir(): true for real directory' );
     mkdir 't/tmp/wtest', 0555;
-    is( $db->validate_dir('t/tmp/wtest'), 0,
-        'validate_dir(): false for non-writeable directory' );
+    eval { $db->validate_dir('t/tmp/wtest') };
+    is( $@, "DB directory 't/tmp/wtest' is not writeable\n",
+        'validate_dir(): die on non-writeable directory' );
     chmod 0777, 't/tmp/wtest';
     is( $db->validate_dir('t/tmp/wtest'), 1,
         'validate_dir(): true for writeable directory' );
@@ -147,7 +156,7 @@ sub __dir {
     $db2->candidate_dirs('_invalid');
     is( $db2->dir, 't/config', 'invalid candidate dirs reverts to default' );
     eval { $db2->dir('_invalid'); };
-    is( $@, "Cannot use DB directory '_invalid'\n", 'die on invalid dir' );
+    is( $@, "DB directory '_invalid' does not exist\n", 'die on invalid dir' );
 }
 
 sub __untie_gotcha {

From 88c55ed9a347d9118b3f8cbc5e5d1b6f08d8d88f Mon Sep 17 00:00:00 2001
From: Jared Johnson <jjohnson@efolder.net>
Date: Mon, 9 Mar 2015 16:59:51 -0500
Subject: [PATCH 4/4] Emit detailed warning on unuseable default db dirs

---
 lib/Qpsmtpd/DB/File/DBM.pm | 33 +++++++++++++++++++--
 t/plugin_tests/greylisting |  3 +-
 t/qpsmtpd-db-file-dbm.t    | 59 ++++++++++++++++++++++++++++++--------
 3 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm
index a95e867..aa9cc94 100644
--- a/lib/Qpsmtpd/DB/File/DBM.pm
+++ b/lib/Qpsmtpd/DB/File/DBM.pm
@@ -186,14 +186,41 @@ sub dir {
         return $self->{dir} = $dir;
     }
     return $self->{dir} if $self->{dir};
+    my @err;
     for my $d ( $self->candidate_dirs ) {
         # Ignore invalid directories for static default directories
         my $is_valid;
         eval { $is_valid = $self->validate_dir($d); };
-        next if $@;
-        next if !$is_valid;
-        return $self->{dir} = $d; # first match wins
+        if ($@) {
+            push @err, $@;
+            next;
+        }
+        else {
+            $self->{dir} = $d; # first match wins
+            last;
+        }
     }
+    if ( !$self->{dir} ) {
+        my $err = join "\n",
+          "Unable to find a useable database directory!",
+          "",
+          @err;
+        die $err;
+    }
+    if (@err) {
+        my $err = join "\n",
+          "Encountered errors while selecting database directory:",
+          "",
+          @err,
+          "Selected database directory: $self->{dir}. Data is now stored in:",
+          "",
+          $self->path,
+          "",
+          "It is recommended to manually specify a useable database directory",
+          "and move any important data into this directory.\n";
+        warn $err;
+    }
+    return $self->{dir};
 }
 
 sub candidate_dirs {
diff --git a/t/plugin_tests/greylisting b/t/plugin_tests/greylisting
index dc5a6d6..8e5e2c3 100644
--- a/t/plugin_tests/greylisting
+++ b/t/plugin_tests/greylisting
@@ -358,9 +358,10 @@ sub test_init_dbm {
     my ($self) = @_;
     delete $self->{db};
     delete $self->{_args}{redis};
+    $self->{_args}{db_dir} = 't/tmp';
     $self->init_db;
     is( $self->db->name, 'greylist', 'init_dbm() sets correct db name' );
-    is( $self->db->path, 't/config/greylist.dbm', 'init_dbm() sets correct path' );
+    is( $self->db->path, 't/tmp/greylist.dbm', 'init_dbm() sets correct path' );
     is( ref $self->db, 'Qpsmtpd::DB::File::DBM', 'init_dbm() gives DBM object' );
 }
 
diff --git a/t/qpsmtpd-db-file-dbm.t b/t/qpsmtpd-db-file-dbm.t
index 2217d21..29b6a17 100644
--- a/t/qpsmtpd-db-file-dbm.t
+++ b/t/qpsmtpd-db-file-dbm.t
@@ -145,18 +145,53 @@ sub __validate_dir {
 
 sub __dir {
     my $db2 = Qpsmtpd::DB::File::DBM->new( name => 'dirtest' );
-    is( $db2->dir(), 't/config', 'default directory' );
-    delete $db2->{dir};
-    $db2->candidate_dirs('_invalid','t/Test');
-    is( $db2->dir, 't/Test', 'skip invalid candidate dirs' );
-    $db2->{dir} = '_cached';
-    is( $db2->dir(), '_cached', 'cached directory' );
-    is( $db2->dir('t/Test'), 't/Test', 'passing candidate dirs resets cache' );
-    delete $db2->{dir};
-    $db2->candidate_dirs('_invalid');
-    is( $db2->dir, 't/config', 'invalid candidate dirs reverts to default' );
-    eval { $db2->dir('_invalid'); };
-    is( $@, "DB directory '_invalid' does not exist\n", 'die on invalid dir' );
+    {
+        local $SIG{__WARN__} = sub {
+            warn @_ if $_[0] !~ /selecting database directory/;
+        };
+        is( $db2->dir(), 't/config', 'default directory' );
+        delete $db2->{dir};
+        $db2->candidate_dirs('_invalid','t/Test');
+        is( $db2->dir, 't/Test', 'skip invalid candidate dirs' );
+        $db2->{dir} = '_cached';
+        is( $db2->dir(), '_cached', 'cached directory' );
+        is( $db2->dir('t/Test'), 't/Test', 'passing candidate dirs resets cache' );
+        delete $db2->{dir};
+        $db2->candidate_dirs('_invalid');
+        is( $db2->dir, 't/config', 'invalid candidate dirs reverts to default' );
+        eval { $db2->dir('_invalid'); };
+        is( $@, "DB directory '_invalid' does not exist\n", 'die on invalid dir' );
+    }
+    {
+        delete $db2->{dir};
+        my $warned;
+        local $SIG{__WARN__} = sub {
+            warn @_ if $_[0] !~ /selecting database directory/;
+            $warned .= join '', @_;
+        };
+        $db2->candidate_dirs('_invalid2','t/Test');
+        is( $db2->dir(), 't/Test', 'default directory' );
+        my $expected_warning =
+          "Encountered errors while selecting database directory:
+
+DB directory '_invalid2' does not exist
+
+Selected database directory: t/Test. Data is now stored in:
+
+t/Test/dirtest.dbm
+
+It is recommended to manually specify a useable database directory
+and move any important data into this directory.\n";
+        is( $warned, $expected_warning, 'Emit warning on bad directories' );
+        delete $db2->{dir};
+        $db2->{candidate_dirs} = ['/___invalid___'];
+        my $expected_err =
+          "Unable to find a useable database directory!
+
+DB directory '/___invalid___' does not exist\n";
+        eval { $db2->dir() };
+        is( $@, $expected_err, 'Die on no valid directories' );
+    }
 }
 
 sub __untie_gotcha {