Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Niko Tyni]

Control: forwarded -1 https://rt.perl.org/Ticket/Display.html?id=128020

On Thu, Apr 28, 2016 at 08:17:09PM +0100, Dominic Hargreaves wrote:
> On Thu, Apr 28, 2016 at 07:05:40PM +0300, Niko Tyni wrote:
> > On Tue, Apr 26, 2016 at 09:55:38PM +0300, Niko Tyni wrote:
> > 
> > > A better fix would be be to wrap everything, but I don't have a good patch
> > > for that. Hacking build_complete_message() could work I guess, but it's
> > > not used by all the sending backends (at least _send_message_mailsend();
> > > not sure we need to care about that.)
> > > 
> > > I think I need to look at this a bit more.
> > 
> > Revised set of two patches attached. Eyeballs would be welcome.
> 
> This looks sane to me. Thanks for taking the time to refactor!

Thank you for the review. I had to make one more modification to avoid
Text::Wrap eating empty lines from the body. Attached is the version I
sent upstream in [perl #128020].
-- 
Niko
>From 64986731cebb7768237489e00a15f93be660751f Mon Sep 17 00:00:00 2001
From: Niko Tyni 
Date: Thu, 28 Apr 2016 18:50:17 +0300
Subject: [PATCH 1/2] perlbug: Refactor duplicated file reading code

_send_message_mailsend() needs to build the message itself rather than
calling build_complete_message() like the other backends, but they can
still share the file reading code, and so can the 'display report' part.
---
 utils/perlbug.PL | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 885785a..6d6c780 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -834,10 +834,7 @@ EOF
 if ( SaveMessage() ) { exit }
 	} elsif ($action =~ /^(d|l|sh)/i ) { # isplay, ist, ow
 		# Display the message
-		open(REP, '<:raw', $filename) or die "Couldn't open file '$filename': $!\n";
-		binmode(REP, ':raw :crlf') if $Is_MSWin32;
-		while () { print $_ }
-		close(REP) or die "Error closing report file '$filename': $!";
+		print _read_report($filename);
 		if ($have_attachment) {
 		print "\n\n---\nAttachment(s):\n";
 		for my $att (split /\s*,\s*/, $attachments) { print "$att\n"; }
@@ -1080,13 +1077,20 @@ ATTACHMENT
 return $attach;
 }
 
+sub _read_report {
+my $fname = shift;
+my $content;
+open( REP, "<:raw", $fname ) or die "Couldn't open file '$fname': $!\n";
+binmode(REP, ':raw :crlf') if $Is_MSWin32;
+while () { $content .= $_; }
+close(REP) or die "Error closing report file '$fname': $!";
+return $content;
+}
+
 sub build_complete_message {
 my $content = _build_header(%{_message_headers()}) . "\n\n";
 $content .= _add_body_start() if $have_attachment;
-open( REP, "<:raw", $filename ) or die "Couldn't open file '$filename': $!\n";
-binmode(REP, ':raw :crlf') if $Is_MSWin32;
-while () { $content .= $_; }
-close(REP) or die "Error closing report file '$filename': $!";
+$content .= _read_report($filename);
 $content .= _add_attachments() if $have_attachment;
 return $content;
 }
@@ -1137,10 +1141,7 @@ sub _send_message_mailsend {
 $fh = $msg->open;
 binmode($fh, ':raw');
 print $fh _add_body_start() if $have_attachment;
-open(REP, "<:raw", $filename) or die "Couldn't open '$filename': $!\n";
-binmode(REP, ':raw :crlf') if $Is_MSWin32;
-while () { print $fh $_ }
-close(REP) or die "Error closing $filename: $!";
+print $fh _read_report($filename);
 print $fh _add_attachments() if $have_attachment;
 $fh->close or die "Error sending mail: $!";
 
-- 
2.8.0.rc3

>From 4bfb47f4d3a237934e9abe181e04c405c928067c Mon Sep 17 00:00:00 2001
From: Niko Tyni 
Date: Mon, 25 Apr 2016 16:31:00 +0300
Subject: [PATCH 2/2] Make perlbug wrap overly long lines

Mail transport agents limit the length of message lines at SMTP time.
One observed limit is 1000 characters per line. Mail user agents typically
work around these limits by MIME-encoding the message. Since perlbug
doesn't do that, it needs to limit the length of its lines manually to
make sure bug reports get delivered.

The longest lines in perlbug reports normally come from Config::myconfig
output, particularly 'config_args', which has been observed to exceed
1000 characters on some configurations, causing report rejection. While
less likely, the list of local patches is another potential source of
overly long lines.

Use Text::Wrap (if available) to wrap the body of the report at an
arbitrarily chosen and hopefully safe limit of 900 characters. No
indentation or continuation line markers are added, though it would
be easy to add those if desired. Attachments and mail headers are not
wrapped.

Bug-Debian: https://bugs.debian.org/822463
---
 utils/perlbug.PL | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 6d6c780..6bc5bad 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -76,6 +76,8 @@ BEGIN {
 $::HaveTemp = ($@ eq "");
 eval { require Module::CoreList; };
 $::Have

Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Dominic Hargreaves]

On Thu, Apr 28, 2016 at 07:05:40PM +0300, Niko Tyni wrote:
> On Tue, Apr 26, 2016 at 09:55:38PM +0300, Niko Tyni wrote:
> 
> > A better fix would be be to wrap everything, but I don't have a good patch
> > for that. Hacking build_complete_message() could work I guess, but it's
> > not used by all the sending backends (at least _send_message_mailsend();
> > not sure we need to care about that.)
> > 
> > I think I need to look at this a bit more.
> 
> Revised set of two patches attached. Eyeballs would be welcome.

This looks sane to me. Thanks for taking the time to refactor!

Cheers,
Dominic.

Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Niko Tyni]

On Tue, Apr 26, 2016 at 09:55:38PM +0300, Niko Tyni wrote:

> A better fix would be be to wrap everything, but I don't have a good patch
> for that. Hacking build_complete_message() could work I guess, but it's
> not used by all the sending backends (at least _send_message_mailsend();
> not sure we need to care about that.)
> 
> I think I need to look at this a bit more.

Revised set of two patches attached. Eyeballs would be welcome.
-- 
Niko
>From b8d0877675fec1968305583ea1b3c598af7a336a Mon Sep 17 00:00:00 2001
From: Niko Tyni 
Date: Thu, 28 Apr 2016 18:50:17 +0300
Subject: [PATCH 1/2] perlbug: Refactor duplicated file reading code

_send_message_mailsend() needs to build the message itself rather than
calling build_complete_message() like the other backends, but they can
still share the file reading code, and so can the 'display report' part.
---
 utils/perlbug.PL | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 885785a..6d6c780 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -834,10 +834,7 @@ EOF
 if ( SaveMessage() ) { exit }
 	} elsif ($action =~ /^(d|l|sh)/i ) { # isplay, ist, ow
 		# Display the message
-		open(REP, '<:raw', $filename) or die "Couldn't open file '$filename': $!\n";
-		binmode(REP, ':raw :crlf') if $Is_MSWin32;
-		while () { print $_ }
-		close(REP) or die "Error closing report file '$filename': $!";
+		print _read_report($filename);
 		if ($have_attachment) {
 		print "\n\n---\nAttachment(s):\n";
 		for my $att (split /\s*,\s*/, $attachments) { print "$att\n"; }
@@ -1080,13 +1077,20 @@ ATTACHMENT
 return $attach;
 }
 
+sub _read_report {
+my $fname = shift;
+my $content;
+open( REP, "<:raw", $fname ) or die "Couldn't open file '$fname': $!\n";
+binmode(REP, ':raw :crlf') if $Is_MSWin32;
+while () { $content .= $_; }
+close(REP) or die "Error closing report file '$fname': $!";
+return $content;
+}
+
 sub build_complete_message {
 my $content = _build_header(%{_message_headers()}) . "\n\n";
 $content .= _add_body_start() if $have_attachment;
-open( REP, "<:raw", $filename ) or die "Couldn't open file '$filename': $!\n";
-binmode(REP, ':raw :crlf') if $Is_MSWin32;
-while () { $content .= $_; }
-close(REP) or die "Error closing report file '$filename': $!";
+$content .= _read_report($filename);
 $content .= _add_attachments() if $have_attachment;
 return $content;
 }
@@ -1137,10 +1141,7 @@ sub _send_message_mailsend {
 $fh = $msg->open;
 binmode($fh, ':raw');
 print $fh _add_body_start() if $have_attachment;
-open(REP, "<:raw", $filename) or die "Couldn't open '$filename': $!\n";
-binmode(REP, ':raw :crlf') if $Is_MSWin32;
-while () { print $fh $_ }
-close(REP) or die "Error closing $filename: $!";
+print $fh _read_report($filename);
 print $fh _add_attachments() if $have_attachment;
 $fh->close or die "Error sending mail: $!";
 
-- 
2.8.0.rc3

>From c2d3290477e3bd58f553522cc2dc872577469054 Mon Sep 17 00:00:00 2001
From: Niko Tyni 
Date: Mon, 25 Apr 2016 16:31:00 +0300
Subject: [PATCH 2/2] Make perlbug wrap overly long lines

Mail transport agents limit the length of message lines at SMTP time.
One observed limit is 1000 characters per line. Mail user agents typically
work around these limits by MIME-encoding the message. Since perlbug
doesn't do that, it needs to limit the length of its lines manually to
make sure bug reports get delivered.

The longest lines in perlbug reports normally come from Config::myconfig
output, particularly 'config_args', which has been observed to exceed
1000 characters on some configurations, causing report rejection. While
less likely, the list of local patches is another potential source of
overly long lines.

Use Text::Wrap (if available) to wrap the body of the report at an
arbitrarily chosen and hopefully safe limit of 900 characters. No
indentation or continuation line markers are added, though it would
be easy to add those if desired. Attachments and mail headers are not
wrapped.

Bug-Debian: https://bugs.debian.org/822463
---
 utils/perlbug.PL | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 6d6c780..ec60224 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -76,6 +76,8 @@ BEGIN {
 $::HaveTemp = ($@ eq "");
 eval { require Module::CoreList; };
 $::HaveCoreList = ($@ eq "");
+eval { require Text::Wrap; };
+$::HaveWrap = ($@ eq "");
 };
 
 my $Version = "1.40";
@@ -1082,7 +1084,16 @@ sub _read_report {
 my $content;
 open( REP, "<:raw", $fname ) or die "Couldn't open file '$fname': $!\n";
 binmode(REP, ':raw :crlf') if $Is_MSWin32;
-while () { $content .= $_; }
+# wrap long lines to make sure the report gets delivered
+local $Text::Wrap::columns = 900;
+local $Text::Wrap::huge = 'overflow';
+   

Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Niko Tyni]

On Mon, Apr 25, 2016 at 09:46:02PM +0100, Dominic Hargreaves wrote:

> The patch looks good. However the example output looks ugly for me
> (particuarly the 100-char one, where most of the patch descriptions
> are uglified by a wrapping out of sync with my 80 character terminal).

I see. The lowest common denominator would indeed be 80 characters.
The problem with wrapping there is that some of the "normal" lines like

intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, 
doublekind=3

currently go over that, and it seems silly to wrap those.
 
> If we are wrapping for technical reasons rather than aesthetic ones,
> then the choice of max length should be based on those technical reasons.
> From [1] the maximum line length appears to be around the 1000 character
> area. So call it 900 to be safe? I don't think we should wrap more
> aggressively than is required to fit within that standard; that way, we
> minimise the damage caused by wrapping, when displayed by MUAs which
> themselves wrap.

I suppose you're right. Not sure if it makes sense to intend the wrapped
line at all in that case.

> I'm not sure what sort of warning is necessary; could you be more
> specific here?

We aren't wrapping everything (at least the environment dump and
the actual user-supplied text aren't modified by the patch), so it's
still possible to hit the limit. I was thinking about documenting this
possibility and advising users to keep well under the limit.

A better fix would be be to wrap everything, but I don't have a good patch
for that. Hacking build_complete_message() could work I guess, but it's
not used by all the sending backends (at least _send_message_mailsend();
not sure we need to care about that.)

I think I need to look at this a bit more.

Thanks for your input, much appreciated!
-- 
Niko

Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Dominic Hargreaves]

On Mon, Apr 25, 2016 at 06:11:50PM +0300, Niko Tyni wrote:
> Control: tag -1 patch
> 
> On Sun, Apr 24, 2016 at 10:25:23PM +0300, Niko Tyni wrote:
> > Package: perl
> > Version: 5.22.1-10
> 
> > So it looks like perlbug is broken on stretch/sid with the default exim4
> > configuration, and exim4 is the default MTA on Debian AFAIK. I think we
> > need to fix this.
> 
> > The options I see are
> > 
> > - make exim4 handle longer lines
> > - somehow shorten the longest line, probably by changing the way we call
> >   Configure
> > - make perlbug split or shorten overly long lines
> > 
> > I think the third option is the best one from a robustness point of view.
> > The 'correct' fix would be to teach perlbug to use MIME quoted-printable
> > or something like that, but that's probably overkill. Maybe just splitting
> > Config::myconfig() results where necessary and adding a warning to the
> > docs would be enough?
> 
> Tentative perlbug patch attached. Not sure where to put a warning if one
> is still needed.
> 
> Opinions before I take this upstream? Example reports wrapped at
> 200 and 100 characters attached. I think I prefer the 100-char one.

The patch looks good. However the example output looks ugly for me
(particuarly the 100-char one, where most of the patch descriptions
are uglified by a wrapping out of sync with my 80 character terminal).

If we are wrapping for technical reasons rather than aesthetic ones,
then the choice of max length should be based on those technical reasons.
>From [1] the maximum line length appears to be around the 1000 character
area. So call it 900 to be safe? I don't think we should wrap more
aggressively than is required to fit within that standard; that way, we
minimise the damage caused by wrapping, when displayed by MUAs which
themselves wrap.

I'm not sure what sort of warning is necessary; could you be more
specific here?

Cheers,
Dominic.

[1] 

Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Niko Tyni]

