From 0a16621f024d078223159671129023cdded49d39 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Sat, 2 Jun 2012 00:43:37 -0400 Subject: [PATCH] connection consistency - $self->qp->connection->notes + $self->connection->notes and all tests pass. --- docs/advanced.pod | 4 ++-- plugins/async/check_earlytalker | 3 +-- plugins/async/dnsbl | 8 ++++---- plugins/async/require_resolvable_fromhost | 4 ++-- plugins/check_basicheaders | 2 +- plugins/check_earlytalker | 14 +++++++------- plugins/count_unrecognized_commands | 6 +++--- plugins/dns_whitelist_soft | 4 ++-- plugins/dnsbl | 12 +++++++----- plugins/greylisting | 2 +- plugins/ident/geoip | 4 ++-- plugins/ident/p0f | 4 ++-- plugins/milter | 22 +++++++++++----------- plugins/random_error | 6 +++--- plugins/require_resolvable_fromhost | 3 +-- plugins/tls | 4 ++-- t/plugin_tests/count_unrecognized_commands | 6 +++--- t/plugin_tests/dnsbl | 14 +++++++------- t/plugin_tests/greylisting | 6 +++--- t/plugin_tests/ident/geoip | 2 +- 20 files changed, 65 insertions(+), 65 deletions(-) diff --git a/docs/advanced.pod b/docs/advanced.pod index f0c691d..caa0d10 100644 --- a/docs/advanced.pod +++ b/docs/advanced.pod @@ -69,8 +69,8 @@ should be configured to run I, like B. unless (($rc == DENY) and $self->{_count_relay_max}); my $count = - ($self->qp->connection->notes('count_relay_attempts') || 0) + 1; - $self->qp->connection->notes('count_relay_attempts', $count); + ($self->connection->notes('count_relay_attempts') || 0) + 1; + $self->connection->notes('count_relay_attempts', $count); return ($rc, @msg) unless ($count > $self->{_count_relay_max}); return Qpsmtpd::DSN->relaying_denied(DENY_DISCONNECT, diff --git a/plugins/async/check_earlytalker b/plugins/async/check_earlytalker index 238bee1..fa0266d 100644 --- a/plugins/async/check_earlytalker +++ b/plugins/async/check_earlytalker @@ -116,8 +116,7 @@ sub read_now { sub check_talker_post { my ($self, $transaction) = @_; - my $conn = $self->qp->connection; - return DECLINED unless $conn->notes('earlytalker'); + return DECLINED unless $self->connection->notes('earlytalker'); return DECLINED if $self->{'defer-reject'}; return (DENY,$MSG) if $self->{_args}->{'action'} eq 'deny'; return (DENYSOFT,$MSG) if $self->{_args}->{'action'} eq 'denysoft'; diff --git a/plugins/async/dnsbl b/plugins/async/dnsbl index 1c51401..e9c99ee 100644 --- a/plugins/async/dnsbl +++ b/plugins/async/dnsbl @@ -64,21 +64,21 @@ sub process_a_result { my ($class, $qp, $result, $query) = @_; my $conn = $qp->connection; - return if $conn->notes('dnsbl'); + return if $class->connection->notes('dnsbl'); - my $templates = $conn->notes('dnsbl_templates'); + my $templates = $class->connection->notes('dnsbl_templates'); my $ip = $conn->remote_ip; my $template = $templates->{$query}; $template =~ s/%IP%/$ip/g; - $conn->notes('dnsbl', $template); + $class->connection->notes('dnsbl', $template); } sub process_txt_result { my ($class, $qp, $result, $query) = @_; - my $conn = $qp->connection; + my $conn = $class->connection; $conn->notes('dnsbl', $result) unless $conn->notes('dnsbl'); } diff --git a/plugins/async/require_resolvable_fromhost b/plugins/async/require_resolvable_fromhost index dd99db4..342f113 100644 --- a/plugins/async/require_resolvable_fromhost +++ b/plugins/async/require_resolvable_fromhost @@ -27,7 +27,7 @@ sub hook_mail_start { my ( $self, $transaction, $sender ) = @_; return DECLINED - if ( $self->qp->connection->notes('whitelisthost') ); + if ( $self->connection->notes('whitelisthost') ); if ( $sender ne "<>" ) { @@ -55,7 +55,7 @@ sub hook_mail_done { my ( $self, $transaction, $sender ) = @_; return DECLINED - if ( $self->qp->connection->notes('whitelisthost') ); + if ( $self->connection->notes('whitelisthost') ); if ( $sender ne "<>" && !$transaction->notes('resolvable_fromhost') ) { # default of temp_resolver_failed is DENYSOFT diff --git a/plugins/check_basicheaders b/plugins/check_basicheaders index 684b9a4..944ae9d 100644 --- a/plugins/check_basicheaders +++ b/plugins/check_basicheaders @@ -165,7 +165,7 @@ sub is_immune { return 1; }; - if ( $self->qp->connection->notes('whitelisthost') ) { + if ( $self->connection->notes('whitelisthost') ) { $self->log(LOGINFO, "skip: whitelisted host"); return 1; }; diff --git a/plugins/check_earlytalker b/plugins/check_earlytalker index 6c4eedf..7df31a2 100644 --- a/plugins/check_earlytalker +++ b/plugins/check_earlytalker @@ -106,7 +106,7 @@ sub apr_connect_handler { my ($self, $transaction) = @_; return DECLINED unless $self->{_args}{'check-at'}{CONNECT}; - return DECLINED if ($self->qp->connection->notes('whitelisthost')); + return DECLINED if ($self->connection->notes('whitelisthost')); my $ip = $self->qp->connection->remote_ip; my $c = $self->qp->{conn}; @@ -117,7 +117,7 @@ sub apr_connect_handler { if ($rc == APR::Const::SUCCESS()) { $self->log(LOGNOTICE, "remote host started talking before we said hello [$ip]"); if ($self->{_args}->{'defer-reject'}) { - $self->qp->connection->notes('earlytalker', 1); + $self->connection->notes('earlytalker', 1); } else { my $msg = 'Connecting host started transmitting before SMTP greeting'; @@ -134,7 +134,7 @@ sub apr_data_handler { my ($self, $transaction) = @_; return DECLINED unless $self->{_args}{'check-at'}{DATA}; - return DECLINED if ($self->qp->connection->notes('whitelisthost')); + return DECLINED if ($self->connection->notes('whitelisthost')); my $ip = $self->qp->connection->remote_ip; my $c = $self->qp->{conn}; @@ -160,13 +160,13 @@ sub connect_handler { return DECLINED unless $self->{_args}{'check-at'}{CONNECT}; return DECLINED - if ($self->qp->connection->notes('whitelisthost')); + if ($self->connection->notes('whitelisthost')); $in->add(\*STDIN) || return DECLINED; if ($in->can_read($self->{_args}->{'wait'})) { $self->log(LOGNOTICE, "remote host started talking before we said hello [$ip]"); if ($self->{_args}->{'defer-reject'}) { - $self->qp->connection->notes('earlytalker', 1); + $self->connection->notes('earlytalker', 1); } else { my $msg = 'Connecting host started transmitting before SMTP greeting'; return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny'; @@ -185,7 +185,7 @@ sub data_handler { return DECLINED unless $self->{_args}{'check-at'}{DATA}; return DECLINED - if ($self->qp->connection->notes('whitelisthost')); + if ($self->connection->notes('whitelisthost')); $in->add(\*STDIN) || return DECLINED; if ($in->can_read($self->{_args}->{'wait'})) { @@ -204,7 +204,7 @@ sub mail_handler { my ($self, $transaction) = @_; my $msg = 'Connecting host started transmitting before SMTP greeting'; - return DECLINED unless $self->qp->connection->notes('earlytalker'); + return DECLINED unless $self->connection->notes('earlytalker'); return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny'; return (DENYSOFT,$msg) if $self->{_args}->{'action'} eq 'denysoft'; return DECLINED; diff --git a/plugins/count_unrecognized_commands b/plugins/count_unrecognized_commands index 40a0e1c..445dca7 100644 --- a/plugins/count_unrecognized_commands +++ b/plugins/count_unrecognized_commands @@ -34,7 +34,7 @@ sub register { sub hook_connect { my $self = shift; - $self->qp->connection->notes('unrec_cmd_count', 0); + $self->connection->notes('unrec_cmd_count', 0); return DECLINED; } @@ -44,8 +44,8 @@ sub hook_unrecognized_command { $self->log(LOGINFO, "Unrecognized command '$cmd'"); my $badcmdcount = - $self->qp->connection->notes( 'unrec_cmd_count', - ($self->qp->connection->notes('unrec_cmd_count') || 0) + 1 + $self->connection->notes( 'unrec_cmd_count', + ($self->connection->notes('unrec_cmd_count') || 0) + 1 ); if ($badcmdcount >= $self->{_unrec_cmd_max}) { diff --git a/plugins/dns_whitelist_soft b/plugins/dns_whitelist_soft index 12f1a74..6ca699b 100644 --- a/plugins/dns_whitelist_soft +++ b/plugins/dns_whitelist_soft @@ -71,7 +71,7 @@ sub hook_connect { $sel->add($res->bgsend("$reversed_ip.$dnsbl", "TXT")); } - $self->qp->connection->notes('whitelist_sockets', $sel); + $self->connection->notes('whitelist_sockets', $sel); return DECLINED; } @@ -79,7 +79,7 @@ sub hook_connect { sub process_sockets { my ($self) = @_; - my $conn = $self->qp->connection; + my $conn = $self->connection; return $conn->notes('whitelisthost') if $conn->notes('whitelisthost'); diff --git a/plugins/dnsbl b/plugins/dnsbl index f20a3c3..3ecbed8 100644 --- a/plugins/dnsbl +++ b/plugins/dnsbl @@ -170,8 +170,8 @@ sub hook_connect { } } - $self->qp->connection->notes('dnsbl_sockets', $sel); - $self->qp->connection->notes('dnsbl_domains', $dom); + $self->connection->notes('dnsbl_sockets', $sel); + $self->connection->notes('dnsbl_domains', $dom); return DECLINED; } @@ -199,7 +199,7 @@ sub ip_whitelisted { my ($self) = @_; my $remote_ip = $self->qp->connection->remote_ip; - my $white = $self->qp->connection->notes('whitelisthost'); + my $white = $self->connection->notes('whitelisthost'); if ( $white ) { $self->log(LOGDEBUG, "skip: whitelist overrode blacklist: $white"); return 1; @@ -219,7 +219,7 @@ sub ip_whitelisted { sub process_sockets { my ($self) = @_; - my $conn = $self->qp->connection; + my $conn = $self->connection; return $conn->notes('dnsbl') if $conn->notes('dnsbl'); @@ -334,13 +334,15 @@ sub hook_rcpt { sub hook_disconnect { my ($self, $transaction) = @_; - $self->qp->connection->notes('dnsbl_sockets', undef); + $self->connection->notes('dnsbl_sockets', undef); return DECLINED; } sub get_reject_type { my $self = shift; + my $default = shift || DENY; + my $deny = $self->{_args}{reject_type} or return $default; return $self->{_args}{reject_type} eq 'temp' ? DENYSOFT : $self->{_args}{reject_type} eq 'disconnect' ? DENY_DISCONNECT diff --git a/plugins/greylisting b/plugins/greylisting index 648a12d..e247402 100644 --- a/plugins/greylisting +++ b/plugins/greylisting @@ -324,7 +324,7 @@ sub is_immune { $self->log(LOGINFO, "skip: relay client"); return 1; }; - if ( $self->qp->connection->notes('whitelisthost') ) { + if ( $self->connection->notes('whitelisthost') ) { $self->log(LOGINFO, "skip: whitelisted host"); return 1; }; diff --git a/plugins/ident/geoip b/plugins/ident/geoip index f6b337f..16f70c8 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -46,10 +46,10 @@ sub connect_handler { my $c_name = $geoip->country_name_by_addr( $remote_ip ); if ( $c_name ) { - $self->qp->connection->notes('geoip_country_name', $c_name); + $self->connection->notes('geoip_country_name', $c_name); }; - $self->qp->connection->notes('geoip_country', $c_code); + $self->connection->notes('geoip_country', $c_code); my $message = $c_code; $message .= ", $c_name" if $c_name; diff --git a/plugins/ident/p0f b/plugins/ident/p0f index 9027aa8..2386980 100644 --- a/plugins/ident/p0f +++ b/plugins/ident/p0f @@ -328,7 +328,7 @@ sub store_v2_results { uptime => $uptime, }; - $self->qp->connection->notes('p0f', $p0f); + $self->connection->notes('p0f', $p0f); $self->log(LOGINFO, $genre." (".$detail.")"); $self->log(LOGERROR,"error: $@") if $@; return $p0f; @@ -354,7 +354,7 @@ sub store_v3_results { $r{uptime} = $r{uptime_min} if $r{uptime_min}; }; - $self->qp->connection->notes('p0f', \%r); + $self->connection->notes('p0f', \%r); $self->log(LOGINFO, "$r{os_name} $r{os_flavor}"); $self->log(LOGDEBUG, join(' ', @values )); $self->log(LOGERROR,"error: $@") if $@; diff --git a/plugins/milter b/plugins/milter index 3cf8da5..64370e9 100644 --- a/plugins/milter +++ b/plugins/milter @@ -49,11 +49,11 @@ sub register { sub hook_disconnect { my ($self) = @_; - my $milter = $self->qp->connection->notes('milter') || return DECLINED; + my $milter = $self->connection->notes('milter') || return DECLINED; $milter->send_quit(); - $self->qp->connection->notes('spam', undef); - $self->qp->connection->notes('milter', undef); + $self->connection->notes('spam', undef); + $self->connection->notes('milter', undef); return DECLINED; } @@ -97,9 +97,9 @@ sub hook_connect { $milter->open($self->{host}, $self->{port}, 'tcp'); $milter->protocol_negotiation(); - $self->qp->connection->notes(milter => $milter); + $self->connection->notes(milter => $milter); - $self->qp->connection->notes( + $self->connection->notes( milter_header_changes => { add => [], delete => [], replace => [], } ); my $remote_ip = $self->qp->connection->remote_ip; @@ -110,7 +110,7 @@ sub hook_connect { $self->check_results($transaction, "connection", $milter->send_connect($remote_host, 'tcp4', 0, $remote_ip)); }; - $self->qp->connection->notes('spam', $@) if $@; + $self->connection->notes('spam', $@) if $@; return DECLINED; } @@ -118,11 +118,11 @@ sub hook_connect { sub hook_helo { my ($self, $transaction) = @_; - if (my $txt = $self->qp->connection->notes('spam')) { + if (my $txt = $self->connection->notes('spam')) { return DENY, $txt; } - my $milter = $self->qp->connection->notes('milter'); + my $milter = $self->connection->notes('milter'); my $helo = $self->qp->connection->hello; my $host = $self->qp->connection->hello_host; @@ -139,7 +139,7 @@ sub hook_helo { sub hook_mail { my ($self, $transaction, $address, %param) = @_; - my $milter = $self->qp->connection->notes('milter'); + my $milter = $self->connection->notes('milter'); $self->log(LOGDEBUG, "milter $self->{name} checking MAIL FROM " . $address->format); eval { $self->check_results($transaction, "MAIL FROM", @@ -152,7 +152,7 @@ sub hook_mail { sub hook_rcpt { my ($self, $transaction, $address, %param) = @_; - my $milter = $self->qp->connection->notes('milter'); + my $milter = $self->connection->notes('milter'); $self->log(LOGDEBUG, "milter $self->{name} checking RCPT TO " . $address->format); @@ -166,7 +166,7 @@ sub hook_rcpt { sub hook_data_post { my ($self, $transaction) = @_; - my $milter = $self->qp->connection->notes('milter'); + my $milter = $self->connection->notes('milter'); $self->log(LOGDEBUG, "milter $self->{name} checking headers"); diff --git a/plugins/random_error b/plugins/random_error index 48e7283..3faf890 100644 --- a/plugins/random_error +++ b/plugins/random_error @@ -16,7 +16,7 @@ of messages. The default is 1. Use a negative number to disable. For use with other plugins, scribble the revised failure rate to - $self->qp->connection->notes('random_fail_%'); + $self->connection->notes('random_fail_%'); =cut @@ -31,7 +31,7 @@ sub register { sub NEXT() { DECLINED } sub random_fail { - my $fpct = $_[0]->qp->connection->notes('random_fail_%'); + my $fpct = $_[0]->connection->notes('random_fail_%'); =head1 calculating the probability of failure @@ -55,7 +55,7 @@ or sub hook_connect { - $_[0]->qp->connection->notes('random_fail_%', $_[0]->{__PACKAGE__.'_how'}); + $_[0]->connection->notes('random_fail_%', $_[0]->{__PACKAGE__.'_how'}); goto &random_fail } diff --git a/plugins/require_resolvable_fromhost b/plugins/require_resolvable_fromhost index 55040b0..a02cba1 100644 --- a/plugins/require_resolvable_fromhost +++ b/plugins/require_resolvable_fromhost @@ -11,8 +11,7 @@ my $has_ipv6 = Qpsmtpd::TcpServer::has_ipv6(); sub hook_mail { my ($self, $transaction, $sender, %param) = @_; - return DECLINED - if ($self->qp->connection->notes('whitelisthost')); + return DECLINED if $self->connection->notes('whitelisthost'); foreach my $i ($self->qp->config("invalid_resolvable_fromhost")) { $i =~ s/^\s*//; diff --git a/plugins/tls b/plugins/tls index 1be2245..df12f65 100644 --- a/plugins/tls +++ b/plugins/tls @@ -305,8 +305,8 @@ sub event_read { if (defined $sock) { $qp->connection( $qp->connection->clone ); $qp->reset_transaction; - $qp->connection->notes('tls_socket', $sock); - $qp->connection->notes('tls_enabled', 1); + $self->connection->notes('tls_socket', $sock); + $self->connection->notes('tls_enabled', 1); $qp->watch_read(1); return 1; } diff --git a/t/plugin_tests/count_unrecognized_commands b/t/plugin_tests/count_unrecognized_commands index b92afef..e7026cb 100644 --- a/t/plugin_tests/count_unrecognized_commands +++ b/t/plugin_tests/count_unrecognized_commands @@ -15,17 +15,17 @@ sub test_hook_unrecognized_command { my $self = shift; $self->{_unrec_cmd_max} = 2; - $self->qp->connection->notes( 'unrec_cmd_count', 0 ); + $self->connection->notes( 'unrec_cmd_count', 0 ); my ($code, $mess) = $self->hook_unrecognized_command(undef,'hiya'); cmp_ok( $code, '==', DECLINED, "good" ); - $self->qp->connection->notes( 'unrec_cmd_count', 2 ); + $self->connection->notes( 'unrec_cmd_count', 2 ); ($code, $mess) = $self->hook_unrecognized_command(undef,'snookums'); cmp_ok( $code, '==', DENY_DISCONNECT, "limit" ); ($code, $mess) = $self->hook_unrecognized_command(undef,'wtf'); cmp_ok( $code, '==', DENY_DISCONNECT, "over limit" ); - cmp_ok( $self->qp->connection->notes( 'unrec_cmd_count'), '==', 4, "correct increment" ); + cmp_ok( $self->connection->notes( 'unrec_cmd_count'), '==', 4, "correct increment" ); }; diff --git a/t/plugin_tests/dnsbl b/t/plugin_tests/dnsbl index e8ebb86..76fe046 100644 --- a/t/plugin_tests/dnsbl +++ b/t/plugin_tests/dnsbl @@ -27,9 +27,9 @@ sub test_ip_whitelisted { $self->qp->connection->relay_client(0); ok( ! $self->ip_whitelisted('10.1.1.1'), "no, -"); - $self->qp->connection->notes('whitelisthost', 'hello honey!'); + $self->connection->notes('whitelisthost', 'hello honey!'); ok( $self->ip_whitelisted('10.1.1.1'), "yes, +"); - $self->qp->connection->notes('whitelisthost', undef); + $self->connection->notes('whitelisthost', undef); }; sub test_is_set_rblsmtpd { @@ -51,15 +51,15 @@ sub test_is_set_rblsmtpd { sub test_hook_connect { my $self = shift; - my $connection = $self->qp->connection; - $connection->relay_client(0); # other tests may leave it enabled - $connection->remote_ip('127.0.0.2'); # standard dnsbl test value + my $conn = $self->qp->connection; + $conn->relay_client(0); # other tests may leave it enabled + $conn->remote_ip('127.0.0.2'); # standard dnsbl test value cmp_ok( DECLINED, '==', $self->hook_connect($self->qp->transaction), "connect +"); - ok($connection->notes('dnsbl_sockets'), "sockets +"); - ok($connection->notes('dnsbl_domains'), "domains +"); + ok($self->connection->notes('dnsbl_sockets'), "sockets +"); + ok($self->connection->notes('dnsbl_domains'), "domains +"); } sub test_hook_rcpt { diff --git a/t/plugin_tests/greylisting b/t/plugin_tests/greylisting index f780393..502cb71 100644 --- a/t/plugin_tests/greylisting +++ b/t/plugin_tests/greylisting @@ -63,9 +63,9 @@ sub test_is_immune { ok( ! $self->is_immune(), "nope -" ); foreach ( qw/ whitelisthost / ) { - $self->qp->connection->notes($_, 1); + $self->connection->notes($_, 1); ok( $self->is_immune(), $_); - $self->qp->connection->notes($_, undef); + $self->connection->notes($_, undef); }; foreach ( qw/ whitelistsender tls_enabled / ) { @@ -186,7 +186,7 @@ sub _reset_transaction { $self->qp->connection->relay_client(0); $self->qp->transaction->notes('whitelistsender',0); - $self->qp->connection->notes('whitelisthost',0); + $self->connection->notes('whitelisthost',0); $self->qp->transaction->notes('tls_enabled',0); $self->{_args}{p0f} = undef; $self->{_args}{geoip} = undef; diff --git a/t/plugin_tests/ident/geoip b/t/plugin_tests/ident/geoip index c5f59ba..2e3a0a2 100644 --- a/t/plugin_tests/ident/geoip +++ b/t/plugin_tests/ident/geoip @@ -23,7 +23,7 @@ sub test_geoip_lookup { $self->qp->connection->remote_ip('24.24.24.24'); cmp_ok( $self->connect_handler(), '==', DECLINED, "exit code"); - cmp_ok( $self->qp->connection->notes('geoip_country'), 'eq', 'US', "note"); + cmp_ok( $self->connection->notes('geoip_country'), 'eq', 'US', "note"); };