From 2aaedbb44554b5cf7683f2b86dbab4aa04f33651 Mon Sep 17 00:00:00 2001
From: Matt Simerson <matt@tnpi.net>
Date: Sun, 21 Sep 2014 16:52:30 -0700
Subject: [PATCH 1/2] address: added tests for canonify

---
 lib/Qpsmtpd/Address.pm | 18 ++++++++++++------
 t/qpsmtpd-address.t    | 24 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/lib/Qpsmtpd/Address.pm b/lib/Qpsmtpd/Address.pm
index 469a23e..ad5159e 100644
--- a/lib/Qpsmtpd/Address.pm
+++ b/lib/Qpsmtpd/Address.pm
@@ -190,7 +190,9 @@ sub canonify {
     my ($dummy, $path) = @_;
 
     # strip delimiters
-    return if $path !~ /^<(.*)>$/;
+    if ($path !~ /^<(.*)>$/) {
+        return undef, undef, 'missing delimiters'; ## no critic (undef)
+    };
     $path = $1;
 
     my $domain = $domain_expr || "$subdomain_expr(?:\.$subdomain_expr)*";
@@ -204,18 +206,22 @@ sub canonify {
     $path =~ s/^\@$domain(?:,\@$domain)*://;
 
     # empty path is ok
-    return '' if $path eq '';
+    if ($path eq '') {
+        return '', undef, 'empty path';
+    };
 
     # bare postmaster is permissible, perl RFC-2821 (4.5.1)
     if ( $path =~ m/^postmaster$/i ) {
-        return 'postmaster';
+        return 'postmaster', undef, 'bare postmaster';
     }
 
     my ($localpart, $domainpart) = ($path =~ /^(.*)\@($domain)$/);
-    return if !defined $localpart;
+    if (!defined $localpart) {
+        return;
+    };
 
     if ($localpart =~ /^$atom_expr(\.$atom_expr)*/) {
-        return $localpart, $domainpart;  # simple case, we are done
+        return $localpart, $domainpart, 'local matches atom';  # simple case, we are done
     }
 
     if ($localpart =~ /^"(($qtext_expr|\\$text_expr)*)"$/) {
@@ -223,7 +229,7 @@ sub canonify {
         $localpart =~ s/\\($text_expr)/$1/g;
         return $localpart, $domainpart;
     }
-    return;
+    return undef, undef, 'fall through';  ## no critic (undef)
 }
 
 sub parse {
diff --git a/t/qpsmtpd-address.t b/t/qpsmtpd-address.t
index c9123b8..2fc7dec 100644
--- a/t/qpsmtpd-address.t
+++ b/t/qpsmtpd-address.t
@@ -17,6 +17,7 @@ BEGIN {
 __new();
 __config();
 __parse();
+__canonify();
 
 done_testing();
 
@@ -186,3 +187,26 @@ sub __config {
         is($sender->config($_->{pref}), $_->{expected}, $_->{descr});
     }
 }
+
+sub __canonify {
+
+    my $as = 'foo@x.example.com';
+    my $ao = Qpsmtpd::Address->new($as);
+    ok( ! defined $Qpsmtpd::Address::domain_expr, "domain_expr is undef");
+    ok( $Qpsmtpd::Address::subdomain_expr, "subdomain_expr is defined, $Qpsmtpd::Address::subdomain_expr");
+
+    my @r = Qpsmtpd::Address->canonify('sample@path');
+    is_deeply(\@r, [ undef, undef, "missing delimiters" ], 'canonify, missing delimiters');
+
+    @r = Qpsmtpd::Address->canonify('');
+    is_deeply(\@r, [ undef, undef, "missing delimiters" ], 'canonify, empty path');
+
+    @r = Qpsmtpd::Address->canonify('<postmaster>');
+    is_deeply(\@r, [ 'postmaster', undef, "bare postmaster" ], 'canonify, bare postmaster');
+
+    @r = Qpsmtpd::Address->canonify('<postmaster@test>');
+    is_deeply(\@r, [ 'postmaster', 'test', 'local matches atom' ], 'canonify, postmaster@test');
+
+    @r = Qpsmtpd::Address->canonify('<postmáster@test>');
+    is_deeply(\@r, [ 'postmáster', 'test', 'local matches atom' ], 'canonify, postmáster@test, local matches atom');
+}

From 7576e64f708c6c9cdb37b39e9ed8f8708bcd6d5e Mon Sep 17 00:00:00 2001
From: Matt Simerson <matt@tnpi.net>
Date: Mon, 22 Sep 2014 10:33:15 -0700
Subject: [PATCH 2/2] Address: renamed $domain/$domain_re (clearer)

and added a couple more tests
---
 lib/Qpsmtpd/Address.pm | 10 +++++-----
 t/qpsmtpd-address.t    |  7 +++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/Qpsmtpd/Address.pm b/lib/Qpsmtpd/Address.pm
index ad5159e..939a253 100644
--- a/lib/Qpsmtpd/Address.pm
+++ b/lib/Qpsmtpd/Address.pm
@@ -195,27 +195,27 @@ sub canonify {
     };
     $path = $1;
 
-    my $domain = $domain_expr || "$subdomain_expr(?:\.$subdomain_expr)*";
+    my $domain_re = $domain_expr || "$subdomain_expr(?:\.$subdomain_expr)*";
 
     # $address_literal_expr may be empty, if a site doesn't allow them
     if (!$domain_expr && $address_literal_expr) {
-        $domain = "(?:$address_literal_expr|$domain)";
+        $domain_re = "(?:$address_literal_expr|$domain_re)";
     };
 
     # strip source route
-    $path =~ s/^\@$domain(?:,\@$domain)*://;
+    $path =~ s/^\@$domain_re(?:,\@$domain_re)*://;
 
     # empty path is ok
     if ($path eq '') {
         return '', undef, 'empty path';
     };
 
-    # bare postmaster is permissible, perl RFC-2821 (4.5.1)
+    # bare postmaster is permissible, per RFC-2821 (4.5.1)
     if ( $path =~ m/^postmaster$/i ) {
         return 'postmaster', undef, 'bare postmaster';
     }
 
-    my ($localpart, $domainpart) = ($path =~ /^(.*)\@($domain)$/);
+    my ($localpart, $domainpart) = $path =~ /^(.*)\@($domain_re)$/;
     if (!defined $localpart) {
         return;
     };
diff --git a/t/qpsmtpd-address.t b/t/qpsmtpd-address.t
index 2fc7dec..94fa11c 100644
--- a/t/qpsmtpd-address.t
+++ b/t/qpsmtpd-address.t
@@ -207,6 +207,13 @@ sub __canonify {
     @r = Qpsmtpd::Address->canonify('<postmaster@test>');
     is_deeply(\@r, [ 'postmaster', 'test', 'local matches atom' ], 'canonify, postmaster@test');
 
+    @r = Qpsmtpd::Address->canonify('<@a:postmaster@test>');
+    is_deeply(\@r, [ 'postmaster', 'test', 'local matches atom' ], 'canonify, @a:postmaster@test (source route)');
+
     @r = Qpsmtpd::Address->canonify('<postmáster@test>');
     is_deeply(\@r, [ 'postmáster', 'test', 'local matches atom' ], 'canonify, postmáster@test, local matches atom');
+
+    @r = Qpsmtpd::Address->canonify('<@192.168.1.1>');
+    is_deeply(\@r, [ undef, undef, 'fall through' ], 'canonify, fall through, @192.168.1.1')
+        or diag Data::Dumper::Dumper(@r);
 }