Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IO::Socket::INET fails because of IPv6 #15175

Open
p5pRT opened this issue Feb 11, 2016 · 9 comments
Open

IO::Socket::INET fails because of IPv6 #15175

p5pRT opened this issue Feb 11, 2016 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 11, 2016

Migrated from rt.perl.org#127519 (status was 'open')

Searchable as RT127519$

@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2016

From shawnw@speakeasy.org

Created by shawnw.mobile@gmail.com

I ran into problems with a script that uses IO​::Socket​::INET to connect
to a remote host failing. Turns out the module uses gethostbyname() to
resolve hostnames to addresses, and was getting IPv6 addresses from it
due to /etc/resolv.conf options, which weren't working too well with
functions expecting IPv4 format. Below is a patch to make it use
getaddrinfo and always force IPv4 resolution. Making it work
transparently with both and adding the domain as a new parameter was
another option, but the documentation says the module is specifically
for the AF_INET domain, and a perl6 bug report (#123282) on the same
issue doesn't have enough discussion to have any consensus on the best
approach. I lean towards IPv4 only and maybe have a new module that uses
both IO​::Socket​::INET and IO​::Socket​::INET6 (from cpan) under the hood.

Patch​:
From e9c029a06c1b167c7ee5196403d40d5ac0a874a6 Mon Sep 17 00​:00​:00 2001
From​: Shawn Wagner <shawnw.mobile@​gmail.com>
Date​: Thu, 11 Feb 2016 11​:36​:49 -0800
Subject​: [PATCH] Make IO​::Socket​::INET use getaddrinfo instead of
gethostbyname

gethostbyname() can return IPv6 addresses, which obviously don't play
well with the IPv4 expecting functions in this module. Now uses getaddrinfo()
to turn a hostname into a list of possible IP addresses, forcing IPv4 only
lookup.
---
dist/IO/lib/IO/Socket/INET.pm | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

Inline Patch
diff --git a/dist/IO/lib/IO/Socket/INET.pm b/dist/IO/lib/IO/Socket/INET.pm
index 7a16947..ac01b7a 100644
--- a/dist/IO/lib/IO/Socket/INET.pm
+++ b/dist/IO/lib/IO/Socket/INET.pm
@@ -123,10 +123,16 @@ sub _error {
 }
 
 sub _get_addr {
-    my($sock,$addr_str, $multi) = @_;
+    my($sock,$addr_str, $type, $proto, $multi) = @_;
     my @addr;
     if ($multi && $addr_str !~ /^\d+(?:\.\d+){3}$/) {
-	(undef, undef, undef, undef, @addr) = gethostbyname($addr_str);
+        my $err;
+        ($err, @addr) = getaddrinfo($addr_str, "",
+                                    {family => AF_INET,
+                                     socktype => $type,
+                                     protocol => $proto});
+        return () if $err != 0;
+        @addr = map {  $_->{"addr"} } @addr;
     } else {
 	my $h = inet_aton($addr_str);
 	push(@addr, $h) if defined $h;
@@ -170,7 +176,7 @@ sub configure {
     my @raddr = ();
 
     if(defined $raddr) {
-	@raddr = $sock->_get_addr($raddr, $arg->{MultiHomed});
+	@raddr = $sock->_get_addr($raddr, $type, $proto, $arg->{MultiHomed});
 	return _error($sock, $EINVAL, "Bad hostname '",$arg->{PeerAddr},"'")
 	    unless @raddr;
     }
-- 
2.7.0
Perl Info

Flags:
    category=library
    severity=medium
    module=IO::Socket::INET

Site configuration information for perl 5.22.1:

Configured by builduser at Tue Dec 15 10:04:11 CET 2015.

Summary of my perl5 (revision 5 version 22 subversion 1) configuration:
   
  Platform:
    osname=linux, osvers=4.3.2-1-arch, archname=x86_64-linux-thread-multi
    uname='linux flo-64 4.3.2-1-arch #1 smp preempt fri dec 11 07:19:47 cet 2015 x86_64 gnulinux '
    config_args='-des -Dusethreads -Duseshrplib -Doptimize=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -Dprefix=/usr -Dvendorprefix=/usr -Dprivlib=/usr/share/perl5/core_perl -Darchlib=/usr/lib/perl5/core_perl -Dsitelib=/usr/share/perl5/site_perl -Dsitearch=/usr/lib/perl5/site_perl -Dvendorlib=/usr/share/perl5/vendor_perl -Dvendorarch=/usr/lib/perl5/vendor_perl -Dscriptdir=/usr/bin/core_perl -Dsitescript=/usr/bin/site_perl -Dvendorscript=/usr/bin/vendor_perl -Dinc_version_list=none -Dman1ext=1perl -Dman3ext=3perl -Dcccdlflags='-fPIC' -Dlddlflags=-shared -Wl,-O1,--sort-common,--as-needed,-z,relro -Dldflags=-Wl,-O1,--sort-common,--as-needed,-z,relro'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion='', gccversion='5.3.0', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags ='-Wl,-O1,--sort-common,--as-needed,-z,relro -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-unknown-linux-gnu/5.3.0/include-fixed /usr/lib /lib/../lib /usr/lib/../lib /lib /lib64 /usr/lib64
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.22.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.22'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib/perl5/core_perl/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -Wl,-O1,--sort-common,--as-needed,-z,relro -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.22.1:
    /usr/lib/perl5/site_perl
    /usr/share/perl5/site_perl
    /usr/lib/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib/perl5/core_perl
    /usr/share/perl5/core_perl
    .


Environment for perl 5.22.1:
    HOME=/home/shawnw
    LANG=en_US.iso88591
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/home/shawnw/bin/:/home/shawnw/bin/:/home/shawnw/bin/
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh


@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2016

From @jkeenan

On Thu Feb 11 12​:11​:21 2016, shawnw@​speakeasy.org wrote​:

This is a bug report for perl from shawnw.mobile@​gmail.com, generated
with the help of perlbug 1.40 running under perl 5.22.1.

-----------------------------------------------------------------
[Please describe your issue here]

I ran into problems with a script that uses IO​::Socket​::INET to
connect
to a remote host failing. Turns out the module uses gethostbyname() to
resolve hostnames to addresses, and was getting IPv6 addresses from it
due to /etc/resolv.conf options, which weren't working too well with
functions expecting IPv4 format. Below is a patch to make it use
getaddrinfo and always force IPv4 resolution. Making it work
transparently with both and adding the domain as a new parameter was
another option, but the documentation says the module is specifically
for the AF_INET domain, and a perl6 bug report (#123282) on the same
issue doesn't have enough discussion to have any consensus on the best
approach. I lean towards IPv4 only and maybe have a new module that
uses
both IO​::Socket​::INET and IO​::Socket​::INET6 (from cpan) under the
hood.

Patch​:
From e9c029a06c1b167c7ee5196403d40d5ac0a874a6 Mon Sep 17 00​:00​:00 2001
From​: Shawn Wagner <shawnw.mobile@​gmail.com>
Date​: Thu, 11 Feb 2016 11​:36​:49 -0800
Subject​: [PATCH] Make IO​::Socket​::INET use getaddrinfo instead of
gethostbyname

gethostbyname() can return IPv6 addresses, which obviously don't play
well with the IPv4 expecting functions in this module. Now uses
getaddrinfo()
to turn a hostname into a list of possible IP addresses, forcing IPv4
only
lookup.
---
dist/IO/lib/IO/Socket/INET.pm | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/dist/IO/lib/IO/Socket/INET.pm
b/dist/IO/lib/IO/Socket/INET.pm
index 7a16947..ac01b7a 100644
--- a/dist/IO/lib/IO/Socket/INET.pm
+++ b/dist/IO/lib/IO/Socket/INET.pm
@​@​ -123,10 +123,16 @​@​ sub _error {
}

sub _get_addr {
- my($sock,$addr_str, $multi) = @​_;
+ my($sock,$addr_str, $type, $proto, $multi) = @​_;
my @​addr;
if ($multi && $addr_str !~ /^\d+(?​:\.\d+){3}$/) {
- (undef, undef, undef, undef, @​addr) =
gethostbyname($addr_str);
+ my $err;
+ ($err, @​addr) = getaddrinfo($addr_str, "",
+ {family => AF_INET,
+ socktype => $type,
+ protocol => $proto});
+ return () if $err != 0;
+ @​addr = map { $_->{"addr"} } @​addr;
} else {
my $h = inet_aton($addr_str);
push(@​addr, $h) if defined $h;
@​@​ -170,7 +176,7 @​@​ sub configure {
my @​raddr = ();

if(defined $raddr) {
- @​raddr = $sock->_get_addr($raddr, $arg->{MultiHomed});
+ @​raddr = $sock->_get_addr($raddr, $type, $proto, $arg-

{MultiHomed});
return _error($sock, $EINVAL, "Bad hostname '",$arg-
{PeerAddr},"'")
unless @​raddr;
}

We should see whether this is the same bug as was reported in https://rt-archive.perl.org/perl5/Ticket/Display.html?id=75740.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Feb 12, 2016

From @jkeenan

On Thu Feb 11 12​:11​:21 2016, shawnw@​speakeasy.org wrote​:

This is a bug report for perl from shawnw.mobile@​gmail.com, generated
with the help of perlbug 1.40 running under perl 5.22.1.

-----------------------------------------------------------------
[Please describe your issue here]

I ran into problems with a script that uses IO​::Socket​::INET to
connect
to a remote host failing. Turns out the module uses gethostbyname() to
resolve hostnames to addresses, and was getting IPv6 addresses from it
due to /etc/resolv.conf options, which weren't working too well with
functions expecting IPv4 format. Below is a patch to make it use
getaddrinfo and always force IPv4 resolution. Making it work
transparently with both and adding the domain as a new parameter was
another option, but the documentation says the module is specifically
for the AF_INET domain, and a perl6 bug report (#123282) on the same
issue doesn't have enough discussion to have any consensus on the best
approach. I lean towards IPv4 only and maybe have a new module that
uses
both IO​::Socket​::INET and IO​::Socket​::INET6 (from cpan) under the
hood.

Patch​:
From e9c029a06c1b167c7ee5196403d40d5ac0a874a6 Mon Sep 17 00​:00​:00 2001
From​: Shawn Wagner <shawnw.mobile@​gmail.com>
Date​: Thu, 11 Feb 2016 11​:36​:49 -0800
Subject​: [PATCH] Make IO​::Socket​::INET use getaddrinfo instead of
gethostbyname

gethostbyname() can return IPv6 addresses, which obviously don't play
well with the IPv4 expecting functions in this module. Now uses
getaddrinfo()
to turn a hostname into a list of possible IP addresses, forcing IPv4
only
lookup.
---
dist/IO/lib/IO/Socket/INET.pm | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/dist/IO/lib/IO/Socket/INET.pm
b/dist/IO/lib/IO/Socket/INET.pm
index 7a16947..ac01b7a 100644
--- a/dist/IO/lib/IO/Socket/INET.pm
+++ b/dist/IO/lib/IO/Socket/INET.pm
@​@​ -123,10 +123,16 @​@​ sub _error {
}

sub _get_addr {
- my($sock,$addr_str, $multi) = @​_;
+ my($sock,$addr_str, $type, $proto, $multi) = @​_;
my @​addr;
if ($multi && $addr_str !~ /^\d+(?​:\.\d+){3}$/) {
- (undef, undef, undef, undef, @​addr) =
gethostbyname($addr_str);
+ my $err;
+ ($err, @​addr) = getaddrinfo($addr_str, "",
+ {family => AF_INET,
+ socktype => $type,
+ protocol => $proto});
+ return () if $err != 0;
+ @​addr = map { $_->{"addr"} } @​addr;
} else {
my $h = inet_aton($addr_str);
push(@​addr, $h) if defined $h;
@​@​ -170,7 +176,7 @​@​ sub configure {
my @​raddr = ();

if(defined $raddr) {
- @​raddr = $sock->_get_addr($raddr, $arg->{MultiHomed});
+ @​raddr = $sock->_get_addr($raddr, $type, $proto, $arg-

{MultiHomed});
return _error($sock, $EINVAL, "Bad hostname '",$arg-
{PeerAddr},"'")
unless @​raddr;
}

The patch was submitted inline and did not apply cleanly. I manually applied it to a branch and tidied up the formatting somewhat. See attachment. 'make test' PASSes.

I'm not an expert in this area, so I'll defer to others as to whether the patch should be applied or not.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Feb 12, 2016

From @jkeenan

127519-0001-Use-getaddrinfo-and-always-force-IPv4-resolution.patch
From a5579ab1523fc46236c3dc350cf092d5766e7d82 Mon Sep 17 00:00:00 2001
From: Shawn Wagner <shawnw.mobile@gmail.com>
Date: Thu, 11 Feb 2016 21:22:08 -0500
Subject: [PATCH] Use getaddrinfo and always force IPv4 resolution.

Increment $VERSION.  Add Shawn Wagner to AUTHORS.

For RT # 127519
---
 AUTHORS                       |  1 +
 dist/IO/lib/IO/Socket/INET.pm | 33 +++++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 3cc2ef1..bbe13c3 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -1098,6 +1098,7 @@ Sérgio Durigan Júnior		<sergiodj@linux.vnet.ibm.com>
 Sergey Alekseev			<varnie29a@mail.ru>
 Shawn				<svicalifornia@gmail.com>
 Shawn M Moore			<sartak@gmail.com>
+Shawn Wagner			<shawnw.mobile@gmail.com>
 Sherm Pendley			<sherm@dot-app.org>
 Shigeya Suzuki			<shigeya@wide.ad.jp>
 Shimpei Yamashita		<shimpei@socrates.patnet.caltech.edu>
diff --git a/dist/IO/lib/IO/Socket/INET.pm b/dist/IO/lib/IO/Socket/INET.pm
index 7a16947..eceb08d 100644
--- a/dist/IO/lib/IO/Socket/INET.pm
+++ b/dist/IO/lib/IO/Socket/INET.pm
@@ -15,7 +15,7 @@ use Exporter;
 use Errno;
 
 @ISA = qw(IO::Socket);
-$VERSION = "1.35";
+$VERSION = "1.36";
 
 my $EINVAL = exists(&Errno::EINVAL) ? Errno::EINVAL() : 1;
 
@@ -123,15 +123,28 @@ sub _error {
 }
 
 sub _get_addr {
-    my($sock,$addr_str, $multi) = @_;
+    my($sock,$addr_str, $type, $proto, $multi) = @_;
+
     my @addr;
     if ($multi && $addr_str !~ /^\d+(?:\.\d+){3}$/) {
-	(undef, undef, undef, undef, @addr) = gethostbyname($addr_str);
-    } else {
-	my $h = inet_aton($addr_str);
-	push(@addr, $h) if defined $h;
+        my $err;
+        ($err, @addr) = getaddrinfo(
+            $addr_str,
+            "",
+            {
+                family => AF_INET,
+                socktype => $type,
+                protocol => $proto,
+            }
+        );
+        return () if $err != 0;
+        @addr = map {  $_->{"addr"} } @addr;
+    }
+    else {
+        my $h = inet_aton($addr_str);
+        push(@addr, $h) if defined $h;
     }
-    @addr;
+    return @addr;
 }
 
 sub configure {
@@ -170,9 +183,9 @@ sub configure {
     my @raddr = ();
 
     if(defined $raddr) {
-	@raddr = $sock->_get_addr($raddr, $arg->{MultiHomed});
-	return _error($sock, $EINVAL, "Bad hostname '",$arg->{PeerAddr},"'")
-	    unless @raddr;
+        @raddr = $sock->_get_addr($raddr, $type, $proto, $arg->{MultiHomed});
+        return _error($sock, $EINVAL, "Bad hostname '",$arg->{PeerAddr},"'")
+            unless @raddr;
     }
 
     while(1) {
-- 
1.9.1

@p5pRT
Copy link
Author

p5pRT commented Feb 12, 2016

From @leonerd

On Thu, 11 Feb 2016 18​:33​:30 -0800
"James E Keenan via RT" <perlbug-followup@​perl.org> wrote​:

On Thu Feb 11 12​:11​:21 2016, shawnw@​speakeasy.org wrote​:

This is a bug report for perl from shawnw.mobile@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.22.1.

-----------------------------------------------------------------
[Please describe your issue here]

I ran into problems with a script that uses IO​::Socket​::INET to
connect
to a remote host failing. Turns out the module uses gethostbyname()
to resolve hostnames to addresses, and was getting IPv6 addresses
from it due to /etc/resolv.conf options, which weren't working too
well with functions expecting IPv4 format. Below is a patch to make
it use getaddrinfo and always force IPv4 resolution.

This seems a pointless thing to do - on any machine that has
getaddrinfo(), you can just use IO​::Socket​::IP directly. The only
use-case for still maintaining :​:INET is for sufficiently-legacy
machines that do not possess a useable getaddrinfo() and so must use
the older gethostbyname() instead.

Making it work
transparently with both and adding the domain as a new parameter was
another option, but the documentation says the module is
specifically for the AF_INET domain, and a perl6 bug report
(#123282) on the same issue doesn't have enough discussion to have
any consensus on the best approach.

I lean towards IPv4 only and
maybe have a new module that uses
both IO​::Socket​::INET and IO​::Socket​::INET6 (from cpan) under the
hood.

Core already has IO​::Socket​::IP, which does transparent IPv{n}
connections by using getaddrinfo(), as is the correct modern approach.

...

The patch was submitted inline and did not apply cleanly. I manually
applied it to a branch and tidied up the formatting somewhat. See
attachment. 'make test' PASSes.

I'm not an expert in this area, so I'll defer to others as to whether
the patch should be applied or not.

Thoughts above.

Additionally, this bug sounds like a more fundamental platform problem
to me. gethostbyname() should never return anything other than IPv4
addresses normally. For example, for me I have a machine that resolves
to both 'v4 and 'v6 addresses​:

  $ socket_getaddrinfo cel --stream
  Resolved host 'cel', service '0'

  socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP) + '[2001​:8b0​:3f7​::2]​:0'
  socket(AF_INET , SOCK_STREAM, IPPROTO_TCP) + '192.168.42.2​:0'

Yet, gethostbyname() still only gives me 'v4 addresses​:

  $ perl -MData​::Dump=pp -E 'say pp( gethostbyname "cel" )'
  ("cel.leo", "", 2, 4, "\xC0\xA8*\2")

If I specifically ask for something that has only v6 addresses​:

  $ socket_getaddrinfo cel-v6 --stream
  Resolved host 'cel-v6', service '0'

  socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP) + '[2001​:8b0​:3f7​::2]​:0'

then gethostbyname() returns me nothing​:

  $ perl -MData​::Dump=pp -E 'say pp( gethostbyname "cel-v6" )'
  ()

As far as I read it, IO​::Socket​::INET is perfectly fine to rely on the
return value of gethostbyname() containing only addresses in a form it
can use (i.e. AF_INET for IPv4). Furthermore, this patch now reduces
the platforms that :​:INET can work on, because now they need a working
getaddrinfo().

I recommend reverting it.

--
Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk
http​://www.leonerd.org.uk/ | https://metacpan.org/author/PEVANS

@p5pRT
Copy link
Author

p5pRT commented Feb 13, 2016

From @jkeenan

On 02/12/2016 08​:05 AM, Paul "LeoNerd" Evans wrote​:

On Thu, 11 Feb 2016 18​:33​:30 -0800
"James E Keenan via RT" <perlbug-followup@​perl.org> wrote​:

On Thu Feb 11 12​:11​:21 2016, shawnw@​speakeasy.org wrote​:

This is a bug report for perl from shawnw.mobile@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.22.1.

-----------------------------------------------------------------
[Please describe your issue here]

I ran into problems with a script that uses IO​::Socket​::INET to
connect
to a remote host failing. Turns out the module uses gethostbyname()
to resolve hostnames to addresses, and was getting IPv6 addresses
from it due to /etc/resolv.conf options, which weren't working too
well with functions expecting IPv4 format. Below is a patch to make
it use getaddrinfo and always force IPv4 resolution.

This seems a pointless thing to do - on any machine that has
getaddrinfo(), you can just use IO​::Socket​::IP directly. The only
use-case for still maintaining :​:INET is for sufficiently-legacy
machines that do not possess a useable getaddrinfo() and so must use
the older gethostbyname() instead.

Making it work
transparently with both and adding the domain as a new parameter was
another option, but the documentation says the module is
specifically for the AF_INET domain, and a perl6 bug report
(#123282) on the same issue doesn't have enough discussion to have
any consensus on the best approach.

I lean towards IPv4 only and
maybe have a new module that uses
both IO​::Socket​::INET and IO​::Socket​::INET6 (from cpan) under the
hood.

Core already has IO​::Socket​::IP, which does transparent IPv{n}
connections by using getaddrinfo(), as is the correct modern approach.

...

The patch was submitted inline and did not apply cleanly. I manually
applied it to a branch and tidied up the formatting somewhat. See
attachment. 'make test' PASSes.

I'm not an expert in this area, so I'll defer to others as to whether
the patch should be applied or not.

Thoughts above.

Additionally, this bug sounds like a more fundamental platform problem
to me. gethostbyname() should never return anything other than IPv4
addresses normally. For example, for me I have a machine that resolves
to both 'v4 and 'v6 addresses​:

$ socket_getaddrinfo cel --stream
Resolved host 'cel', service '0'

socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP) + '[2001​:8b0​:3f7​::2]​:0'
socket(AF_INET , SOCK_STREAM, IPPROTO_TCP) + '192.168.42.2​:0'

Yet, gethostbyname() still only gives me 'v4 addresses​:

$ perl -MData​::Dump=pp -E 'say pp( gethostbyname "cel" )'
("cel.leo", "", 2, 4, "\xC0\xA8*\2")

If I specifically ask for something that has only v6 addresses​:

$ socket_getaddrinfo cel-v6 --stream
Resolved host 'cel-v6', service '0'

socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP) + '[2001​:8b0​:3f7​::2]​:0'

then gethostbyname() returns me nothing​:

$ perl -MData​::Dump=pp -E 'say pp( gethostbyname "cel-v6" )'
()

As far as I read it, IO​::Socket​::INET is perfectly fine to rely on the
return value of gethostbyname() containing only addresses in a form it
can use (i.e. AF_INET for IPv4). Furthermore, this patch now reduces
the platforms that :​:INET can work on, because now they need a working
getaddrinfo().

I recommend reverting it.

I didn't apply the patch; I merely prepared it in a more usable form.
So there's nothing to revert.

Anyone else want to sound in?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Feb 15, 2016

From shawnw@speakeasy.org

On Fri, 12 Feb 2016 05​:06​:07 -0800
"Paul LeoNerd Evans via RT" <perlbug-followup@​perl.org> wrote​:

On Thu, 11 Feb 2016 18​:33​:30 -0800
"James E Keenan via RT" <perlbug-followup@​perl.org> wrote​:

On Thu Feb 11 12​:11​:21 2016, shawnw@​speakeasy.org wrote​:

This is a bug report for perl from shawnw.mobile@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.22.1.

This seems a pointless thing to do - on any machine that has
getaddrinfo(), you can just use IO​::Socket​::IP directly. The only
use-case for still maintaining :​:INET is for sufficiently-legacy
machines that do not possess a useable getaddrinfo() and so must use
the older gethostbyname() instead.

I didn't know about IO​::Socket​::IP. Still, what OSes don't support
getaddrinfo()? Even Windows has it! I disagree that fixing a bug that
breaks a core module is pointless. I'm also not sure of a better way to
fix it; glibc has gethostbyname2(), but if you think a standard
function like getaddrinfo() isn't portable enough...

Suppose you could also check to make sure the h_addrtype field to see
if it's AF_INET and just fail otherwise, but I'd rather have something
that works.

Additionally, this bug sounds like a more fundamental platform problem
to me. gethostbyname() should never return anything other than IPv4
addresses normally. For example, for me I have a machine that resolves
to both 'v4 and 'v6 addresses​:

It's a fundamental design problem dating back to the early days before
the concept of having computers with multiple IP stacks and having to
deal with both. getaddrinfo() is the solution.

$ socket_getaddrinfo cel --stream
Resolved host 'cel', service '0'

socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP) + '[2001​:8b0​:3f7​::2]​:0'
socket(AF_INET , SOCK_STREAM, IPPROTO_TCP) + '192.168.42.2​:0'

Yet, gethostbyname() still only gives me 'v4 addresses​:

$ perl -MData​::Dump=pp -E 'say pp( gethostbyname "cel" )'
("cel.leo", "", 2, 4, "\xC0\xA8*\2")

I get

(
  "localhost.localdomain",
  "localhost localhost",
  10,
  16,
  "\0\0\0\0\0\0\0\0\0\0\xFF\xFF\x7F\0\0\1",
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\1",
)

What gethostbyname() returns depends on how your resolver is
configured. I change one line in /etc/resolve.conf and get

(
  "localhost.localdomain",
  "localhost localhost",
  2,
  4,
  "\x7F\0\0\1",
  "\x7F\0\0\1",
)

I don't want to have to do that just so an old program works right. I'd
rather fix the root bug in the library the program uses until such
time as the program itself can be made a bit more modern but still
functions in the meantime.

--
Shawn Wagner
shawnw@​speakeasy.org

@p5pRT
Copy link
Author

p5pRT commented Feb 15, 2016

From @leonerd

On Fri, 12 Feb 2016 13​:01​:42 -0800
Shawn Wagner <shawnw@​speakeasy.org> wrote​:

Suppose you could also check to make sure the h_addrtype field to see
if it's AF_INET and just fail otherwise, but I'd rather have something
that works.

That's probably a better approach, then. Given as the gethostby*()
return value currently indicates the address family too, just have it
assert that it definitely got AF_INET otherwise clear out the list.
That's a oneline fix.

Line 129​:

  (undef, undef, undef, undef, @​addr) = gethostbyname($addr_str);

change that to

  (undef, undef, my $addrtype, undef, @​addr) = gethostbyname($addr_str);
  @​addr = () unless $addrtype == AF_INET;

Then at least it will not get confused by non-AF_INET return values.

--
Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk
http​://www.leonerd.org.uk/ | https://metacpan.org/author/PEVANS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants