From 44cafde7d4155971d2495b249dcec2aee3c477b9 Mon Sep 17 00:00:00 2001
From: Jonathan Hall <jhall@efolder.net>
Date: Tue, 16 Sep 2014 13:48:19 -0500
Subject: [PATCH] More complete clamdcscan tests  * construct our own
 Qpsmtpd::Transaction object for testing, so we're sure    to have a pristine
 state  * Move some logic into a should_scan() sub, to help separate scan_all
 from    is_not_multipart (since the multipart state of a message has nothing
 to    do with any configuration parameters)

---
 plugins/virus/clamdscan        | 18 ++++++++++------
 t/plugin_tests/virus/clamdscan | 39 ++++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/plugins/virus/clamdscan b/plugins/virus/clamdscan
index 5621cc6..2b1cfc3 100644
--- a/plugins/virus/clamdscan
+++ b/plugins/virus/clamdscan
@@ -140,7 +140,7 @@ sub register {
     if ( ! defined $self->{_args}{deny_viruses} ) {
         $self->{_args}{deny_viruses} = 'yes';
     }
-    if ( ! defined $self->{_args}{scan_all} ) {
+    if ( ! $self->{_args}{scan_all} ) {
         $self->{_args}{scan_all} = 1;
     }
     for my $setting (qw( deny_viruses defer_on_error scan_all )) {
@@ -161,8 +161,7 @@ sub data_post_handler {
         $self->log(LOGINFO, "skip, naughty");
         return (DECLINED);
     }
-    return (DECLINED) if $self->is_too_big($transaction);
-    return (DECLINED) if $self->is_not_multipart($transaction);
+    return DECLINED if ! $self->should_scan($transaction);
 
     my $clamd = $self->get_clamd()
       or return $self->err_and_return("Cannot instantiate ClamAV::Client");
@@ -328,8 +327,6 @@ sub is_not_multipart {
     my $self = shift;
     my $transaction = shift || $self->qp->transaction;
 
-    return if $self->{'_args'}{'scan_all'};
-
     return 1 if !$transaction->header;
 
     # Ignore non-multipart emails
@@ -340,5 +337,14 @@ sub is_not_multipart {
         return 1;
     }
 
-    return;
+    return 0;
+}
+
+sub should_scan {
+    my $self = shift;
+    my $tran = shift;
+    return 0 if $self->is_too_big($tran);
+    return 1 if $self->{_args}{scan_all};
+    return 0 if $self->is_not_multipart($tran);
+    return 1;
 }
diff --git a/t/plugin_tests/virus/clamdscan b/t/plugin_tests/virus/clamdscan
index bd432ac..0aa450f 100644
--- a/t/plugin_tests/virus/clamdscan
+++ b/t/plugin_tests/virus/clamdscan
@@ -4,6 +4,8 @@ use strict;
 use warnings;
 
 use Qpsmtpd::Constants;
+use Qpsmtpd::Transaction;
+use Mail::Header;
 
 sub register_tests {
     my $self = shift;
@@ -20,6 +22,7 @@ sub register_tests {
     $self->register_test('test_set_permission', 1);
     $self->register_test('test_is_too_big', 2);
     $self->register_test('test_is_not_multipart', 2);
+    $self->register_test('test_should_scan',4);
 }
 
 sub test_register {
@@ -64,22 +67,40 @@ sub test_get_clamd {
 
 sub test_is_too_big {
     my $self = shift;
-    my $tran = shift || $self->qp->transaction();
+    my $tran = Qpsmtpd::Transaction->new();
 
     $self->{_args}{max_size} = 8;
-    $tran->{_body_size} = (7 * 1024 );
-    ok( ! $self->is_too_big( $tran ), "is_too_big");
+    $tran->{_body_size} = 7 * 1024;
+    ok( ! $self->is_too_big( $tran ), "is_too_big 1");
 
-    $tran->{_body_size} = (9 * 1024 );
-    ok( $self->is_too_big( $tran ), "is_too_big");
+    $tran->{_body_size} = 9 * 1024;
+    ok( $self->is_too_big( $tran ), "is_too_big 2");
 }
 
 sub test_is_not_multipart {
     my $self = shift;
-    my $tran = $self->qp->transaction();
+    my $tran = Qpsmtpd::Transaction->new();
 
-    ok( $self->is_not_multipart(), "not_multipart" );
-    $tran->header->add('Content-Type', 'multipart/alternative; boundary="Jx3Wbb8BMHsO=_?:"');
-    ok( ! $self->is_not_multipart(), "not_multipart" );
+    ok( $self->is_not_multipart($tran), "not_multipart 1" );
+    $tran->header( Mail::Header->new( [
+        'Content-Type: multipart/alternative; boundary="Jx3Wbb8BMHsO=_?:"'
+    ] ) );
+    ok( ! $self->is_not_multipart($tran), "not_multipart 2" );
 }
 
+sub test_should_scan {
+    my $self = shift;
+    my $trans = Qpsmtpd::Transaction->new();
+    $trans->{_body_size} = 1;
+    $self->{_args}{scan_all} = 1;
+    ok( $self->should_scan($trans), "Should scan small message, scan_all=1");
+    $self->{_args}{scan_all} = 0;
+    ok( ! $self->should_scan($trans), "Should not scan small message, scan_all=0");
+    $trans->{_body_size} = 99999999999;
+    ok( !$self->should_scan($trans), "Should not scan large message" );
+    $trans->{_body_size} = 1;
+    $trans->header( Mail::Header->new( [
+        'Content-Type: multipart/alternative; boundary="Jx3Wbb8BMHsO=_?:"'
+    ] ) );
+    ok( $self->should_scan($trans), "Should not scan multi-part message" );
+}