Control: tag -1 patch

On Sun, Apr 24, 2016 at 10:25:23PM +0300, Niko Tyni wrote:
> Package: perl
> Version: 5.22.1-10

> So it looks like perlbug is broken on stretch/sid with the default exim4
> configuration, and exim4 is the default MTA on Debian AFAIK. I think we
> need to fix this.

> The options I see are
> 
> - make exim4 handle longer lines
> - somehow shorten the longest line, probably by changing the way we call
>   Configure
> - make perlbug split or shorten overly long lines
> 
> I think the third option is the best one from a robustness point of view.
> The 'correct' fix would be to teach perlbug to use MIME quoted-printable
> or something like that, but that's probably overkill. Maybe just splitting
> Config::myconfig() results where necessary and adding a warning to the
> docs would be enough?

Tentative perlbug patch attached. Not sure where to put a warning if one
is still needed.

Opinions before I take this upstream? Example reports wrapped at
200 and 100 characters attached. I think I prefer the 100-char one.
-- 
Niko
>From a2bfdab19eb99920d0a171588b45b7102b0e7492 Mon Sep 17 00:00:00 2001
From: Niko Tyni 
Date: Mon, 25 Apr 2016 16:31:00 +0300
Subject: [PATCH] Make perlbug wrap overly long lines

