Bug#887536: dh-make-perl depends on libemail-address-perl

[Pali Rohár]

On Tuesday 26 June 2018 19:11:03 gregor herrmann wrote:
> On Tue, 26 Jun 2018 14:26:00 +0200, Pali Rohár wrote:
> 
> > Seems that very similar code is in license-reconcile package. So very
> > similar patch like above should be applied also for license-reconcile
> > package (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887550).
> 
> In this case the info would be in a better place if added to #887550
> 
> Cc'ing this bug to add a pointer there.

In attachment is a patch for license-reconcile. It is exactly same as
for dh-make. I have not tested it yet.

-- 
Pali Rohár
pali.ro...@gmail.com
diff -Nurp license-reconcile-0.14.orig/Build.PL license-reconcile-0.14/Build.PL
--- license-reconcile-0.14.orig/Build.PL	2017-01-28 15:51:20.0 +0100
+++ license-reconcile-0.14/Build.PL	2018-06-30 17:01:04.596353038 +0200
@@ -25,7 +25,7 @@ my $builder = Module::Build->new(
 'Debian::Copyright' => '0.2',
 'Dpkg::Version' => 0,
 'Parse::DebianChangelog' => 0,
-'Email::Address' => 0,
+'Email::Address::XS' => '1.01',
 'List::MoreUtils'=>0,
 'Readonly'=>0,
 'File::Slurp' => 0,
diff -Nurp license-reconcile-0.14.orig/lib/Debian/LicenseReconcile/Filter/ChangeLog.pm license-reconcile-0.14/lib/Debian/LicenseReconcile/Filter/ChangeLog.pm
--- license-reconcile-0.14.orig/lib/Debian/LicenseReconcile/Filter/ChangeLog.pm	2017-01-28 15:51:20.0 +0100
+++ license-reconcile-0.14/lib/Debian/LicenseReconcile/Filter/ChangeLog.pm	2018-06-30 17:04:57.643697170 +0200
@@ -4,33 +4,7 @@ use 5.006;
 use strict;
 use warnings;
 use base qw(Debian::LicenseReconcile::Filter);
-use Readonly;
-
-Readonly my $ACTUAL_NAME_RE => '\pL[\s\pL\-\'\.]*\pL';
-
-# See http://www.faqs.org/rfcs/rfc2822.html
-# Section 3.4.1
-use Email::Address;
-Readonly my $EMAIL_RE => $Email::Address::addr_spec;
-
-Readonly my $EMAIL_CHANGES_RE => qr{
-^   # beginining of line
-\s+\*\s # item marker
-Email\schange:\s# email change token
-($ACTUAL_NAME_RE)   # actual name
-\s+->\s+# gap between name and email
-($EMAIL_RE) # email address
-$   # end of line
-}xms;
-
-Readonly my $PERSON_PARSE_RE => qr{
-\A  # beginining of string
-($ACTUAL_NAME_RE)   # actual name
-\s  # gap
-\<$EMAIL_RE\>   # logged email
-\z  # end of string
-}xms;
-
+use Email::Address::XS 1.01;
 
 sub get_info {
 my $self = shift;
@@ -42,17 +16,23 @@ sub get_info {
 my $date= $_->Date;
 my @date_pieces = split( " ", $date );
 my $year= $date_pieces[3];
-if (my %changes = ($_->Changes =~ m/$EMAIL_CHANGES_RE/xmsg)) {
+if (my %changes = ($_->Changes =~ m/^\s+\*\sEmail\schange:\s+(.*?)\s+->\s+(.*?)\s*$/xmsg)) {
 # This way round since we are going backward in time thru changelog
 foreach my $p (keys %changes) {
-$changes{$p} =~ s{[\s\n]+$}{}xms;
+# Parse bare email address; undef if it not an email address
+my $address = Email::Address::XS->parse_bare_address($changes{$p})->address();
+if ($address) {
+$changes{$p} = $address;
+} else {
+delete $changes{$p};
+}
 }
 %email_changes = (
 %changes,
 %email_changes
 );
 }
-if (my ($name) = ($person =~ $PERSON_PARSE_RE)) {
+if (my $name = Email::Address::XS->parse($person)->phrase()) {
 if (exists $email_changes{$name}) {
 $person = "$name <$email_changes{$name}>";
 }


signature.asc
Description: PGP signature

Bug#887536: dh-make-perl depends on libemail-address-perl

[gregor herrmann]

On Tue, 26 Jun 2018 14:26:00 +0200, Pali Rohár wrote:

> Seems that very similar code is in license-reconcile package. So very
> similar patch like above should be applied also for license-reconcile
> package (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887550).

In this case the info would be in a better place if added to #887550

Cc'ing this bug to add a pointer there.
 

Cheers,
gregor


-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   NP: Bob Dylan: If You Ever Go To Houston


signature.asc
Description: Digital Signature

Bug#887536: dh-make-perl depends on libemail-address-perl

[Pali Rohár]

On Saturday 19 May 2018 18:18:03 Pali Rohár wrote:
> On Saturday 19 May 2018 15:28:14 gregor herrmann wrote:
> > On Wed, 17 Jan 2018 20:50:05 +0100, Pali Rohár wrote:
> > 
> > > Hi! Package dh-make-perl depends on libemail-address-perl which is
> > > vulnerable to CVE-2015-7686, see bug #868170. libemail-address-perl
> > > provides perl module Email::Address which is now unmaintained. There is
> > > a new perl module Email::Address::XS which is API compatible replacement
> > > for Email::Address and is available in libemail-address-xs-perl. Please
> > > port dh-make-perl package to use libemail-address-xs-perl. 
> > 
> > dh-make-perl uses
> > 
> > % grep -r Email::Address
> > Build.PL:'Email::Address'=> 0,
> > lib/DhMakePerl/Command/Packaging.pm:use Email::Address;
> > lib/DhMakePerl/Command/Packaging.pm:my $EMAIL_RE = 
> > $Email::Address::addr_spec;
> > 
> > And I think there is no ::addr_spec in libemail-address-xs-perl?
> 
> Yes, Email::Address::XS does not have these regexes defined.
> 
> > > If you need
> > > help with porting let me know.
> > > 
> > Yes, please :)
> 
> I looked at that Packaging.pm file and I'm really not sure that it is
> doing...
> 
> For me it looks like that $PERSON_PARSE_RE just extract phrase (display
> name) from the email address. For this action simple ->parse() method
> should be enough and then ->phrase() would return it.
> 
> $EMAIL_CHANGES_RE seems to extract list of pairs 
> which matches some specific format. So the only thing needed here is to
> check if _address_ is really email address without phrase and angle
> brackets. For parsing ->parse_bare_address() method can be used and then
> check ->address() that returned something.
> 
> I created patch with these changes, but I'm not sure if it is correct
> due to fact that I do not know what that code should do. So it would be
> needed to properly test these changes.
> 
> Anyway, do you really need to parse email address according to RFC2822?
> And is not (.*) in these cases enough?
> 
> Here is patch:
> 
> diff --git a/Build.PL b/Build.PL
> index eb88fa8..a54fc0f 100644
> --- a/Build.PL
> +++ b/Build.PL
> @@ -25,7 +25,7 @@ my $builder = My::Builder->new(
>  'Cwd'   => 0,
>  'Dpkg'  => 0,
>  'Dpkg::Source::Package' => '1.01',
> -'Email::Address'=> 0,
> +'Email::Address::XS'=> '1.01',
>  'Email::Date::Format'   => 0,
>  'File::Basename'=> 0,
>  'File::Copy'=> 0,
> diff --git a/lib/DhMakePerl/Command/Packaging.pm 
> b/lib/DhMakePerl/Command/Packaging.pm
> index 8f14caa..9fb9a9e 100644
> --- a/lib/DhMakePerl/Command/Packaging.pm
> +++ b/lib/DhMakePerl/Command/Packaging.pm
> @@ -35,6 +35,7 @@ use Debian::Control::FromCPAN;
>  use Debian::Dependencies;
>  use Debian::Rules;
>  use DhMakePerl::PodParser ();
> +use Email::Address::XS 1.01;
>  use File::Basename qw(basename dirname);
>  use File::Find qw(find);
>  use File::Path ();
> @@ -1210,31 +1211,6 @@ sub upsurl {
>  }
>  
>  
> -my $ACTUAL_NAME_RE = '\pL[\s\pL\-\'\.]*\pL';
> -
> -# See http://www.faqs.org/rfcs/rfc2822.html
> -# Section 3.4.1
> -use Email::Address;
> -my $EMAIL_RE = $Email::Address::addr_spec;
> -
> -my $EMAIL_CHANGES_RE = qr{
> -^   # beginining of line
> -\s+\*\s # item marker
> -Email\schange:\s# email change token
> -($ACTUAL_NAME_RE)   # actual name
> -\s+->\s+# gap between name and email
> -($EMAIL_RE) # email address
> -$   # end of line
> -}xms;
> -
> -my $PERSON_PARSE_RE = qr{
> -\A  # beginining of string
> -($ACTUAL_NAME_RE)   # actual name
> -\s  # gap
> -\<$EMAIL_RE\>   # logged email
> -\z  # end of string
> -}xms;
> -
>  # This is what needs fixing.
>  sub copyright_from_changelog {
>  my ( $self, $firstmaint, $firstyear ) = @_;
> @@ -1248,17 +1224,23 @@ sub copyright_from_changelog {
>  my $date= $_->Date;
>  my @date_pieces = split( " ", $date );
>  my $year= $date_pieces[3];
> -if (my %changes = ($_->Changes =~ m/$EMAIL_CHANGES_RE/xmsg)) {
> +if (my %changes = ($_->Changes =~ 
> m/^\s+\*\sEmail\schange:\s+(.*?)\s+->\s+(.*?)\s*$/xmsg)) {
>  # This way round since we are going backward in time thru 
> changelog
>  foreach my $p (keys %changes) {
> -$changes{$p} =~ s{[\s\n]+$}{}xms;
> +# Parse bare email address; undef if it not an email address
> +my $address = 
> Email::Address::XS->parse_bare_address($changes{$p})->address();
> +if ($address) {
> +$changes{$p} = $address;
> +} else {
> +delete 

Bug#887536: dh-make-perl depends on libemail-address-perl

[gregor herrmann]

On Thu, 24 May 2018 19:30:01 +, Damyan Ivanov wrote:

> > I created patch with these changes, but I'm not sure if it is correct
> > due to fact that I do not know what that code should do. So it would be
> > needed to properly test these changes.
> > 
> > Anyway, do you really need to parse email address according to RFC2822?
> > And is not (.*) in these cases enough?
> > 
> > Here is patch:
> 
> Thanks.
> 
> FWIW this code is from Nicolas Bamber and deals with filling 
> debian/copyright from the contents of debian/changelog. This is used 
> by 'dh-make-perl refresh' and is probably used in some part of the 
> test suite.
> 
> So I'd say go ahead and apply the patch as it is and either hope that 
> somebody will notice if something broke or add a test that explicitly 
> covers that specific aspect.

Thanks for the review!

I've applied the patch in git now.


Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   NP: Supertramp: C'est Le Bon


signature.asc
Description: Digital Signature

Bug#887536: dh-make-perl depends on libemail-address-perl

[Damyan Ivanov]

-=| Pali Rohár, 19.05.2018 18:18:03 +0200 |=-
> On Saturday 19 May 2018 15:28:14 gregor herrmann wrote:
> > On Wed, 17 Jan 2018 20:50:05 +0100, Pali Rohár wrote:
> > 
> > > Hi! Package dh-make-perl depends on libemail-address-perl which is
> > > vulnerable to CVE-2015-7686, see bug #868170. libemail-address-perl
> > > provides perl module Email::Address which is now unmaintained. There is
> > > a new perl module Email::Address::XS which is API compatible replacement
> > > for Email::Address and is available in libemail-address-xs-perl. Please
> > > port dh-make-perl package to use libemail-address-xs-perl. 
> > 
> > dh-make-perl uses
> > 
> > % grep -r Email::Address
> > Build.PL:'Email::Address'=> 0,
> > lib/DhMakePerl/Command/Packaging.pm:use Email::Address;
> > lib/DhMakePerl/Command/Packaging.pm:my $EMAIL_RE = 
> > $Email::Address::addr_spec;
> > 
> > And I think there is no ::addr_spec in libemail-address-xs-perl?
> 
> Yes, Email::Address::XS does not have these regexes defined.
> 
> > > If you need
> > > help with porting let me know.
> > > 
> > Yes, please :)
> 
> I looked at that Packaging.pm file and I'm really not sure that it is
> doing...
> 
> For me it looks like that $PERSON_PARSE_RE just extract phrase (display
> name) from the email address. For this action simple ->parse() method
> should be enough and then ->phrase() would return it.
> 
> $EMAIL_CHANGES_RE seems to extract list of pairs 
> which matches some specific format. So the only thing needed here is to
> check if _address_ is really email address without phrase and angle
> brackets. For parsing ->parse_bare_address() method can be used and then
> check ->address() that returned something.
> 
> I created patch with these changes, but I'm not sure if it is correct
> due to fact that I do not know what that code should do. So it would be
> needed to properly test these changes.
> 
> Anyway, do you really need to parse email address according to RFC2822?
> And is not (.*) in these cases enough?
> 
> Here is patch:

Thanks.

FWIW this code is from Nicolas Bamber and deals with filling 
debian/copyright from the contents of debian/changelog. This is used 
by 'dh-make-perl refresh' and is probably used in some part of the 
test suite.

So I'd say go ahead and apply the patch as it is and either hope that 
somebody will notice if something broke or add a test that explicitly 
covers that specific aspect.


-- dam

Bug#887536: dh-make-perl depends on libemail-address-perl

[gregor herrmann]

Control: tag -1 + patch

On Sat, 19 May 2018 18:18:03 +0200, Pali Rohár wrote:

> > And I think there is no ::addr_spec in libemail-address-xs-perl?
> Yes, Email::Address::XS does not have these regexes defined.

Ok.
 
> > > If you need
> > > help with porting let me know.
> > Yes, please :)
> I looked at that Packaging.pm file and I'm really not sure that it is
> doing...

Thanks alot!

And I agree, it's not totally clear what all this stuff wanted to do
when it was written long ago :)
 
> Here is patch:

I just tried and applied the patch locally, and the testsuite still
passes. (And it seems to test the Packaging.pm as it failed before I
fixed the build dependencies :))

Maybe Dam has time to take a closer look, and I hope to find some
time in the next days as well.


Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   


signature.asc
Description: Digital Signature

Bug#887536: dh-make-perl depends on libemail-address-perl

[Pali Rohár]

On Saturday 19 May 2018 15:28:14 gregor herrmann wrote:
> On Wed, 17 Jan 2018 20:50:05 +0100, Pali Rohár wrote:
> 
> > Hi! Package dh-make-perl depends on libemail-address-perl which is
> > vulnerable to CVE-2015-7686, see bug #868170. libemail-address-perl
> > provides perl module Email::Address which is now unmaintained. There is
> > a new perl module Email::Address::XS which is API compatible replacement
> > for Email::Address and is available in libemail-address-xs-perl. Please
> > port dh-make-perl package to use libemail-address-xs-perl. 
> 
> dh-make-perl uses
> 
> % grep -r Email::Address
> Build.PL:'Email::Address'=> 0,
> lib/DhMakePerl/Command/Packaging.pm:use Email::Address;
> lib/DhMakePerl/Command/Packaging.pm:my $EMAIL_RE = $Email::Address::addr_spec;
> 
> And I think there is no ::addr_spec in libemail-address-xs-perl?

Yes, Email::Address::XS does not have these regexes defined.

> > If you need
> > help with porting let me know.
> > 
> Yes, please :)

I looked at that Packaging.pm file and I'm really not sure that it is
doing...

For me it looks like that $PERSON_PARSE_RE just extract phrase (display
name) from the email address. For this action simple ->parse() method
should be enough and then ->phrase() would return it.

$EMAIL_CHANGES_RE seems to extract list of pairs 
which matches some specific format. So the only thing needed here is to
check if _address_ is really email address without phrase and angle
brackets. For parsing ->parse_bare_address() method can be used and then
check ->address() that returned something.

I created patch with these changes, but I'm not sure if it is correct
due to fact that I do not know what that code should do. So it would be
needed to properly test these changes.

Anyway, do you really need to parse email address according to RFC2822?
And is not (.*) in these cases enough?

Here is patch:

diff --git a/Build.PL b/Build.PL
index eb88fa8..a54fc0f 100644
--- a/Build.PL
+++ b/Build.PL
@@ -25,7 +25,7 @@ my $builder = My::Builder->new(
 'Cwd'   => 0,
 'Dpkg'  => 0,
 'Dpkg::Source::Package' => '1.01',
-'Email::Address'=> 0,
+'Email::Address::XS'=> '1.01',
 'Email::Date::Format'   => 0,
 'File::Basename'=> 0,
 'File::Copy'=> 0,
diff --git a/lib/DhMakePerl/Command/Packaging.pm 
b/lib/DhMakePerl/Command/Packaging.pm
index 8f14caa..9fb9a9e 100644
--- a/lib/DhMakePerl/Command/Packaging.pm
+++ b/lib/DhMakePerl/Command/Packaging.pm
@@ -35,6 +35,7 @@ use Debian::Control::FromCPAN;
 use Debian::Dependencies;
 use Debian::Rules;
 use DhMakePerl::PodParser ();
+use Email::Address::XS 1.01;
 use File::Basename qw(basename dirname);
 use File::Find qw(find);
 use File::Path ();
@@ -1210,31 +1211,6 @@ sub upsurl {
 }
 
 
-my $ACTUAL_NAME_RE = '\pL[\s\pL\-\'\.]*\pL';
-
-# See http://www.faqs.org/rfcs/rfc2822.html
-# Section 3.4.1
-use Email::Address;
-my $EMAIL_RE = $Email::Address::addr_spec;
-
-my $EMAIL_CHANGES_RE = qr{
-^   # beginining of line
-\s+\*\s # item marker
-Email\schange:\s# email change token
-($ACTUAL_NAME_RE)   # actual name
-\s+->\s+# gap between name and email
-($EMAIL_RE) # email address
-$   # end of line
-}xms;
-
-my $PERSON_PARSE_RE = qr{
-\A  # beginining of string
-($ACTUAL_NAME_RE)   # actual name
-\s  # gap
-\<$EMAIL_RE\>   # logged email
-\z  # end of string
-}xms;
-
 # This is what needs fixing.
 sub copyright_from_changelog {
 my ( $self, $firstmaint, $firstyear ) = @_;
@@ -1248,17 +1224,23 @@ sub copyright_from_changelog {
 my $date= $_->Date;
 my @date_pieces = split( " ", $date );
 my $year= $date_pieces[3];
-if (my %changes = ($_->Changes =~ m/$EMAIL_CHANGES_RE/xmsg)) {
+if (my %changes = ($_->Changes =~ 
m/^\s+\*\sEmail\schange:\s+(.*?)\s+->\s+(.*?)\s*$/xmsg)) {
 # This way round since we are going backward in time thru changelog
 foreach my $p (keys %changes) {
-$changes{$p} =~ s{[\s\n]+$}{}xms;
+# Parse bare email address; undef if it not an email address
+my $address = 
Email::Address::XS->parse_bare_address($changes{$p})->address();
+if ($address) {
+$changes{$p} = $address;
+} else {
+delete $changes{$p};
+}
 }
 %email_changes = (
 %changes,
 %email_changes
 );
 }
-if (my ($name) = ($person =~ $PERSON_PARSE_RE)) {
+if (my $name = 

Bug#887536: dh-make-perl depends on libemail-address-perl

[gregor herrmann]

On Wed, 17 Jan 2018 20:50:05 +0100, Pali Rohár wrote:

> Hi! Package dh-make-perl depends on libemail-address-perl which is
> vulnerable to CVE-2015-7686, see bug #868170. libemail-address-perl
> provides perl module Email::Address which is now unmaintained. There is
> a new perl module Email::Address::XS which is API compatible replacement
> for Email::Address and is available in libemail-address-xs-perl. Please
> port dh-make-perl package to use libemail-address-xs-perl. 

dh-make-perl uses

% grep -r Email::Address
Build.PL:'Email::Address'=> 0,
lib/DhMakePerl/Command/Packaging.pm:use Email::Address;
lib/DhMakePerl/Command/Packaging.pm:my $EMAIL_RE = $Email::Address::addr_spec;

And I think there is no ::addr_spec in libemail-address-xs-perl?

> If you need
> help with porting let me know.
> 
Yes, please :)


Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   


signature.asc
Description: Digital Signature

Bug#887536: dh-make-perl depends on libemail-address-perl

[Pali Rohár]

Package: dh-make-perl
Version: 0.98
Severity: wishlist

Hi! Package dh-make-perl depends on libemail-address-perl which is
vulnerable to CVE-2015-7686, see bug #868170. libemail-address-perl
provides perl module Email::Address which is now unmaintained. There is
a new perl module Email::Address::XS which is API compatible replacement
for Email::Address and is available in libemail-address-xs-perl. Please
port dh-make-perl package to use libemail-address-xs-perl. If you need
help with porting let me know.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature