lib/perlbug.t: avoid spurious failure when testing long PATH line

[p5pRT]

Migrated from rt.perl.org#129048 (status was 'open')

Searchable as RT129048$

[p5pRT]

From @jkeenan

Please see perl5-porters discussion beginning here​:

http​://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239185.html

lib/perlbug.t holds tests which sanity-check the operation of the 'perlbug' utility by creating bug reports without actually sending them. Earlier this year we added tests (c04bead) provided by Niko Tyni with respect to this rationale​:

#####
Mail transport agents limit the length of message lines at SMTP time. One observed limit is 1000 characters per line. ... [perlbug] needs to limit the length of its lines manually to make sure bug reports get delivered.
#####

Whether this objective is met is tested by this code​:

#####
my $B = 'B'x9;
...
my $maxlen1 = 0; # body
my $maxlen2 = 0; # attachment
for (split(/\n/, $contents)) {
  my $len = length;
  $maxlen1 = $len if $len > $maxlen1 and !/$B/;
  $maxlen2 = $len if $len > $maxlen2 and /$B/;
}
ok($maxlen1 < 1000, "[perl #128020] long body lines are wrapped​: maxlen $maxlen1");
ok($maxlen2 > 1000, "long attachment lines are not wrapped​: maxlen $maxlen2");
#####

In this case, $B is used simply to generate dummy copy for an attachment. So, in the first of the two unit tests shown, we're testing to make sure that no bug report line other than in an attachment exceeds 1000 characters.

There is however, one line in a perlbug report which can quite plausibly exceed 1000 *unbroken* characters​: the line dumping the PATH environmental variable. No (unescaped) wordspaces are permitted in a PATH, so the line does not wrap. A PATH line greater than 1000 characters in length can occur in at least two plausible circumstances​: (1) For business reasons, the user has many directories in her path; or (2) The PATH (a) is shorter than 1000 characters; (b) terminates with '​:$PATH'; and (c) is incremented in a particular shell -- a shell in which 'lib/perlbug.t' is subsequently run -- by use of commands like 'source ~/.bashrc' which the user is invoking to modify a .*rc or .profile file.

This can cause the kind of entirely spurious test failure reported in the p5p thread cited above​:

