diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm index c038618..aa9cc94 100644 --- a/lib/Qpsmtpd/DB/File/DBM.pm +++ b/lib/Qpsmtpd/DB/File/DBM.pm @@ -180,12 +180,47 @@ sub flush { } sub dir { - my ( $self, @arg ) = @_; - return $self->{dir} if $self->{dir} and ! @arg; - for my $d ( $self->candidate_dirs(@arg) ) { - next if ! $self->validate_dir($d); - return $self->{dir} = $d; # first match wins + my ( $self, $dir ) = @_; + if ( $dir ) { + $self->validate_dir($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); }; + 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 { @@ -199,8 +234,9 @@ sub candidate_dirs { sub validate_dir { my ( $self, $d ) = @_; - return 0 if ! $d; - return 0 if ! -d $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/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/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 58a94a7..29b6a17 100644 --- a/t/qpsmtpd-db-file-dbm.t +++ b/t/qpsmtpd-db-file-dbm.t @@ -119,22 +119,79 @@ 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; + 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' ); + rmdir 't/tmp/wtest'; } 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' ); - $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' ); + { + 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 {