Skip Menu |
 

This queue is for tickets about the libnet CPAN distribution.

Report information
The Basics
Id: 88491
Status: open
Priority: 0/
Queue: libnet

People
Owner: Nobody in particular
Requestors: alexander_bluhm [...] genua.de
Cc: jkeenan [...] cpan.org
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

Attachments
0001-Correct-Net-Cmd-datasend-to-conform-to-RFC-2821-Sect.patch
0002-Correct-Net-Cmd-datasend-dataend-to-conform-to-RFC-5.patch



CC: jkeenan [...] cpan.org
Subject: [RT #38387] Net::Cmd does not handle CR and LF correctly
Download (untitled) / with headers
text/plain 6.1k
The bug report below was originally submitted to the Perl 5 bug tracker in January 2006. It never received a response. libnet is included in the Perl 5 core distribution, but until Aug 12 2013 the place to which bugs were to be reported was undefined. Since Perl 5's Porting/Maintainers.pl now lists "UPSTREAM => 'cpan'", I am transferring this bug report to this queue on rt.cpan.org. --jkeenan This is a bug report for perl from alexander_bluhm@genua.de, generated with the help of perlbug 1.35 running under perl v5.8.6. ----------------------------------------------------------------- [Please enter your report here] Hi, The escaping of CR and LF in Net::Cmd->dataend is somewhat broken. A sequence "a\r" is escaped as "a\015\015\012.\015\012". There is even a test for this behavior. But it does not conform to RFC 2821 Section 2.3.7: In addition, the appearance of "bare" "CR" or "LF" characters in text (i.e., either without the other) has a long history of causing problems in mail implementations and applications that use the mail system as a tool. SMTP client implementations MUST NOT transmit these characters except when they are intended as line terminators and then MUST, as indicated above, transmit them only as a <CRLF> sequence. So sending the sequence "\015\015" is illegal because the first "\015" is not followed by a "\012". I have made a patch for that bug. I have also added a lot of other test cases to the t/datasend.t test. I wrote these tests, after we had encountered real world SMTP problems with different versions of Net::Cmd. I am using Net::Cmd Version 2.26 from the libnet-1.19 package in the Perl 5.8.6 core. Alexander --- ./Net/Cmd.pm.orig Wed Jun 30 15:56:11 2004 +++ ./Net/Cmd.pm Tue Jan 31 21:52:14 2006 @@ -512,6 +512,9 @@ if (!defined $ch) { return 1; } + elsif ($ch eq "\015") { + $tosend = "\012"; + } elsif ($ch ne "\012") { $tosend = "\015\012"; } --- ./t/datasend.t.orig Wed Jun 30 15:56:10 2004 +++ ./t/datasend.t Tue Jan 31 22:56:21 2006 @@ -30,7 +33,7 @@ (my $libnet_t = __FILE__) =~ s/datasend.t/libnet_t.pl/; require $libnet_t or die; -print "1..51\n"; +print "1..212\n"; sub check { my $expect = pop; @@ -46,8 +49,6 @@ ); } -my $cmd; - check( # nothing @@ -63,7 +64,7 @@ check( "a\r", - "a\015\015\012.\015\012", + "a\015\012.\015\012", ); check( @@ -144,3 +145,67 @@ "a\015\012..\015\012.\015\012", ); +# Send data with \r\n on block break. +my @rn = ( + [ "single line" ], + [ "two\nlines" ], + [ "endnl\n" ], + [ "endcr\r" ], + [ "blockendnl", "\n" ], + [ "\n", "blockstartnl" ], + [ "\nbothnl\n" ], + [ "\n", "blockbothnl", "\n" ], + [ "separate", "lines" ], + [ "separate\n", "newlines\n" ], + [ "only\rcr" ], + [ "end\rcr\r" ], + [ "after", "\nbreak", "\r\ncr" ], + [ "before\n", "break\r\n", "cr" ], + [ "both\n", "\nbreak\r", "\ncr" ], + [ "break\r", "\ncr\nnewline" ], + [ "a\r\n\r\n", "b\r\n\r", "\nc\r\n", "\r\nd\r", "\n\r\ne", "\r\n\r\nf" ], + [ "\r\r\nx\r", "\r\ny", "\r\r\nz\r\r" ], + [ "foo\r", "\nbar\n", "Hello\nworld\r" ], + [ "newlines\n\n" ], + [ "tripple\r", "\n\n\n" ], +); + +# Send data with 0. on block break. +my @zd = ( + [ "foo\n.\nbar" ], + [ "foo", "\n.\nbar" ], + [ "foo\n", ".\nbar" ], + [ "foo\n.", "\nbar" ], + [ "foo\n.\n", "bar" ], + + [ "foo\n." ], + [ "foo", "\n." ], + [ "foo\n", "." ], + + [ ".\nbar" ], + [ ".", "\nbar" ], + [ ".\n", "bar" ], + + [ "foo0.\nbar" ], + [ "foo", "0.\nbar" ], + [ "foo0", ".\nbar" ], + [ "foo0.", "\nbar" ], + [ "foo0.\n", "bar" ], + + [ "foo0\n." ], + [ "foo", "0\n." ], + [ "foo0", "\n." ], + [ "foo0\n", "." ], +); + +foreach (@rn, @zd) { + my @data = @$_; + my $expect = join("", @data); + $expect .= "\n" if $expect !~ /\n$/; + $expect =~ s/\r?\n/\r\n/gs; + $expect =~ s/(?:^|(?<=\r\n))\.\r\n/..\r\n/gs; + $expect .= ".\r\n"; + $expect =~ tr/\r/\015/; + $expect =~ tr/\n/\012/; + check(@data, $expect); +} [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=library severity=medium --- Site configuration information for perl v5.8.6: Configured by root at Thu Jan 1 0:00:00 UTC 1970. Summary of my perl5 (revision 5 version 8 subversion 6) configuration: Platform: osname=openbsd, osvers=3.9, archname=i386-openbsd uname='openbsd' config_args='-dsE -Dopenbsd_distribution=defined' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -fno-delete-null-pointer-checks -pipe -I/usr/local/include', optimize='-O2', cppflags='-fno-strict-aliasing -fno-delete-null-pointer-checks -pipe -I/usr/local/include' ccversion='', gccversion='3.3.5 (propolice)', gccosandvers='openbsd3.9' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags ='-Wl,-E ' libpth=/usr/lib libs=-lm -lutil -lc perllibs=-lm -lutil -lc libc=/usr/lib/libc.so.39.0, so=so, useshrplib=true, libperl=libperl.so.10.0 gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-R/usr/libdata/perl5/i386-openbsd/5.8.6/CORE' cccdlflags='-DPIC -fPIC ', lddlflags='-shared -fPIC ' Locally applied patches: SUIDPERLIO1 - fix PERLIO_DEBUG buffer overflow (CAN-2005-0156) SPRINTF0 - fixes for sprintf formatting issues - CVE-2005-3962 --- @INC for perl v5.8.6: /usr/libdata/perl5/i386-openbsd/5.8.6 /usr/local/libdata/perl5/i386-openbsd/5.8.6 /usr/libdata/perl5 /usr/local/libdata/perl5 /usr/local/libdata/perl5/site_perl/i386-openbsd /usr/libdata/perl5/site_perl/i386-openbsd /usr/local/libdata/perl5/site_perl /usr/libdata/perl5/site_perl /usr/local/lib/perl5/site_perl . --- Environment for perl v5.8.6: HOME=/home/bluhm LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/bin:/bin:/usr/sbin:/sbin:/usr/X11R6/bin:/usr/local/bin:/home/bluhm/bin PERL_BADLANG (unset) SHELL=/bin/ksh
RT-Send-CC: alexander_bluhm [...] genua.de
Download (untitled) / with headers
text/plain 2.9k
Show quoted text
> The escaping of CR and LF in Net::Cmd->dataend is somewhat broken. > A sequence "a\r" is escaped as "a\015\015\012.\015\012". > There is even a test for this behavior. But it does not conform > to RFC 2821 Section 2.3.7: > > In addition, the appearance of "bare" "CR" or "LF" characters in text > (i.e., either without the other) has a long history of causing > problems in mail implementations and applications that use the mail > system as a tool. SMTP client implementations MUST NOT transmit > these characters except when they are intended as line terminators > and then MUST, as indicated above, transmit them only as a <CRLF> > sequence. > > So sending the sequence "\015\015" is illegal because the first > "\015" is not followed by a "\012". > > I have made a patch for that bug. I have also added a lot of other > test cases to the t/datasend.t test. I wrote these tests, after > we had encountered real world SMTP problems with different versions > of Net::Cmd. >
Thanks for the patch. I've attached it here, based on James Keenan's updated patch from rt.perl.org #38387, but now adjusted to apply to the libnet repository instead of core perl. However, I do not think the patch is correct. You note that "a\r" is escaped as "a\015\015\012.\015\012", which does not conform to RFC 2821 (actually now obsoleted by RFC 5321, but which contains the same text about CR/LF) and your patch fixes that -- namely, the specific case of an "\r" at the end of the data, by adjusting the dataend() method. But this does nothing to address the issue of an \r appearing in the middle of the data being sent. You have even added tests for such cases, e.g. "only\rcr". The tests pass, but only because the expected data that you are testing against has the same bug as the Net::Cmd module still has, i.e. that it also doesn't fix up bare \r characters in the middle of the data. With your patch "only\rcr" is escaped as "only\rcr\r\n.\r\n", which is also transmitting a bare \r, which is disallowed by the very same section of the RFC that you have quoted. So I've also attached an updated/corrected patch, which I believe fixes the case of bare \r characters anywhere else in the data too. I have one other concern about these changes: We have been talking about the RFCs that define the SMTP protocol, but Net::Cmd is a base class used by classes other than Net::SMTP. In particular, Net::NNTP uses it, and the RFC that defines NNTP (RFC 3977) does not, as far as I can see, contain similar notes about the transmission of CR/LF. However, if things are broken for NNTP (as they do indeed seem to be; e.g. see CPAN RT#78922) then they must already be broken, and I do not think these changes will make the situation any worse for NNTP users. That issue will need addressing separately, perhaps on CPAN RT#78922. Alexander, are you able to confirm whether my patch (0002-Correct-Net-Cmd-datasend-dataend-to-conform-to-RFC-5.patch) fixes the problems you were having?
Subject: 0001-Correct-Net-Cmd-datasend-to-conform-to-RFC-2821-Sect.patch
From f366cfecfb431c718c3fc70279950c5e8d383b24 Mon Sep 17 00:00:00 2001 From: Alexander Bluhm <Alexander_Bluhm@genua.de> Date: Sun, 4 Dec 2011 09:19:47 -0500 Subject: [PATCH] Correct Net::Cmd::datasend to conform to RFC 2821 Section 2.3.7. Add tests to datasend.t. Patches submitted by Alexander Bluhm for RT #38387. --- lib/Net/Cmd.pm | 3 +++ t/datasend.t | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/lib/Net/Cmd.pm b/lib/Net/Cmd.pm index 2934e02..80946c2 100644 --- a/lib/Net/Cmd.pm +++ b/lib/Net/Cmd.pm @@ -558,6 +558,9 @@ sub dataend { if (!defined $ch) { return 1; } + elsif ($ch eq "\015") { + $tosend = "\012"; + } elsif ($ch ne "\012") { $tosend = "\015\012"; } diff --git a/t/datasend.t b/t/datasend.t index 3a97c4b..290462c 100644 --- a/t/datasend.t +++ b/t/datasend.t @@ -44,7 +44,7 @@ BEGIN { (my $libnet_t = __FILE__) =~ s/datasend.t/libnet_t.pl/; require $libnet_t or die; -print "1..51\n"; +print "1..212\n"; sub check { my $expect = pop; @@ -60,8 +60,6 @@ sub check { ); } -my $cmd; - check( # nothing @@ -77,7 +75,7 @@ check( check( "a\r", - "a\015\015\012.\015\012", + "a\015\012.\015\012", ); check( @@ -158,3 +156,68 @@ check( "a\015\012..\015\012.\015\012", ); +# Send data with \r\n on block break. +my @rn = ( + [ "single line" ], + [ "two\nlines" ], + [ "endnl\n" ], + [ "endcr\r" ], + [ "blockendnl", "\n" ], + [ "\n", "blockstartnl" ], + [ "\nbothnl\n" ], + [ "\n", "blockbothnl", "\n" ], + [ "separate", "lines" ], + [ "separate\n", "newlines\n" ], + [ "only\rcr" ], + [ "end\rcr\r" ], + [ "after", "\nbreak", "\r\ncr" ], + [ "before\n", "break\r\n", "cr" ], + [ "both\n", "\nbreak\r", "\ncr" ], + [ "break\r", "\ncr\nnewline" ], + [ "a\r\n\r\n", "b\r\n\r", "\nc\r\n", "\r\nd\r", "\n\r\ne", "\r\n\r\nf" ], + [ "\r\r\nx\r", "\r\ny", "\r\r\nz\r\r" ], + [ "foo\r", "\nbar\n", "Hello\nworld\r" ], + [ "newlines\n\n" ], + [ "tripple\r", "\n\n\n" ], +); + +# Send data with 0. on block break. +my @zd = ( + [ "foo\n.\nbar" ], + [ "foo", "\n.\nbar" ], + [ "foo\n", ".\nbar" ], + [ "foo\n.", "\nbar" ], + [ "foo\n.\n", "bar" ], + + [ "foo\n." ], + [ "foo", "\n." ], + [ "foo\n", "." ], + + [ ".\nbar" ], + [ ".", "\nbar" ], + [ ".\n", "bar" ], + + [ "foo0.\nbar" ], + [ "foo", "0.\nbar" ], + [ "foo0", ".\nbar" ], + [ "foo0.", "\nbar" ], + [ "foo0.\n", "bar" ], + + [ "foo0\n." ], + [ "foo", "0\n." ], + [ "foo0", "\n." ], + [ "foo0\n", "." ], +); + +foreach (@rn, @zd) { + my @data = @$_; + my $expect = join("", @data); + $expect .= "\n" if $expect !~ /\n$/; + $expect =~ s/\r?\n/\r\n/gs; + $expect =~ s/(?:^|(?<=\r\n))\.\r\n/..\r\n/gs; + $expect .= ".\r\n"; + $expect =~ tr/\r/\015/; + $expect =~ tr/\n/\012/; + check(@data, $expect); +} + -- 1.9.5.msysgit.0
Subject: 0002-Correct-Net-Cmd-datasend-dataend-to-conform-to-RFC-5.patch
From dfeaff9d8a2640f69676afb164402fa64da1576d Mon Sep 17 00:00:00 2001 From: Steve Hay <steve.m.hay@googlemail.com> Date: Sun, 8 Feb 2015 13:53:54 +0000 Subject: [PATCH] Correct Net::Cmd::datasend/dataend to conform to RFC 5321 Section 2.3.8. Add tests to datasend.t. Based on a patch submitted by Alexander Bluhm for rt.perl.org #38387. --- lib/Net/Cmd.pm | 18 +++++++++++--- t/datasend.t | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/lib/Net/Cmd.pm b/lib/Net/Cmd.pm index 2934e02..c32a326 100644 --- a/lib/Net/Cmd.pm +++ b/lib/Net/Cmd.pm @@ -453,18 +453,27 @@ sub datasend { my $first_ch = ''; if ($last_ch eq "\015") { - # Remove \012 so it does not get prefixed with another \015 below - # and escape the . if there is one following it because the fixup + # Remove any leading \012 so it does not get prefixed with another \015 + # below, and escape the . if there is one following it because the fixup # below will not find it - $first_ch = "\012" if $line =~ s/^\012(\.?)/$1$1/; + $line =~ s/^\012(\.?)/$1$1/; + + # Always start with \012 if the last data ended with \015. + $first_ch = "\012"; } elsif ($last_ch eq "\012") { # Fixup below will not find the . as the first character of the buffer $first_ch = "." if $line =~ /^\./; } + # Make sure any \012s are preceded by \015s, + # and double-up any .s that follow a \012. $line =~ s/\015?\012(\.?)/\015\012$1$1/sg; + # Make sure any \015s are followed by \012s, + # except at the end of the data (in case the next data begins with \012). + $line =~ s/\015(?=[^\012])/\015\012/sg; + substr($line, 0, 0) = $first_ch; ${*$cmd}{'net_cmd_last_ch'} = substr($line, -1, 1); @@ -558,6 +567,9 @@ sub dataend { if (!defined $ch) { return 1; } + elsif ($ch eq "\015") { + $tosend = "\012"; + } elsif ($ch ne "\012") { $tosend = "\015\012"; } diff --git a/t/datasend.t b/t/datasend.t index 3a97c4b..1c9563c 100644 --- a/t/datasend.t +++ b/t/datasend.t @@ -44,7 +44,7 @@ BEGIN { (my $libnet_t = __FILE__) =~ s/datasend.t/libnet_t.pl/; require $libnet_t or die; -print "1..51\n"; +print "1..212\n"; sub check { my $expect = pop; @@ -60,8 +60,6 @@ sub check { ); } -my $cmd; - check( # nothing @@ -77,25 +75,25 @@ check( check( "a\r", - "a\015\015\012.\015\012", + "a\015\012.\015\012", ); check( "a\rb", - "a\015b\015\012.\015\012", + "a\015\012b\015\012.\015\012", ); check( "a\rb\n", - "a\015b\015\012.\015\012", + "a\015\012b\015\012.\015\012", ); check( "a\rb\n\n", - "a\015b\015\012\015\012.\015\012", + "a\015\012b\015\012\015\012.\015\012", ); check( @@ -158,3 +156,67 @@ check( "a\015\012..\015\012.\015\012", ); +# Send data with \r\n on block break. +my @rn = ( + [ "single line" ], + [ "two\nlines" ], + [ "endnl\n" ], + [ "endcr\r" ], + [ "blockendnl", "\n" ], + [ "\n", "blockstartnl" ], + [ "\nbothnl\n" ], + [ "\n", "blockbothnl", "\n" ], + [ "separate", "lines" ], + [ "separate\n", "newlines\n" ], + [ "only\rcr" ], + [ "end\rcr\r" ], + [ "after", "\nbreak", "\r\ncr" ], + [ "before\n", "break\r\n", "cr" ], + [ "both\n", "\nbreak\r", "\ncr" ], + [ "break\r", "\ncr\nnewline" ], + [ "a\r\n\r\n", "b\r\n\r", "\nc\r\n", "\r\nd\r", "\n\r\ne", "\r\n\r\nf" ], + [ "\r\r\nx\r", "\r\ny", "\r\r\nz\r\r" ], + [ "foo\r", "\nbar\n", "Hello\nworld\r" ], + [ "newlines\n\n" ], + [ "tripple\r", "\n\n\n" ], +); + +# Send data with 0. on block break. +my @zd = ( + [ "foo\n.\nbar" ], + [ "foo", "\n.\nbar" ], + [ "foo\n", ".\nbar" ], + [ "foo\n.", "\nbar" ], + [ "foo\n.\n", "bar" ], + + [ "foo\n." ], + [ "foo", "\n." ], + [ "foo\n", "." ], + + [ ".\nbar" ], + [ ".", "\nbar" ], + [ ".\n", "bar" ], + + [ "foo0.\nbar" ], + [ "foo", "0.\nbar" ], + [ "foo0", ".\nbar" ], + [ "foo0.", "\nbar" ], + [ "foo0.\n", "bar" ], + + [ "foo0\n." ], + [ "foo", "0\n." ], + [ "foo0", "\n." ], + [ "foo0\n", "." ], +); + +foreach (@rn, @zd) { + my @data = @$_; + my $expect = join("", @data); + $expect .= "\n" if $expect !~ /\n$/; + $expect =~ s/\r?\n/\r\n/gs; + $expect =~ s/\r(?!\n)/\r\n/gs; + $expect =~ s/(?:^|(?<=\r\n))\.\r\n/..\r\n/gs; + $expect .= ".\r\n"; + $expect =~ tr/\r\n/\015\012/ unless "\r" eq "\015"; + check(@data, $expect); +} -- 1.9.5.msysgit.0


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.