#####
ok 20 - attachment included in fake bug report
not ok 21 - [perl \#128020] long body lines are wrapped​: maxlen 1133
ok 22 - long attachment lines are not wrapped​: maxlen 1200
# Failed test 21 - [perl \#128020] long body lines are wrapped​: maxlen
1133 at ../lib/perlbug.t line 154
Failed 1/22 subtests
#####

The patch attached excludes the 'PATH' line from test #21 in lib/perlbug.t and thereby reduces the possibility of spurious failures. Here is how I tested this in a fresh shell.

#####
[perl] 13 $ echo -n $PATH | wc -m
432
[perl] 14 $ source ~/.bashrc
[perl] 15 $ source ~/.bashrc
[perl] 16 $ source ~/.bashrc
[perl] 17 $ echo -n $PATH | wc -m
1128
# configure and build perl in branch to which patch has been applied

$ cd t;./perl harness -v ../lib/perlbug.t; cd -

ok 1 - config information dumped with -d
[snip]
ok 20 - attachment included in fake bug report
ok 21 - [perl \#128020] long body lines are wrapped​: maxlen 899
ok 22 - long attachment lines are not wrapped​: maxlen 1200
ok
All tests successful.
Files=1, Tests=22, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.58 cusr 0.03 csys = 0.62 CPU)
Result​: PASS
#####

Please review.

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @jkeenan

Please see perl5-porters discussion beginning here​:

http​://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239185.html

lib/perlbug.t holds tests which sanity-check the operation of the 'perlbug' utility by creating bug reports without actually sending them. Earlier this year we added tests (c04bead) provided by Niko Tyni with respect to this rationale​:

#####
Mail transport agents limit the length of message lines at SMTP time. One observed limit is 1000 characters per line. ... [perlbug] needs to limit the length of its lines manually to make sure bug reports get delivered.
#####

Whether this objective is met is tested by this code​:

#####
my $B = 'B'x9;
...
my $maxlen1 = 0; # body
my $maxlen2 = 0; # attachment
for (split(/\n/, $contents)) {
  my $len = length;
  $maxlen1 = $len if $len > $maxlen1 and !/$B/;
  $maxlen2 = $len if $len > $maxlen2 and /$B/;
}
ok($maxlen1 < 1000, "[perl #128020] long body lines are wrapped​: maxlen $maxlen1");
ok($maxlen2 > 1000, "long attachment lines are not wrapped​: maxlen $maxlen2");
#####

In this case, $B is used simply to generate dummy copy for an attachment. So, in the first of the two unit tests shown, we're testing to make sure that no bug report line other than in an attachment exceeds 1000 characters.

There is however, one line in a perlbug report which can quite plausibly exceed 1000 *unbroken* characters​: the line dumping the PATH environmental variable. No (unescaped) wordspaces are permitted in a PATH, so the line does not wrap. A PATH line greater than 1000 characters in length can occur in at least two plausible circumstances​: (1) For business reasons, the user has many directories in her path; or (2) The PATH (a) is shorter than 1000 characters; (b) terminates with '​:$PATH'; and (c) is incremented in a particular shell -- a shell in which 'lib/perlbug.t' is subsequently run -- by use of commands like 'source ~/.bashrc' which the user is invoking to modify a .*rc or .profile file.

This can cause the kind of entirely spurious test failure reported in the p5p thread cited above​:

#####
ok 20 - attachment included in fake bug report
not ok 21 - [perl \#128020] long body lines are wrapped​: maxlen 1133
ok 22 - long attachment lines are not wrapped​: maxlen 1200
# Failed test 21 - [perl \#128020] long body lines are wrapped​: maxlen
1133 at ../lib/perlbug.t line 154
Failed 1/22 subtests
#####

The patch attached excludes the 'PATH' line from test #21 in lib/perlbug.t and thereby reduces the possibility of spurious failures. Here is how I tested this in a fresh shell.

#####
[perl] 13 $ echo -n $PATH | wc -m
432
[perl] 14 $ source ~/.bashrc
[perl] 15 $ source ~/.bashrc
[perl] 16 $ source ~/.bashrc
[perl] 17 $ echo -n $PATH | wc -m
1128
# configure and build perl in branch to which patch has been applied

$ cd t;./perl harness -v ../lib/perlbug.t; cd -

ok 1 - config information dumped with -d
[snip]
ok 20 - attachment included in fake bug report
ok 21 - [perl \#128020] long body lines are wrapped​: maxlen 899
ok 22 - long attachment lines are not wrapped​: maxlen 1200
ok
All tests successful.
Files=1, Tests=22, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.58 cusr 0.03 csys = 0.62 CPU)
Result​: PASS
#####

Please review.

Thank you very much.
Jim Keenan

[p5pRT]

From @ntyni

On Mon, Aug 22, 2016 at 06​:28​:59AM -0700, James E Keenan wrote​:

# New Ticket Created by James E Keenan
# Please include the string​: [perl #129048]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129048 >

Please see perl5-porters discussion beginning here​:

http​://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239185.html

lib/perlbug.t holds tests which sanity-check the operation of the 'perlbug' utility by creating bug reports without actually sending them. Earlier this year we added tests (c04bead) provided by Niko Tyni with respect to this rationale​:

#####
Mail transport agents limit the length of message lines at SMTP time. One observed limit is 1000 characters per line. ... [perlbug] needs to limit the length of its lines manually to make sure bug reports get delivered.
#####

There is however, one line in a perlbug report which can quite plausibly exceed 1000 *unbroken* characters​: the line dumping the PATH environmental variable. No (unescaped) wordspaces are permitted in a PATH, so the line does not wrap. A PATH line greater than 1000 characters in length can occur in at least two plausible circumstances​: (1) For business reasons, the user has many directories in her path; or (2) The PATH (a) is shorter than 1000 characters; (b) terminates with '​:$PATH'; and (c) is incremented in a particular shell -- a shell in which 'lib/perlbug.t' is subsequently run -- by use of commands like 'source ~/.bashrc' which the user is invoking to modify a .*rc or .profile file.

The patch attached excludes the 'PATH' line from test #21 in lib/perlbug.t and thereby reduces the possibility of spurious failures. Here is how I tested this in a fresh shell.

Thanks for looking at this. The patch seems to have been lost at some point?

Allowing long unbroken PATH lines in the report means that bug submissions
with a long PATH will face a risk of getting dropped at SMTP time. A
more robust fix would be wrapping the PATH line even though there's no
whitespace - probably at a colon ('​:').

That said, excluding PATH from the wrapping check could still be
considered a valid hotfix​: the test has just revealed a longstanding
issue that is (IMO) less urgent than the spurious test failures. Maybe
the test could be marked as 'TODO' for PATH lines somehow?
--
Niko Tyni ntyni@​debian.org

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From zefram@fysh.org

Niko Tyni wrote​:

more robust fix would be wrapping the PATH line even though there's no
whitespace - probably at a colon ('​:').

There's a better way​: emit the message in quoted-printable encoding,
with the appropriate MIME headers. Q-p allows a line to be wrapped at
the RFC 822 level (and hence at the SMTP level) without introducing a
line break to the represented content.

-zefram

[p5pRT]

From @jkeenan

On Wed Aug 24 12​:46​:02 2016, ntyni@​debian.org wrote​:

On Mon, Aug 22, 2016 at 06​:28​:59AM -0700, James E Keenan wrote​:

# New Ticket Created by James E Keenan
# Please include the string​: [perl #129048]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129048 >

Please see perl5-porters discussion beginning here​:

http​://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239185.html

lib/perlbug.t holds tests which sanity-check the operation of the
'perlbug' utility by creating bug reports without actually sending
them. Earlier this year we added tests (c04bead) provided by Niko
Tyni with respect to this rationale​:

#####
Mail transport agents limit the length of message lines at SMTP time.
One observed limit is 1000 characters per line. ... [perlbug] needs
to limit the length of its lines manually to make sure bug reports
get delivered.
#####

There is however, one line in a perlbug report which can quite
plausibly exceed 1000 *unbroken* characters​: the line dumping the
PATH environmental variable. No (unescaped) wordspaces are permitted
in a PATH, so the line does not wrap. A PATH line greater than 1000
characters in length can occur in at least two plausible
circumstances​: (1) For business reasons, the user has many
directories in her path; or (2) The PATH (a) is shorter than 1000
characters; (b) terminates with '​:$PATH'; and (c) is incremented in a
particular shell -- a shell in which 'lib/perlbug.t' is subsequently
run -- by use of commands like 'source ~/.bashrc' which the user is
invoking to modify a .*rc or .profile file.

The patch attached excludes the 'PATH' line from test #21 in
lib/perlbug.t and thereby reduces the possibility of spurious
failures. Here is how I tested this in a fresh shell.

Thanks for looking at this. The patch seems to have been lost at some
point?

Indeed! My error. Attaching original intended patch now.

Allowing long unbroken PATH lines in the report means that bug
submissions
with a long PATH will face a risk of getting dropped at SMTP time. A
more robust fix would be wrapping the PATH line even though there's no
whitespace - probably at a colon ('​:').

That said, excluding PATH from the wrapping check could still be
considered a valid hotfix​: the test has just revealed a longstanding
issue that is (IMO) less urgent than the spurious test failures. Maybe
the test could be marked as 'TODO' for PATH lines somehow?

--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @jkeenan

0001-Avoid-spurious-test-failure-due-to-PATH-line-1000-ch.patch

From b197f9a55e2ae877b3089282cfe07f3647d240f9 Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Mon, 22 Aug 2016 09:25:08 -0400
Subject: [PATCH] Avoid spurious test failure due to PATH line > 1000
 characters.

---
 lib/perlbug.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/perlbug.t b/lib/perlbug.t
index ed32c04..8ff8991 100644
--- a/lib/perlbug.t
+++ b/lib/perlbug.t
@@ -148,7 +148,7 @@ my $maxlen1 = 0; # body
 my $maxlen2 = 0; # attachment
 for (split(/\n/, $contents)) {
         my $len = length;
-        $maxlen1 = $len if $len > $maxlen1 and !/$B/;
+        $maxlen1 = $len if $len > $maxlen1 and ! (/(?:$B|PATH)/);
         $maxlen2 = $len if $len > $maxlen2 and  /$B/;
 }
 ok($maxlen1 < 1000, "[perl #128020] long body lines are wrapped: maxlen $maxlen1");
-- 
2.7.4

[p5pRT]

From @ntyni

On Wed, Aug 24, 2016 at 09​:45​:12PM +0100, Zefram wrote​:

Niko Tyni wrote​:

more robust fix would be wrapping the PATH line even though there's no
whitespace - probably at a colon ('​:').

There's a better way​: emit the message in quoted-printable encoding,
with the appropriate MIME headers. Q-p allows a line to be wrapped at
the RFC 822 level (and hence at the SMTP level) without introducing a
line break to the represented content.

Indeed, at the cost of some added complexity that I was hesitant
to add initially. It may well be time for that.

I see there's q-p functionality in cpan/MIME-Base64/, but that relies on
XS code. Would it be OK for perlbug to use that? I suppose it would just
have to fall back gracefully to skip the encoding phase if MIME​::Base64
can't be loaded.
--
Niko Tyni ntyni@​debian.org

[jkeenan]

P5P: What is the status of the perlbug utility subsequent to transition from rt.perl.org to GH issues?

If perlbug is dead, then this ticket is moot and can be closed.

Thank you very much.
Jim Keenan

[JJ]

Still happening in perl 5.34 and Focal Fossa

[hakonhagland]

Added a PR to fix this: see #20497.