Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
p5pRT opened this issue Aug 22, 2016 · 11 comments
Open

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

p5pRT opened this issue Aug 22, 2016 · 11 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 22, 2016

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

Searchable as RT129048$

@p5pRT
Copy link
Author

p5pRT commented Aug 22, 2016

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
Copy link
Author

p5pRT commented Aug 22, 2016

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
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 24, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 25, 2016

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
Copy link
Contributor

jkeenan commented Jan 31, 2020

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

haampie pushed a commit to spack/spack that referenced this issue Sep 27, 2021
…26245)

Fix the perl test case bug Perl/perl5#15544
Variable PATH longer than 1000 characters (as is usual with spack) fails a perl test case
The fix is: Don't test PATH in testcase perlbug.t
Fixes `spack install --test=all` for specs triggering a build and test of perl!
@xenu xenu removed the affects-5.25 label Nov 19, 2021
@xenu xenu removed the Severity Low label Dec 29, 2021
@JJ
Copy link

JJ commented Jun 18, 2022

Still happening in perl 5.34 and Focal Fossa

@hakonhagland
Copy link
Contributor

Added a PR to fix this: see #20497.

khwilliamson pushed a commit to khwilliamson/perl5 that referenced this issue Feb 7, 2023
This fixes issue Perl#15544. If the user has PATH, or similar variables
containing a list of directories separated by e.g. a colon character
these can easily become lines with more than 1000 characters in a perlbug
report, resulting in a test failure in lib/perlbug.t.
pjacklam pushed a commit to pjacklam/perl5 that referenced this issue May 20, 2023
This fixes issue Perl#15544. If the user has PATH, or similar variables
containing a list of directories separated by e.g. a colon character
these can easily become lines with more than 1000 characters in a perlbug
report, resulting in a test failure in lib/perlbug.t.
pjacklam pushed a commit to pjacklam/perl5 that referenced this issue May 20, 2023
This fixes issue Perl#15544. If the user has PATH, or similar variables
containing a list of directories separated by e.g. a colon character
these can easily become lines with more than 1000 characters in a perlbug
report, resulting in a test failure in lib/perlbug.t.
khwilliamson pushed a commit to khwilliamson/perl5 that referenced this issue Jul 10, 2023
This fixes issue Perl#15544. If the user has PATH, or similar variables
containing a list of directories separated by e.g. a colon character
these can easily become lines with more than 1000 characters in a perlbug
report, resulting in a test failure in lib/perlbug.t.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants