From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 3E69D1F46C for ; Sat, 14 Dec 2019 01:02:57 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/2] address: use Email::Address::XS if available Date: Sat, 14 Dec 2019 01:02:56 +0000 Message-Id: <20191214010256.19858-3-e@80x24.org> In-Reply-To: <20191214010256.19858-1-e@80x24.org> References: <20191214010256.19858-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Email::Address::XS is a dependency of modern versions of Email::MIME, so it's likely loaded and installed on newer systems, already; and capable of handling more corner-cases than our pure-Perl fallback. We still fallback to the imperfect-but-good-enough-in-practice pure-Perl code while avoiding the non-XS Email::Address (which was susceptible to DoS attacks (CVE-2015-7686)). We just need to keep "git fast-import" happy. --- MANIFEST | 1 + lib/PublicInbox/Address.pm | 44 ++++++++++------------- lib/PublicInbox/AddressPP.pm | 38 ++++++++++++++++++++ t/address.t | 67 ++++++++++++++++++++++-------------- 4 files changed, 98 insertions(+), 52 deletions(-) create mode 100644 lib/PublicInbox/AddressPP.pm diff --git a/MANIFEST b/MANIFEST index 044bbfef..3e760f89 100644 --- a/MANIFEST +++ b/MANIFEST @@ -81,6 +81,7 @@ examples/unsubscribe.milter examples/unsubscribe.psgi examples/varnish-4.vcl lib/PublicInbox/Address.pm +lib/PublicInbox/AddressPP.pm lib/PublicInbox/Admin.pm lib/PublicInbox/AdminEdit.pm lib/PublicInbox/AltId.pm diff --git a/lib/PublicInbox/Address.pm b/lib/PublicInbox/Address.pm index a58d1eff..433b36eb 100644 --- a/lib/PublicInbox/Address.pm +++ b/lib/PublicInbox/Address.pm @@ -4,35 +4,27 @@ package PublicInbox::Address; use strict; use warnings; -# very loose regexes, here. We don't need RFC-compliance, -# just enough to make thing sanely displayable and pass to git +sub xs_emails { map { $_->address() } parse_email_addresses($_[0]) } -sub emails { - ($_[0] =~ /([\w\.\+=\?"\(\)\-!#\$%&'\*\/\^\`\|\{\}~]+\@[\w\.\-\(\)]+) - (?:\s[^>]*)?>?\s*(?:\(.*?\))?(?:,\s*|\z)/gx) +sub xs_names { + map { + my $n = $_->name; + $n = $_->user if $n eq $_->address; + $n; + } parse_email_addresses($_[0]); } -sub names { - my @p = split(/]+)\@[\w\.\-]+>?\s*(\(.*?\))?(?:,\s*|\z)/, - $_[0]); - my @ret; - for (my $i = 0; $i <= $#p;) { - my $phrase = $p[$i++]; - $phrase =~ tr/\r\n\t / /s; - $phrase =~ s/\A['"\s]*//; - $phrase =~ s/['"\s]*\z//; - my $user = $p[$i++] // ''; - my $comment = $p[$i++] // ''; - if ($phrase =~ /\S/) { - $phrase =~ s/\@\S+\z//; - push @ret, $phrase; - } elsif ($comment =~ /\A\((.*?)\)\z/) { - push @ret, $1; - } else { - push @ret, $user; - } - } - @ret; +eval { + require Email::Address::XS; + Email::Address::XS->import(qw(parse_email_addresses)); + *emails = \&xs_emails; + *names = \&xs_names; +}; + +if ($@) { + require PublicInbox::AddressPP; + *emails = \&PublicInbox::AddressPP::emails; + *names = \&PublicInbox::AddressPP::names; } 1; diff --git a/lib/PublicInbox/AddressPP.pm b/lib/PublicInbox/AddressPP.pm new file mode 100644 index 00000000..cd7aedb9 --- /dev/null +++ b/lib/PublicInbox/AddressPP.pm @@ -0,0 +1,38 @@ +# Copyright (C) 2016-2019 all contributors +# License: AGPL-3.0+ +package PublicInbox::AddressPP; +use strict; + +# very loose regexes, here. We don't need RFC-compliance, +# just enough to make thing sanely displayable and pass to git +# We favor Email::Address::XS for conformance if available + +sub emails { + ($_[0] =~ /([\w\.\+=\?"\(\)\-!#\$%&'\*\/\^\`\|\{\}~]+\@[\w\.\-\(\)]+) + (?:\s[^>]*)?>?\s*(?:\(.*?\))?(?:,\s*|\z)/gx) +} + +sub names { + my @p = split(/]+)\@[\w\.\-]+>?\s*(\(.*?\))?(?:,\s*|\z)/, + $_[0]); + my @ret; + for (my $i = 0; $i <= $#p;) { + my $phrase = $p[$i++]; + $phrase =~ tr/\r\n\t / /s; + $phrase =~ s/\A['"\s]*//; + $phrase =~ s/['"\s]*\z//; + my $user = $p[$i++] // ''; + my $comment = $p[$i++] // ''; + if ($phrase =~ /\S/) { + $phrase =~ s/\@\S+\z//; + push @ret, $phrase; + } elsif ($comment =~ /\A\((.*?)\)\z/) { + push @ret, $1; + } else { + push @ret, $user; + } + } + @ret; +} + +1; diff --git a/t/address.t b/t/address.t index 2a287102..e7c0d6a8 100644 --- a/t/address.t +++ b/t/address.t @@ -5,34 +5,49 @@ use warnings; use Test::More; use_ok 'PublicInbox::Address'; -is_deeply([qw(e@example.com e@example.org)], - [PublicInbox::Address::emails('User , e@example.org')], - 'address extraction works as expected'); - -is_deeply(['user@example.com'], - [PublicInbox::Address::emails('')], - 'comment after domain accepted before >'); - -my @names = PublicInbox::Address::names( - 'User , e@e, "John A. Doe" , , (xyz), '. - 'U Ser (do not use)'); -is_deeply(\@names, ['User', 'e', 'John A. Doe', 'x', 'xyz', 'U Ser'], - 'name extraction works as expected'); - -@names = PublicInbox::Address::names('"user@example.com" '); -is_deeply(['user'], \@names, 'address-as-name extraction works as expected'); - - -{ - my $backwards = 'u@example.com (John Q. Public)'; - @names = PublicInbox::Address::names($backwards); - is_deeply(\@names, ['John Q. Public'], 'backwards name OK'); - my @emails = PublicInbox::Address::emails($backwards); - is_deeply(\@emails, ['u@example.com'], 'backwards emails OK'); +sub test_pkg { + my ($pkg) = @_; + my $emails = \&{"${pkg}::emails"}; + my $names = \&{"${pkg}::names"}; + + is_deeply([qw(e@example.com e@example.org)], + [$emails->('User , e@example.org')], + 'address extraction works as expected'); + + is_deeply(['user@example.com'], + [$emails->('')], + 'comment after domain accepted before >'); + + my @names = $names->( + 'User , e@e, "John A. Doe" , , (xyz), '. + 'U Ser (do not use)'); + is_deeply(\@names, ['User', 'e', 'John A. Doe', 'x', 'xyz', 'U Ser'], + 'name extraction works as expected'); + + @names = $names->('"user@example.com" '); + is_deeply(['user'], \@names, + 'address-as-name extraction works as expected'); + + { + my $backwards = 'u@example.com (John Q. Public)'; + @names = $names->($backwards); + is_deeply(\@names, ['John Q. Public'], 'backwards name OK'); + my @emails = $emails->($backwards); + is_deeply(\@emails, ['u@example.com'], 'backwards emails OK'); + } + + @names = $names->('"Quote Unneeded" '); + is_deeply(['Quote Unneeded'], \@names, 'extra quotes dropped'); } +test_pkg('PublicInbox::Address'); -@names = PublicInbox::Address::names('"Quote Unneeded" '); -is_deeply(['Quote Unneeded'], \@names, 'extra quotes dropped'); +SKIP: { + if ($INC{'PublicInbox/AddressPP.pm'}) { + skip 'Email::Address::XS missing', 8; + } + use_ok 'PublicInbox::AddressPP'; + test_pkg('PublicInbox::AddressPP'); +} done_testing;