Mail transport agents limit the length of message lines at SMTP time.
One observed limit is 1000 characters per line. Mail user agents typically
work around these limits by MIME-encoding the message. Since perlbug
doesn't do that, it needs to limit the length of its lines manually to
make sure bug reports get delivered.

The longest lines in perlbug reports normally come from Config::myconfig
output, particularly 'config_args', which has been observed to exceed
1000 characters on some configurations, causing a rejected report. While
less likely, the list of local patches is another potential source of
overly long lines.

Use Text::Wrap (if available) to wrap these parts of the report at
an arbitrarily chosen limit of 200 characters. Continuation lines are
intended with six spaces, arguably making an even lower limit attractive
for aesthetic reasons.

Bug-Debian: https://bugs.debian.org/822463
---
 utils/perlbug.PL | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 885785a..c0f9240 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -76,6 +76,8 @@ BEGIN {
 $::HaveTemp = ($@ eq "");
 eval { require Module::CoreList; };
 $::HaveCoreList = ($@ eq "");
+eval { require Text::Wrap; };
+$::HaveWrap = ($@ eq "");
 };
 
 my $Version = "1.40";
@@ -699,12 +701,24 @@ EOF
 if ($::Config{cf_by} and $::Config{cf_time}) {
 	print OUT "Configured by $::Config{cf_by} at $::Config{cf_time}.\n\n";
 }
