Skip Menu |

This queue is for tickets about the libnet CPAN distribution.

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

People
Owner: Nobody in particular
Requestors: ianburrell [...] gmail.com
Cc:
AdminCc:

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



Subject: Net::FTP::I::read has different parameters than standard read
I am trying to fix #33231 which involves "Timeout" errors from IO::Uncompress::Gunzip when reading from Net::FTP data connection. The cause seems to be that IO::Uncompress::Base::smartRead does: *$self->{FH}->read($$out, $get_size, $offset); The $offset defaults to 0. The Net::FTP::I::read method has the arguments of $buf, $size, $timeout. The 0 timeout causes the "Timeout" error whenever there is no data available for reading.
From: ianburrell [...] gmail.com
Here is a patch which changes Net::FTP::I and Net::FTP::A to add offset argument to read. It correctly pads the buffer as described in the perlfunc docs. I also added a test for the new behavior. Unfortunately, this does incompatibly change the interface.
Index: t/ftp.t =================================================================== --- t/ftp.t (revision 352) +++ t/ftp.t (working copy) @@ -5,9 +5,9 @@ chdir 't' if -d 't'; @INC = '../lib'; } - if (!eval "require Socket") { - print "1..0 # no Socket\n"; exit 0; - } +# if (!eval "require Socket") { +# print "1..0 # no Socket\n"; exit 0; +# } if (ord('A') == 193 && !eval "require Convert::EBCDIC") { print "1..0 # EBCDIC but no Convert::EBCDIC\n"; exit 0; } @@ -22,7 +22,7 @@ } my $t = 1; -print "1..7\n"; +print "1..8\n"; $ftp = Net::FTP->new($NetConfig{ftp_testhost}) or (print("not ok 1\n"), exit); @@ -54,9 +54,17 @@ $data->close; print "not " unless $text eq $buf; printf "ok %d\n",$t++; + + $buf = ''; + $data = $ftp->retr('libnet.tst'); + $data->read($buf, 10); + $data->read($buf, length($text) - 10, 10); + $data->close; + print "not " unless $text eq $buf; + printf "ok %d\n",$t++; + $ftp->delete('libnet.tst') or print "not "; printf "ok %d\n",$t++; - } else { print "# ",$ftp->message,"\n"; Index: Net/FTP.pm =================================================================== --- Net/FTP.pm (revision 352) +++ Net/FTP.pm (working copy) @@ -1723,9 +1723,9 @@ =over 4 -=item read ( BUFFER, SIZE [, TIMEOUT ] ) +=item read ( BUFFER, SIZE, [, OFFSET] [, TIMEOUT ] ) -Read C<SIZE> bytes of data from the server and place it into C<BUFFER>, also +Read C<SIZE> bytes of data from the server and place it into C<BUFFER> at C<OFFSET>, also performing any <CRLF> translation necessary. C<TIMEOUT> is optional, if not given, the timeout value from the command connection will be used. Index: Net/FTP/I.pm =================================================================== --- Net/FTP/I.pm (revision 352) +++ Net/FTP/I.pm (working copy) @@ -17,8 +17,9 @@ my $data = shift; local *buf = \$_[0]; shift; - my $size = shift || croak 'read($buf,$size,[$timeout])'; - my $timeout = @_ ? shift: $data->timeout; + my $size = shift || croak 'read($buf,$size,[$offset],[$timeout])'; + my $offset = shift || 0; + my $timeout = @_ ? shift : $data->timeout; my $n; @@ -35,7 +36,14 @@ } } - $buf = substr(${*$data}, 0, $size); + if ($offset) { + if (length($buf) < $offset) { + $buf .= ('\0' x ($offset + 1 - length($buf))); + } + substr($buf, $offset) = substr(${*$data}, 0, $size); + } else { + $buf = substr(${*$data}, 0, $size); + } $n = length($buf); Index: Net/FTP/A.pm =================================================================== --- Net/FTP/A.pm (revision 352) +++ Net/FTP/A.pm (working copy) @@ -17,8 +17,9 @@ my $data = shift; local *buf = \$_[0]; shift; - my $size = shift || croak 'read($buf,$size,[$offset])'; - my $timeout = @_ ? shift: $data->timeout; + my $size = shift || croak 'read($buf,$size,[$offset],[$timeout])'; + my $offset = shift || 0; + my $timeout = @_ ? shift : $data->timeout; if (length(${*$data}) < $size && !${*$data}{'net_ftp_eof'}) { my $blksize = ${*$data}{'net_ftp_blksize'}; @@ -62,7 +63,15 @@ } } - $buf = substr(${*$data}, 0, $size); + if ($offset) { + if (length($buf) < $offset) { + $buf .= ('\0' x ($offset + 1 - length($buf))); + } + substr($buf, $offset) = substr(${*$data}, 0, $size); + } else { + $buf = substr(${*$data}, 0, $size); + } + substr(${*$data}, 0, $size) = ''; length $buf;
Subject: Re: [rt.cpan.org #33268] Net::FTP::I::read has different parameters than standard read
Date: Thu, 14 Feb 2008 18:48:57 -0600
To: bug-libnet [...] rt.cpan.org
From: Graham Barr <gbarr [...] pobox.com>
On Feb 14, 2008, at 5:44 PM, Ian Burrell via RT wrote: Show quoted text
> Queue: libnet > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=33268 > > > Here is a patch which changes Net::FTP::I and Net::FTP::A to add > offset > argument to read. It correctly pads the buffer as described in the > perlfunc docs. I also added a test for the new behavior. > Unfortunately, this does incompatibly change the interface.
Which is why I cannot accept it. if you want to use a ::dataconn object as a filehandle then you can read($fh,$buffer,$offset) but the method interface is as it is defined. It is unfortunate that the read/write methods do not match that of the IO::Handle class and was a bad choice. But it has been this way for 10 years and I am not going to make an incompatible change now. Graham.
On Thu Feb 14 19:49:14 2008, gbarr@pobox.com wrote: Show quoted text
> On Feb 14, 2008, at 5:44 PM, Ian Burrell via RT wrote:
> > Queue: libnet > > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=33268 > > > > > Here is a patch which changes Net::FTP::I and Net::FTP::A to add > > offset > > argument to read. It correctly pads the buffer as described in the > > perlfunc docs. I also added a test for the new behavior. > > Unfortunately, this does incompatibly change the interface.
> > Which is why I cannot accept it. if you want to use a ::dataconn > object as a filehandle then you can read($fh,$buffer,$offset) but the > method interface is as it is defined. > > It is unfortunate that the read/write methods do not match that of > the IO::Handle class and was a bad choice. But it has been this way > for 10 years and I am not going to make an incompatible change now.
I agree with you on this Graham. With the benefit of hindsight, it probably wasn't a good idea to mess with the signature for read. But it's too late now. I'm changing IO::Uncompress::* to make the problem go away, but perhaps it might be worth adding a note in the Net::FTP documentation to highlight the non-standard nature of the read signature. Paul