Bug #88965 for libnet: bug: uninitialized value $code in substr warning

Wed Sep 25 11:11:42 2013 winston [...] rentec.com - Ticket created

Subject:	bug: uninitialized value $code in substr warning
Date:	Wed, 25 Sep 2013 11:11:20 -0400 (EDT)
To:	bug-libnet [...] rt.cpan.org
From:	Winston Wang <winston [...] rentec.com>

Hello, I'd like to report a minor bug, but one that I believe has a simple and obvious fix. In Net/Cmd.pm, the response function ends with a call to: substr($code,0,1); However, the variable $code may be uninitialized. This is the source of warnings I am getting in a script: Use of uninitialized value $code in substr at /usr/local/products/perl/5.16.0/lib/5.16.0/Net/Cmd.pm line 366. Argument "" isn't numeric in numeric ne (!=) at /usr/local/products/perl/5.16.0/Net/FTP.pm line 1082 My proposed fix is to modify this block of code: ($code,$more) = $cmd->parse_response($str); unless(defined $code) { $cmd->ungetline($str); last; } to become this: ($code,$more) = $cmd->parse_response($str); unless(defined $code) { $cmd->ungetline($str); return CMD_ERROR; } Returning here instead of breaking out of the loop avoids the substr() call, and ensures that the function does not return undef, which will prevent the warning message in Net/FTP.pm. Below is a patch for your convenience. Thank you very much for your time, Winston --- Cmd.pm.orig 2013-09-25 11:03:42.397090000 -0400 +++ Cmd.pm 2013-09-25 11:04:05.718207000 -0400 @@ -341,7 +341,7 @@ unless(defined $code) { $cmd->ungetline($str); - last; + return CMD_ERROR; } ${*$cmd}{'net_cmd_code'} = $code;

Wed Jan 29 03:48:18 2014 shay [...] cpan.org - Correspondence added

On Wed Sep 25 11:11:42 2013, winston@rentec.com wrote: Show quoted text

> > Hello, > > I'd like to report a minor bug, but one that I believe has a simple and > obvious fix. In Net/Cmd.pm, the response function ends with a call to: > > substr($code,0,1); > > However, the variable $code may be uninitialized. This is the source of > warnings I am getting in a script: > > Use of uninitialized value $code in substr at > /usr/local/products/perl/5.16.0/lib/5.16.0/Net/Cmd.pm line 366. > > Argument "" isn't numeric in numeric ne (!=) at > /usr/local/products/perl/5.16.0/Net/FTP.pm line 1082 > > My proposed fix is to modify this block of code: > > ($code,$more) = $cmd->parse_response($str); > unless(defined $code) > { > $cmd->ungetline($str); > last; > } > > to become this: > > ($code,$more) = $cmd->parse_response($str); > unless(defined $code) > { > $cmd->ungetline($str); > return CMD_ERROR; > } > > Returning here instead of breaking out of the loop avoids the substr() > call, and ensures that the function does not return undef, which will > prevent the warning message in Net/FTP.pm. Below is a patch for your > convenience. > > Thank you very much for your time, > > Winston > > > --- Cmd.pm.orig 2013-09-25 11:03:42.397090000 -0400 > +++ Cmd.pm 2013-09-25 11:04:05.718207000 -0400 > @@ -341,7 +341,7 @@ > unless(defined $code) > { > $cmd->ungetline($str); > - last; > + return CMD_ERROR; > } > > ${*$cmd}{'net_cmd_code'} = $code; >

How is the substr() getting reached in this case? The function returns undef on the line before the substr() in exactly the case in question -- when $code is not defined. Also, changing the return value from undef to CMD_ERROR (5) may also break things since it is documented to return undef on failure.

Wed Jan 29 03:48:19 2014 The RT System itself - Status changed from 'new' to 'open'

Wed Jan 29 11:40:39 2014 winston [...] rentec.com - Correspondence added

