Skip Menu |
 

This queue is for tickets about the libnet CPAN distribution.

Report information
The Basics
Id: 14875
Status: resolved
Priority: 0/
Queue: libnet

People
Owner: Nobody in particular
Requestors: tmetro [...] cpan.org
Cc: PHILIPP [...] cpan.org
AdminCc:

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



Date: Sun, 02 Oct 2005 02:06:10 -0400
From: Tom Metro <tmetro [...] cpan.org>
To: bug-libnet [...] rt.cpan.org
Subject: Net::Cmd returns obsolete error codes after connection failure
Download (untitled) / with headers
text/plain 6.1k
While using Net::SMTP in a proxy server, I observed that under certain circumstances Net::SMTP was returning the error code from the previous command when the current command failed. A specific scenario in which this might occur is when the server Net::SMTP was talking to drops the connection during the DATA command state, resulting in the dataend() method failing. At this point if you call code() or message() methods, they will return the values set by the data() method, which is normally 354. In a proxy server context, when the client machine see 354 in response to "." (dataend) (instead of the expected 250), it considers it a protocol error, and in the case of sendmail, also considers it a permanent failure and bounces the message. I tracked the problem down to the internals of Net::Cmd and confirmed the problem still exists in the current version (2.26). I've created a test server and client (attached) to demonstrate this problem. First I start the server: % perl crash_server.pl 1 Passing anything on the command line activates the "crash mode," which causes the server to intentionally crash (exit) during the dataend state. Then the client: % perl -MCarp=verbose client.pl CMD: banner Net::SMTP>>> Net::SMTP(2.26) Net::SMTP>>> Net::Cmd(2.26) Net::SMTP>>> Exporter(5.58) Net::SMTP>>> IO::Socket::INET(1.27) Net::SMTP>>> IO::Socket(1.28) Net::SMTP>>> IO::Handle(1.24) Net::SMTP=GLOB(0x1b6dda4)<<< 220 [A] crash-server CMD: helo example.com Net::SMTP=GLOB(0x1b6dda4)>>> EHLO example.com Net::SMTP=GLOB(0x1b6dda4)<<< 500 Syntax error: unrecognized command Net::SMTP=GLOB(0x1b6dda4)>>> HELO example.com Net::SMTP=GLOB(0x1b6dda4)<<< 250 [B] helo example.com CMD: mail from <user@example.com> Net::SMTP=GLOB(0x1b6dda4)>>> MAIL FROM:<user@example.com> Net::SMTP=GLOB(0x1b6dda4)<<< 250 [C] mail from <user@example.com> CMD: rcpt to <user@example.com> Net::SMTP=GLOB(0x1b6dda4)>>> RCPT TO:<user@example.com> Net::SMTP=GLOB(0x1b6dda4)<<< 250 [D] rcpt to <user@example.com> CMD: data_init Net::SMTP=GLOB(0x1b6dda4)>>> DATA Net::SMTP=GLOB(0x1b6dda4)<<< 354 [E] data_init CMD: data_part Net::SMTP=GLOB(0x1b6dda4)>>> data data data data data data data data data data CMD: data_end Net::SMTP=GLOB(0x1b6dda4)>>> . Net::SMTP: Unexpected EOF on command channel at C:/bin/dev/Perl/5.8.4/lib/Net/Cmd.pm line 273 Net::Cmd::getline('Net::SMTP=GLOB(0x1b6dda4)') called at C:/bin/dev/Perl/5.8.4/lib/Net/Cmd.pm line 332 Net::Cmd::response('Net::SMTP=GLOB(0x1b6dda4)') called at C:/bin/dev/Perl/5.8.4/lib/Net/Cmd.pm line 530 Net::Cmd::dataend('Net::SMTP=GLOB(0x1b6dda4)') called at client.pl line 42 Client returned: 354 [E] data_init What's important to note is that you see: Net::SMTP=GLOB(0x1b6dda4)<<< 354 [E] data_init in response to the DATA command, and then the same values returned from code() and message() after the dataend() method fails: Client returned: 354 [E] data_init Reviewing Net::Cmd, it appears to have some problems with how the object properties accessed by code() and message() are initialized. Currently they're initialized near the end of command(): sub command { [...] ${*$cmd}{'net_cmd_resp'} = []; # the response ${*$cmd}{'net_cmd_code'} = "000"; # Made this one up :-) } $cmd; } which has two problems: 1. things can happen to cause command() to exit before it gets to that point, and 2. this seems like it should really be in response(), which can be called independent of command(). Looking at response() we see: sub response { my $cmd = shift; my($code,$more) = (undef) x 2; ${*$cmd}{'net_cmd_resp'} ||= []; [...] which suggests that response() can be called iteratively to build up a multi-line response, yet it has internal logic that contradicts that. The internal logic says that it won't return until all of the lines in a multi-line response are received. My best guess is that this construct was unintentionally copied from getline(), which has a similar conditional initialization of its buffer, but there it makes sense. response() also makes no attempt to initialize ${*$cmd}{'net_cmd_code'}, yet has failure modes in which it will return prior to setting it. More generally, internal error handling is not done consistently. At the top of command() we see: unless (defined fileno($cmd)) { $cmd->set_status("599", "Connection closed"); return $cmd; } which seems like a good approach, though this is the only place in the code where a pseudo (internally generated) result code is used. Many other places in which the code tests for a closed file handle or IO errors it simply returns without communicating that the failure was internal, and leaves the code and message properties in an undetermined state. The same situation is true for trapped timeouts, as mentioned in bug #9394. (On a side note, I'd argue that "599" - a permanent failure - is not a good choice for this purpose. Especially when used as part of a proxy server such that the code gets passed on to a client. I propose 421, which signifies a temporary connection failure.) It may be argued (though not documented as such) that the return values from code() or message() methods should not be used when a method fails, but that logic doesn't hold, because the cause of a method returning false more typically is due to the remote server returning an error code. So the normal reaction to a failed method (in calling code) is going to be examining the result code. With some clean up in the initialization code, it could be made so that Net::Cmd consistently returns "000" for internal errors, which would have the advantage of distinguishing itself from remote server errors (given that "000" is out of range for protocol errors), but that then precludes being able to pass on the code in a proxy scenario, and it requires that the calling code introduce a special case to handle it. It seems the best solution is to embrace the idea of pseudo result codes, and update the code to consistently set the code and message properties when internal IO errors are trapped. I've created a patch to do just that, and I'll post it as a follow up to this message after it has undergone some testing. -Tom
Download client.pl
text/x-perl 983b
use strict; use Net::SMTP; warn "CMD: banner\n"; my $smtpc = Net::SMTP->new('localhost', Port => 9025, Hello => undef, Debug => 99 ) or do { die q/connection failed/; }; my $hostname = 'example.com'; warn "CMD: helo $hostname\n"; $smtpc->hello($hostname) or do { client_error($smtpc); }; my $address = "<user\@$hostname>"; warn "CMD: mail from $address\n"; $smtpc->mail($address) or do { client_error($smtpc); }; warn "CMD: rcpt to $address\n"; $smtpc->recipient($address) or do { client_error($smtpc); }; warn "CMD: data_init\n"; $smtpc->data() or do { client_error($smtpc); }; my $chunk = 'data ' x 10 . "\n"; warn "CMD: data_part\n"; $smtpc->datasend([$chunk]) or do { client_error($smtpc); }; warn "CMD: data_end\n"; $smtpc->dataend() or do { client_error($smtpc); }; warn "CMD: quit\n"; $smtpc->quit() or do { client_error($smtpc); }; sub client_error { my $client = shift; die "Client returned: ",$client->code()," ",$client->message(),"\n"; }
Download crash_server.pl
text/x-perl 1.8k
use strict; use Net::Server::Mail::SMTP; use IO::Socket::INET; # pass 1 (or anything true) on the command line to invoke # "crash mode" my $crash = shift; my $server = new IO::Socket::INET Listen => 1, LocalPort => 9025; warn "Listening on port 9025\n"; warn "Crash mode enabled\n" if $crash; my $conn; while ($conn = $server->accept) { warn "Connected to: ",$conn->peerhost(),"\n"; my $smtpd = new Net::Server::Mail::SMTP socket => $conn; $smtpd->set_callback('banner' => \&banner); $smtpd->set_callback('HELO' => \&helo); $smtpd->set_callback('MAIL' => \&mail); $smtpd->set_callback('RCPT' => \&rcpt); $smtpd->set_callback('DATA-INIT' => \&data_init); $smtpd->set_callback('DATA-PART' => \&data_part); $smtpd->set_callback('DATA' => \&data_end); $smtpd->set_callback('QUIT' => \&quit); warn "Entering SMTP processing loop...\n"; $smtpd->process; # cleanup $smtpd = undef; $conn = undef; } exit(0); ## subs sub banner { my $session = shift; warn "CMD: banner\n"; return(0, 220, '[A] crash-server'); } sub helo { my $session = shift; my $hostname = shift; warn "CMD: helo $hostname\n"; return (1,250, "[B] helo $hostname"); } sub mail { my $session = shift; my $address = shift; warn "CMD: mail from $address\n"; return (1, 250, "[C] mail from $address"); } sub rcpt { my $session = shift; my $address = shift; warn "CMD: rcpt to $address\n"; return (1, 250, "[D] rcpt to $address"); } sub data_init { my $session = shift; warn "CMD: data_init\n"; return (1, 354, '[E] data_init'); } sub data_part { my $session = shift; my $chunk = shift; # ref to buffer warn "CMD: data_part\n"; if ($crash) { warn "** crashing **\n"; exit(0); } return (1); } sub data_end { my $session = shift; warn "CMD: data_end\n"; return (1, 250, '[F] data_end'); } sub quit { my $session = shift; warn "CMD: quit\n"; return (1, 221, '[G] quit'); }
Download (untitled) / with headers
text/plain 206b
Hey Tom, I was working through old tickets and came across this one. You ended with saying you would post a followup with a patch. I am guessing it has been so long you don't have that anymore. Graham.
Subject: Re: [rt.cpan.org #14875] Net::Cmd returns obsolete error codes after connection failure
Date: Sat, 09 Feb 2008 22:22:26 -0500
To: bug-libnet [...] rt.cpan.org
From: Tom Metro <tmetro [...] cpan.org>
Download (untitled) / with headers
text/plain 1.3k
Graham_Barr via RT wrote: Show quoted text
> You ended with saying you would post a followup with a patch. I am > guessing it has been so long you don't have that anymore.
I think the reason I never posted a path is that the changes I made never fully resolved the problem. I might have opened a separate ticket for this, but I seem to recall tracking the issue down to the use of higher layer I/O calls instead of sysread/syswrite, and yet an attempt at fixing that introduced other problems. The code is still in active use as part of a custom anti-spam proxy, but generally the problem crops up infrequently enough that I haven't been adequately motivated to dig into the problem. I've also been planning to migrate the code to another platform or replace it entirely "any day now," either of which would probably eliminate the Let me take a look at the version of Net::Cmd I'm using...looks like I'm currently using an unmodified 2.26. I do have a patched 2.24 kicking around. I don't recall why I didn't migrate the changes forward. I've attached the diff for it, though I no longer recall the specifics of the changes, though most of it seems to correspond with stuff covered in this ticket. The good news is that it looks like I wrote a test program to demonstrate the bug, so it shouldn't be too tough to see if the problem is resolved in 2.26 and fix it again, if it isn't. -Tom
Download cmd.pm-2.24.patch
text/x-diff 9.8k
--- cmd.pm.orig Tue Jun 01 10:49:34 2004 +++ cmd.pm.mod Sun Oct 02 01:31:28 2005 @@ -1,4 +1,4 @@ -# Net::Cmd.pm $Id: //depot/libnet/Net/Cmd.pm#33 $ +# Net::Cmd.pm $Id: cmd.pm,v 1.2 2005-09-30 03:31:09-04 administrator Exp administrator $ # # Copyright (c) 1995-1997 Graham Barr <gbarr@pobox.com>. All rights reserved. # This program is free software; you can redistribute it and/or @@ -32,6 +32,8 @@ sub CMD_ERROR { 5 } sub CMD_PENDING { 0 } +sub DEF_REPLY_CODE { 421 } + my %debug = (); my $tr = $^O eq 'os390' ? Convert::EBCDIC->new() : undef; @@ -156,7 +158,7 @@ my $cmd = shift; - ${*$cmd}{'net_cmd_code'} = "000" + ${*$cmd}{'net_cmd_code'} = $cmd->DEF_REPLY_CODE unless exists ${*$cmd}{'net_cmd_code'}; ${*$cmd}{'net_cmd_code'}; @@ -186,16 +188,40 @@ 1; } -sub command +sub _set_status_timeout { my $cmd = shift; + my $pkg = ref($cmd) || $cmd; - unless (defined fileno($cmd)) - { - $cmd->set_status("599", "Connection closed"); - return $cmd; - } + $cmd->set_status($cmd->DEF_REPLY_CODE, "[$pkg] Timeout"); + carp(ref($cmd) . ": " . (caller(1))[3] . "(): timeout") if $cmd->debug; +} + +sub _set_status_closed +{ + my $cmd = shift; + my $pkg = ref($cmd) || $cmd; + $cmd->set_status($cmd->DEF_REPLY_CODE, "[$pkg] Connection closed"); + carp(ref($cmd) . ": " . (caller(1))[3] + . "(): unexpected EOF on command channel: $!") if $cmd->debug; +} + +sub _is_closed +{ + my $cmd = shift; + if (!defined fileno($cmd)) { + $cmd->_set_status_closed; + return 1; + } + return 0; +} + +sub command +{ + my $cmd = shift; + + $cmd->_is_closed && return $cmd; $cmd->dataend() if(exists ${*$cmd}{'net_cmd_need_crlf'}); @@ -211,14 +237,14 @@ my $len = length $str; my $swlen; - $cmd->close - unless (defined($swlen = syswrite($cmd,$str,$len)) && $swlen == $len); - $cmd->debug_print(1,$str) if($cmd->debug); - ${*$cmd}{'net_cmd_resp'} = []; # the response - ${*$cmd}{'net_cmd_code'} = "000"; # Made this one up :-) + unless (defined($swlen = syswrite($cmd,$str,$len)) && $swlen == $len) { + $cmd->close; + $cmd->_set_status_closed; + return $cmd; + } } $cmd; @@ -236,8 +262,8 @@ { my $cmd = shift; - ${*$cmd}{'net_cmd_resp'} = [ 'Unsupported command' ]; - ${*$cmd}{'net_cmd_code'} = 580; + $cmd->set_status(580, 'Unsupported command'); + 0; } @@ -252,11 +278,10 @@ my $partial = defined(${*$cmd}{'net_cmd_partial'}) ? ${*$cmd}{'net_cmd_partial'} : ""; - my $fd = fileno($cmd); - return undef - unless defined $fd; + $cmd->_is_closed && return undef; + my $fd = fileno($cmd); my $rin = ""; vec($rin,$fd,1) = 1; @@ -270,11 +295,10 @@ { unless (sysread($cmd, $buf="", 1024)) { - carp(ref($cmd) . ": Unexpected EOF on command channel") - if $cmd->debug; $cmd->close; + $cmd->_set_status_closed; return undef; - } + } substr($buf,0,0) = $partial; ## prepend from last sysread @@ -287,7 +311,7 @@ } else { - carp("$cmd: Timeout") if($cmd->debug); + $cmd->_set_status_timeout; return undef; } } @@ -325,7 +349,7 @@ my $cmd = shift; my($code,$more) = (undef) x 2; - ${*$cmd}{'net_cmd_resp'} ||= []; + $cmd->set_status($cmd->DEF_REPLY_CODE, undef); # initialize the response while(1) { @@ -340,8 +364,9 @@ ($code,$more) = $cmd->parse_response($str); unless(defined $code) { + carp("$cmd: response(): parse error in '$str'") if ($cmd->debug); $cmd->ungetline($str); - last; + return CMD_ERROR; } ${*$cmd}{'net_cmd_code'} = $code; @@ -390,12 +415,12 @@ my $arr = @_ == 1 && ref($_[0]) ? $_[0] : \@_; my $line = join("" ,@$arr); - return 0 unless defined(fileno($cmd)); + $cmd->_is_closed && return 0; unless (length $line) { # Even though we are not sending anything, the fact we were # called means that dataend needs to be called before the next - # command, which happens of net_cmd_need_crlf exists + # command, which happens if net_cmd_need_crlf exists ${*$cmd}{'net_cmd_need_crlf'} ||= 0; return 1; } @@ -428,9 +453,10 @@ if (select(undef,$wout=$win, undef, $timeout) > 0) { my $w = syswrite($cmd, $line, $len, $offset); - unless (defined($w)) + unless (defined($w) && $w == $len) { - carp("$cmd: $!") if $cmd->debug; + $cmd->close; + $cmd->_set_status_closed; return undef; } $len -= $w; @@ -438,7 +464,7 @@ } else { - carp("$cmd: Timeout") if($cmd->debug); + $cmd->_set_status_timeout; return undef; } } @@ -452,7 +478,7 @@ my $arr = @_ == 1 && ref($_[0]) ? $_[0] : \@_; my $line = join("" ,@$arr); - return 0 unless defined(fileno($cmd)); + $cmd->_is_closed && return 0; return 1 unless length($line); @@ -476,9 +502,10 @@ if (select(undef,$wout=$win, undef, $timeout) > 0) { my $w = syswrite($cmd, $line, $len, $offset); - unless (defined($w)) + unless (defined($w) && $w == $len) { - carp("$cmd: $!") if $cmd->debug; + $cmd->close; + $cmd->_set_status_closed; return undef; } $len -= $w; @@ -486,7 +513,7 @@ } else { - carp("$cmd: Timeout") if($cmd->debug); + $cmd->_set_status_timeout; return undef; } } @@ -498,19 +525,28 @@ { my $cmd = shift; - return 0 unless defined(fileno($cmd)); + $cmd->_is_closed && return 0; return 1 unless(exists ${*$cmd}{'net_cmd_need_crlf'}); local $SIG{PIPE} = 'IGNORE' unless $^O eq 'MacOS'; - syswrite($cmd,"\015\012",2) - if ${*$cmd}{'net_cmd_need_crlf'}; $cmd->debug_print(1, ".\n") if($cmd->debug); - syswrite($cmd,".\015\012",3); + my $terminator = ${*$cmd}{'net_cmd_need_crlf'} ? + "\015\012" : ''; + + $terminator .= ".\015\012"; + my $len = length($terminator); + my $w = syswrite($cmd,$terminator,$len); + unless (defined($w) && $w == $len) + { + $cmd->close; + $cmd->_set_status_closed; + return 0; + } delete ${*$cmd}{'net_cmd_need_crlf'}; @@ -626,12 +662,13 @@ =item message () -Returns the text message returned from the last command +Returns the text message returned from the last command. +(See L<PSEUDO RESPONSES> below.) =item code () Returns the 3-digit code from the last command. If a command is pending -then the value 0 is returned +then the value 0 is returned. (See L<PSEUDO RESPONSES> below.) =item ok () @@ -672,21 +709,21 @@ data being sent to the server. Calls C<debug_text> before printing to STDERR. -=item debug_text ( TEXT ) +=item debug_text ( undef, TEXT ) This method is called to print debugging information. TEXT is -the text being sent. The method should return the text to be printed +the text being sent. The method should return the text to be printed. This is primarily meant for the use of modules such as FTP where passwords are sent, but we do not want to display them in the debugging information. =item command ( CMD [, ARGS, ... ]) -Send a command to the command server. All arguments a first joined with +Send a command to the command server. All arguments are first joined with a space character and CRLF is appended, this string is then sent to the command server. -Returns undef upon failure +Returns undef upon failure. =item unsupported () @@ -696,14 +733,14 @@ =item response () Obtain a response from the server. Upon success the most significant digit -of the status code is returned. Upon failure, timeout etc., I<undef> is +of the status code is returned. Upon failure, timeout etc., I<CMD_ERROR> is returned. =item parse_response ( TEXT ) This method is called by C<response> as a method with one argument. It should return an array of 2 values, the 3-digit status code and a flag which is true -when this is part of a multi-line response and this line is not the list. +when this is part of a multi-line response and this line is not the last. =item getline () @@ -741,6 +778,44 @@ =back +=head1 PSEUDO RESPONSES + +Normally the values returned by C<message()> and C<code()> are +obtained from the remote server, but in a few circumstances, as +detailed below, C<Net::Cmd> will return values that it sets. You +can alter this behavior by overriding DEF_REPLY_CODE() to specify +a different default reply code, or overriding one of the specific +error handling methods below. + +=over 4 + +=item Initial value + +Before any command has executed or if an unexpected error occurs +C<code()> will return "421" (temporary connection failure) and +C<message()> will return undef. + +=item Connection closed + +If the underlying C<IO::Handle> is closed, or if there are +any read or write failures, the file handle will be forced closed, +and C<code()> will return "421" (temporary connection failure) +and C<message()> will return "[$pkg] Connection closed" +(where $pkg is the name of the class that subclassed C<Net::Cmd>). +The _set_status_closed() method can be overridden to set a different +message (by calling set_status()) or otherwise trap this error. + +=item Timeout + +If if there is a read or write timeout C<code()> will return "421" +(temporary connection failure) and C<message()> will return +"[$pkg] Timeout" (where $pkg is the name of the class +that subclassed C<Net::Cmd>). The _set_status_timeout() method +can be overridden to set a different message (by calling set_status()) +or otherwise trap this error. + +=back + =head1 EXPORTS C<Net::Cmd> exports six subroutines, five of these, C<CMD_INFO>, C<CMD_OK>, @@ -759,6 +834,6 @@ =for html <hr> -I<$Id: //depot/libnet/Net/Cmd.pm#33 $> +I<$Id: cmd.pm,v 1.2 2005-09-30 03:31:09-04 administrator Exp administrator $> =cut
Subject: Re: [rt.cpan.org #14875] Net::Cmd returns obsolete error codes after connection failure
Date: Sun, 10 Feb 2008 07:50:33 -0600
To: bug-libnet [...] rt.cpan.org
From: Graham Barr <gbarr [...] pobox.com>
Download (untitled) / with headers
text/plain 559b
On Feb 9, 2008, at 9:22 PM, Tom Metro via RT wrote: Show quoted text
> Graham_Barr via RT wrote:
>> You ended with saying you would post a followup with a patch. I am >> guessing it has been so long you don't have that anymore.
>
Show quoted text
> The good news is that it looks like I wrote a test program to > demonstrate the bug, so it shouldn't be too tough to see if the > problem > is resolved in 2.26 and fix it again, if it isn't.
Please try from my repository, although I suspect this has not fixed your problem. http://svn.goingon.net/repos/libnet/trunk/Net/Cmd.pm Graham.
Download (untitled) / with headers
text/plain 414b
We seem to be hitting the same problem. Very occasional failures of $smtp->dataend() with a code() of 354 that seems to be the obsolete code from the earlier data() command. In our case Email::Sender::Transport::SMTP fails with "error at after DATA". Is there any news on this Tom? (Or Steve or Graham (hi!)) Tim. p.s. The https://rt.cpan.org/Ticket/Display.html?id=47557 case might also overlap with this one.
Download (untitled) / with headers
text/plain 1.3k
On Fri Aug 29 08:17:59 2014, TIMB wrote: Show quoted text
> We seem to be hitting the same problem. > > Very occasional failures of $smtp->dataend() with a code() of 354 that > seems to be the obsolete code from the earlier data() command. In our > case Email::Sender::Transport::SMTP fails with "error at after DATA". > > Is there any news on this Tom? (Or Steve or Graham (hi!)) > > Tim. > > p.s. The https://rt.cpan.org/Ticket/Display.html?id=47557 case might > also overlap with this one.
I've had a read through this ticket and it certainly contains a lot of useful comments about how Net::Cmd could be improved. I'm inclined to apply the patch, but I would love to be able to reproduce the failure first and see the fix in action. However, when I run the crash_server.pl and client.pl programs I find that the crash_server.pl crashes a little too enthusiastically: On my 5.18.0 system (on Windows) it actually causes perl.exe to crash when the client.pl connects and starts doing stuff!!! Tim, are you able to apply the patch to your system (or preferably to the current development version in my Git repo) and see that it cures your problem? If not then I might just go ahead and apply it anyway since it all looks like good improvements to me, but I would welcome any feedback first. (And yes, 47557 looks like the same problem so I will ask if the reporter of that ticket can test this patch too.)
Download (untitled) / with headers
text/plain 628b
On Fri Oct 03 03:59:18 2014, SHAY wrote: Show quoted text
> (And yes, 47557 looks like the same problem so I will ask if the > reporter of that ticket can test this patch too.)
Successful use of the patch on several versions of perl over a couple of years have been reported on #47557, so I've applied the patch in commit: https://github.com/steve-m-hay/perl-libnet/commit/6b249e0122f764e186b14f7a7663a0ffd3055121 with a minor fix-up in commit: https://github.com/steve-m-hay/perl-libnet/commit/b3e899dfaad77c48bf0f83c66a9a8a3c2233beb2 Many thanks for the detailed report and the patch. This will be in libnet-1.28, to be released shortly.
Download (untitled) / with headers
text/plain 623b
This is bad patch. It introduces a fundamental error in the treatment of reads and writes. Consider this example: my $w = syswrite($cmd, $line, $len, $offset); - unless (defined($w)) + unless (defined($w) && $w == $len) syswrite is allowed to return less than the requested amount of data (which is why it's in a loop and len/offset are adjusted by $w later). This patch makes a short write into an error condition and does so pretty much everywhere that syswrite is used. Separately, I note that even the original code doesn't check $! for EINTR if $w is not defined. It should so check and continue.
Download (untitled) / with headers
text/plain 672b
On Fri Dec 25 16:54:14 2015, DAGOLDEN wrote: Show quoted text
> This is bad patch. It introduces a fundamental error in the treatment > of reads and writes. > syswrite is allowed to return less than the requested amount of data...
That seems like a valid point, and a behavior of syswrite I was well aware of at the time of the original patch, so I don't understand why the code wouldn't have anticipated getting back underfull buffers from syswrite, unless there is some explanation I'm not immediately gleaning from a quick read of the original patch. (A decade later, lots of context has been lost to time.) Your proposed pull request on github looks like a suitable refinement.
Download (untitled) / with headers
text/plain 109b
PR#24 now merged, so marking this ticket as resolved once more. The fix will be in 3.08, to be released soon.


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.