-print OUT Config::myconfig;
-
-if (@patches) {
-	print OUT join "\n", "Locally applied patches:", @patches;
-	print OUT "\n";
-};
+if ($::HaveWrap) {
+# wrap long lines to make sure the report gets delivered
+local $Text::Wrap::columns = 200;
+local $Text::Wrap::huge = 'overflow';
+print OUT join("\n", map { Text::Wrap::wrap('', ' 'x6, $_) }
+split(/\n/, Config::myconfig));
+if (@patches) {
+print OUT join "\n", "Locally applied patches:",
+map { Text::Wrap::wrap('', ' 'x6, $_) } @patches;
+print OUT "\n";
+}
+} else {
+print OUT Config::myconfig;
+if (@patches) {
+print OUT join "\n", "Locally applied patches:", @patches;
+print OUT "\n";
+}
+}
 
 print OUT 
From: Niko Tyni 
To: perl...@perl.org
Subject: Wrapping test
Reply-To: Niko Tyni 


This is a bug report for perl from Niko Tyni ,
generated with the help of perlbug 1.40 running under perl 5.22.1.


-
[Please describe your issue here]

This is how a perlbug report wrapped to 200 characters would look like.

[Please do not change anything below this line]
-
---
Flags:
category=core
severity=low
---
Site configuration information for perl 5.22.1:

