Bug #116624 for Socket: Better error messages for undef arguments to pack/unpack helpers

On Mon Aug 01 08:53:58 2016, PEVANS wrote: Show quoted text
Fixed. $ perl -Mblib -MSocket=pack_sockaddr_in -E 'pack_sockaddr_in 0, undef' Undefined address for Socket::pack_sockaddr_in at -e line 1. -- Paul Evans

=== modified file 'Socket.xs' --- Socket.xs 2016-03-09 23:39:39 +0000 +++ Socket.xs 2016-08-01 14:31:44 +0000 @@ -817,6 +817,9 @@ char * pathname_pv; int addr_len; + if (!SvOK(pathname)) + croak("Undefined path for %s", "Socket::pack_sockaddr_un"); + Zero(&sun_ad, sizeof(sun_ad), char); sun_ad.sun_family = AF_UNIX; pathname_pv = SvPV(pathname,len); @@ -877,8 +880,11 @@ #ifdef I_SYS_UN struct sockaddr_un addr; STRLEN sockaddrlen; - char * sun_ad = SvPVbyte(sun_sv,sockaddrlen); + char * sun_ad; int addr_len = 0; + if (!SvOK(sun_sv)) + croak("Undefined address for %s", "Socket::unpack_sockaddr_un"); + sun_ad = SvPVbyte(sun_sv,sockaddrlen); # if defined(__linux__) || defined(HAS_SOCKADDR_SA_LEN) /* On Linux or *BSD sockaddrlen on sockets returned by accept, recvfrom, getpeername and getsockname is not equal to sizeof(addr). */ @@ -931,15 +937,21 @@ } void -pack_sockaddr_in(port, ip_address_sv) - unsigned short port +pack_sockaddr_in(port_sv, ip_address_sv) + SV * port_sv SV * ip_address_sv CODE: { struct sockaddr_in sin; struct in_addr addr; STRLEN addrlen; + unsigned short port; char * ip_address; + if (!SvOK(port_sv)) + croak("Undefined port for %s", "Socket::pack_sockaddr_in"); + port = SvUV(port_sv); + if (!SvOK(ip_address_sv)) + croak("Undefined address for %s", "Socket::pack_sockaddr_in"); if (DO_UTF8(ip_address_sv) && !sv_utf8_downgrade(ip_address_sv, 1)) croak("Wide character in %s", "Socket::pack_sockaddr_in"); ip_address = SvPVbyte(ip_address_sv, addrlen); @@ -971,7 +983,10 @@ STRLEN sockaddrlen; struct sockaddr_in addr; SV *ip_address_sv; - char * sin = SvPVbyte(sin_sv,sockaddrlen); + char * sin; + if (!SvOK(sin_sv)) + croak("Undefined address for %s", "Socket::unpack_sockaddr_in"); + sin = SvPVbyte(sin_sv,sockaddrlen); if (sockaddrlen != sizeof(addr)) { croak("Bad arg length for %s, length is %"UVuf", should be %"UVuf, "Socket::unpack_sockaddr_in", (UV)sockaddrlen, (UV)sizeof(addr)); @@ -994,17 +1009,23 @@ } void -pack_sockaddr_in6(port, sin6_addr, scope_id=0, flowinfo=0) - unsigned short port +pack_sockaddr_in6(port_sv, sin6_addr, scope_id=0, flowinfo=0) + SV * port_sv SV * sin6_addr unsigned long scope_id unsigned long flowinfo CODE: { #ifdef HAS_SOCKADDR_IN6 + unsigned short port; struct sockaddr_in6 sin6; char * addrbytes; STRLEN addrlen; + if (!SvOK(port_sv)) + croak("Undefined port for %s", "Socket::pack_sockaddr_in6"); + port = SvUV(port_sv); + if (!SvOK(sin6_addr)) + croak("Undefined address for %s", "Socket::pack_sockaddr_in6"); if (DO_UTF8(sin6_addr) && !sv_utf8_downgrade(sin6_addr, 1)) croak("Wide character in %s", "Socket::pack_sockaddr_in6"); addrbytes = SvPVbyte(sin6_addr, addrlen); @@ -1042,8 +1063,11 @@ #ifdef HAS_SOCKADDR_IN6 STRLEN addrlen; struct sockaddr_in6 sin6; - char * addrbytes = SvPVbyte(sin6_sv, addrlen); + char * addrbytes; SV *ip_address_sv; + if (!SvOK(sin6_sv)) + croak("Undefined address for %s", "Socket::unpack_sockaddr_in6"); + addrbytes = SvPVbyte(sin6_sv, addrlen); if (addrlen != sizeof(sin6)) croak("Bad arg length for %s, length is %"UVuf", should be %"UVuf, "Socket::unpack_sockaddr_in6", (UV)addrlen, (UV)sizeof(sin6)); === modified file 't/sockaddr.t' --- t/sockaddr.t 2012-08-18 21:02:58 +0000 +++ t/sockaddr.t 2016-08-01 14:28:11 +0000 @@ -10,7 +10,7 @@ sockaddr_family sockaddr_un ); -use Test::More tests => 33; +use Test::More tests => 44; # inet_aton, inet_ntoa { @@ -80,6 +80,17 @@ is(sockaddr_family(scalar sockaddr_in(200,v10.30.50.70)), AF_INET, 'sockaddr_in in scalar context packs'); + + my $warnings = 0; + local $SIG{__WARN__} = sub { $warnings++ }; + ok( !eval { pack_sockaddr_in undef, ""; 1 }, + 'pack_sockaddr_in undef port is fatal' ); + ok( !eval { pack_sockaddr_in 0, undef; 1 }, + 'pack_sockaddr_in undef addr is fatal' ); + ok( !eval { unpack_sockaddr_in undef; 1 }, + 'unpack_sockaddr_in undef is fatal' ); + + is( $warnings, 0, 'undefined values produced no warnings' ); } # pack_sockaddr_in6, unpack_sockaddr_in6 @@ -104,6 +115,17 @@ is(sockaddr_family(scalar Socket::sockaddr_in6(0x1357, "02468ace13579bdf")), $AF_INET6, 'sockaddr_in6 in scalar context packs' ); + + my $warnings = 0; + local $SIG{__WARN__} = sub { $warnings++ }; + ok( !eval { Socket::pack_sockaddr_in6( undef, "" ); 1 }, + 'pack_sockaddr_in6 undef port is fatal' ); + ok( !eval { Socket::pack_sockaddr_in6( 0, undef ); 1 }, + 'pack_sockaddr_in6 undef addr is fatal' ); + ok( !eval { Socket::unpack_sockaddr_in6( undef ); 1 }, + 'unpack_sockaddr_in6 undef is fatal' ); + + is( $warnings, 0, 'undefined values produced no warnings' ); } # sockaddr_un @@ -118,6 +140,15 @@ # see if we calculate the address structure length correctly is(length ($test_abstract_socket) + 2, length $addr, 'sockaddr_un abstract address length'); + + my $warnings = 0; + local $SIG{__WARN__} = sub { $warnings++ }; + ok( !eval { pack_sockaddr_un( undef ); 1 }, + 'pack_sockaddr_un undef path is fatal' ); + ok( !eval { unpack_sockaddr_un( undef ); 1 }, + 'unpack_sockaddr_un undef is fatal' ); + + is( $warnings, 0, 'undefined values produced no warnings' ); } # warnings