Subject:	Re: [rt.cpan.org #88965] bug: uninitialized value $code in substr warning
Date:	Wed, 29 Jan 2014 11:40:24 -0500 (EST)
To:	Steve Hay via RT <bug-libnet [...] rt.cpan.org>
From:	Winston Wang <winston [...] rentec.com>

Hi Steve, Thanks you for looking into this. Perhaps we are looking at different versions of the code because this statement is not true for me: "The function returns undef on the line before the substr() in exactly the case in question -- when $code is not defined." For clarity, here is what I currently see in the code: while(1) { my $str = $cmd->getline(); return CMD_ERROR unless defined($str); $cmd->debug_print(0,$str) if ($cmd->debug); ($code,$more) = $cmd->parse_response($str); unless(defined $code) { $cmd->ungetline($str); last; } ${*$cmd}{'net_cmd_code'} = $code; push(@{${*$cmd}{'net_cmd_resp'}},$str); last unless($more); } substr($code,0,1); When $code is undefined, the function calls "last", which breaks us out of the while loop, which leads to the substr() call. By the way, these messages are no longer a problem for us because I figured out that putting "use warnings" instead of invoking perl with the "-w" argument limits the scope of the warnings. Thanks, Winston On Wed, 29 Jan 2014, Steve Hay via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=88965 > > > On Wed Sep 25 11:11:42 2013, winston@rentec.com wrote:

>> >> Hello, >> >> I'd like to report a minor bug, but one that I believe has a simple and >> obvious fix. In Net/Cmd.pm, the response function ends with a call to: >> >> substr($code,0,1); >> >> However, the variable $code may be uninitialized. This is the source of >> warnings I am getting in a script: >> >> Use of uninitialized value $code in substr at >> /usr/local/products/perl/5.16.0/lib/5.16.0/Net/Cmd.pm line 366. >> >> Argument "" isn't numeric in numeric ne (!=) at >> /usr/local/products/perl/5.16.0/Net/FTP.pm line 1082 >> >> My proposed fix is to modify this block of code: >> >> ($code,$more) = $cmd->parse_response($str); >> unless(defined $code) >> { >> $cmd->ungetline($str); >> last; >> } >> >> to become this: >> >> ($code,$more) = $cmd->parse_response($str); >> unless(defined $code) >> { >> $cmd->ungetline($str); >> return CMD_ERROR; >> } >> >> Returning here instead of breaking out of the loop avoids the substr() >> call, and ensures that the function does not return undef, which will >> prevent the warning message in Net/FTP.pm. Below is a patch for your >> convenience. >> >> Thank you very much for your time, >> >> Winston >> >> >> --- Cmd.pm.orig 2013-09-25 11:03:42.397090000 -0400 >> +++ Cmd.pm 2013-09-25 11:04:05.718207000 -0400 >> @@ -341,7 +341,7 @@ >> unless(defined $code) >> { >> $cmd->ungetline($str); >> - last; >> + return CMD_ERROR; >> } >> >> ${*$cmd}{'net_cmd_code'} = $code; >>

> > > How is the substr() getting reached in this case? The function returns undef on the line before the substr() in exactly the case in question -- when $code is not defined. > > Also, changing the return value from undef to CMD_ERROR (5) may also break things since it is documented to return undef on failure. > >

Wed Jan 29 13:22:09 2014 shay [...] cpan.org - Correspondence added

On Wed Jan 29 11:40:39 2014, winston@rentec.com wrote: Show quoted text

> > Hi Steve, > > Thanks you for looking into this. > > Perhaps we are looking at different versions of the code because this > statement is not true for me: > > "The function returns undef on the line before the substr() in exactly > the > case in question -- when $code is not defined." > > For clarity, here is what I currently see in the code: > > while(1) > { > my $str = $cmd->getline(); > > return CMD_ERROR > unless defined($str); > > $cmd->debug_print(0,$str) > if ($cmd->debug); > > ($code,$more) = $cmd->parse_response($str); > unless(defined $code) > { > $cmd->ungetline($str); > last; > } > > ${*$cmd}{'net_cmd_code'} = $code; > > push(@{${*$cmd}{'net_cmd_resp'}},$str); > > last unless($more); > } > > substr($code,0,1); > > When $code is undefined, the function calls "last", which breaks us > out of > the while loop, which leads to the substr() call. >

Ah, that's libnet-1.23's version of the code. Sorry, I should have thought to look at older versions... I'm looking at libnet-1.24, which now has this: [...] ${*$cmd}{'net_cmd_code'} = $code; push(@{${*$cmd}{'net_cmd_resp'}}, $str); last unless ($more); } return undef unless defined $code; substr($code, 0, 1); Would you be able to try out libnet-1.24 and confirm whether it fixes your issue? If so then we can close this ticket as already fixed.

Wed Feb 05 16:10:09 2014 winston [...] rentec.com - Correspondence added

Subject:	Re: [rt.cpan.org #88965] bug: uninitialized value $code in substr warning
Date:	Wed, 5 Feb 2014 16:09:57 -0500 (EST)
To:	Steve Hay via RT <bug-libnet [...] rt.cpan.org>
From:	Winston Wang <winston [...] rentec.com>

Yes, it looks like this issue has been fixed so the ticket can be closed. Thanks, Winston On Wed, 29 Jan 2014, Steve Hay via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=88965 > > > On Wed Jan 29 11:40:39 2014, winston@rentec.com wrote:

>> >> Hi Steve, >> >> Thanks you for looking into this. >> >> Perhaps we are looking at different versions of the code because this >> statement is not true for me: >> >> "The function returns undef on the line before the substr() in exactly >> the >> case in question -- when $code is not defined." >> >> For clarity, here is what I currently see in the code: >> >> while(1) >> { >> my $str = $cmd->getline(); >> >> return CMD_ERROR >> unless defined($str); >> >> $cmd->debug_print(0,$str) >> if ($cmd->debug); >> >> ($code,$more) = $cmd->parse_response($str); >> unless(defined $code) >> { >> $cmd->ungetline($str); >> last; >> } >> >> ${*$cmd}{'net_cmd_code'} = $code; >> >> push(@{${*$cmd}{'net_cmd_resp'}},$str); >> >> last unless($more); >> } >> >> substr($code,0,1); >> >> When $code is undefined, the function calls "last", which breaks us >> out of >> the while loop, which leads to the substr() call. >>

> > Ah, that's libnet-1.23's version of the code. Sorry, I should have thought to look at older versions... I'm looking at libnet-1.24, which now has this: > > [...] > ${*$cmd}{'net_cmd_code'} = $code; > > push(@{${*$cmd}{'net_cmd_resp'}}, $str); > > last unless ($more); > } > > return undef unless defined $code; > substr($code, 0, 1); > > Would you be able to try out libnet-1.24 and confirm whether it fixes your issue? If so then we can close this ticket as already fixed. > >

Thu Feb 06 03:18:16 2014 shay [...] cpan.org - Correspondence added

Thanks for confirming this is fixed now. Closing ticket.

Thu Feb 06 03:18:18 2014 shay [...] cpan.org - Status changed from 'open' to 'resolved'

Thu Feb 06 03:18:18 2014 shay [...] cpan.org - Fixed in 1.24 added