Configured by Debian Project at Sun Mar 13 11:54:18 UTC 2016.

Summary of my perl5 (revision 5 version 22 subversion 1) configuration:

  Platform:
osname=linux, osvers=3.16.0, archname=x86_64-linux-gnu-thread-multi
uname='linux localhost 3.16.0 #1 smp debian 3.16.0 x86_64 gnulinux '
config_args='-Dusethreads -Duselargefiles -Dcc=x86_64-linux-gnu-gcc 
-Dcpp=x86_64-linux-gnu-cpp -Dld=x86_64-linux-gnu-gcc -Dccflags=-DDEBIAN 
-Wdate-time -D_FORTIFY_SOURCE=2 -g -O2
  -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= 
-Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC 
-Darchname=x86_64-linux-gnu -Dprefix=/usr
  -Dprivlib=/usr/share/perl/5.22 
-Darchlib=/usr/lib/x86_64-linux-gnu/p

Bug#822463: perl: perlbug reports get rejected by exim4 due to long lines in 'perl -V'

[Niko Tyni]

Package: perl
Version: 5.22.1-10

Not quite sure of the correct severity.

My 'perlbug' report was rejected by the local exim4 MTA with 

2016-04-24 18:28:06 1auLx8-0003Rd-1l ** perl...@perl.org R=smarthost 
T=remote_smtp_smarthost: message is too big 
(transport limit = 1)

Turns out 'transport limit = 1' actually means
'line length > 998 characters', as defined in
  /etc/exim4/conf.d/transport/30_exim4-config_remote_smtp_smarthost
with this:
  # Refuse to send any messsage with over-long lines, which could have
  # been received other than via SMTP. The use of message_size_limit to
  # enforce this is a red herring.
  [...]
  message_size_limit = ${if > {$max_received_linelength}{998} {1}{0}}

which seems to be the default configuration on Debian.

Now, perlbug includes the equivalent of 'perl -V' output (Config::myconfig())
in the report, and

  % perl -V | grep config_args|wc
1  481001

So it looks like perlbug is broken on stretch/sid with the default exim4
configuration, and exim4 is the default MTA on Debian AFAIK. I think we
need to fix this.

Fortunately this was lower on jessie so this only concerns stretch and
sid. On the other hand, Ubuntu just released an LTS version with 5.22.1-9
which is presumably affected too (though I don't think they use exim4
by default.)

The options I see are

- make exim4 handle longer lines
- somehow shorten the longest line, probably by changing the way we call
  Configure
- make perlbug split or shorten overly long lines

I think the third option is the best one from a robustness point of view.
The 'correct' fix would be to teach perlbug to use MIME quoted-printable
or something like that, but that's probably overkill. Maybe just splitting
Config::myconfig() results where necessary and adding a warning to the
docs would be enough?
-- 
Niko Tyni   nt...@debian.org