IO::Socket fails to propagate socket type information to accept()ed children #12079
Comments
From @jmdhDear all, I am filing this as a new rt.perl.org ticket, but it relates to extensive This is also tracked at I won't attempt to summarise the whole of both threads, but the current There appears to be strong reasons to apply this patch in Debian perl, The current proposed patch is http://perl5.git.perl.org/perl.git/commit/25153b9f6968c445f8551f2c0aae1c46580b7767 The smoke reports indicate that the tests currently fail on anything http://perl.develop-help.com/?b=smoke-me%2Fcpan61577-dom I would be very grateful for any fresh feedback on this issue. Thanks, -- |
From @tonycozOn Wed, May 02, 2012 at 03:31:39PM -0700, Dominic Hargreaves wrote:
At the very least it will need TODO or SKIP on the unsupported Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @nwc10On Fri, May 04, 2012 at 02:15:56PM +1000, Tony Cook wrote:
> The current proposed patch is
Sounds like it might be easier to TODO or SKIP on anything other than Rather minor, but I noticed that the new tests were written using Test. Nicholas Clark |
From dkg@fifthhorseman.netAttached is an amended patch against blead. Its main changes compared * uses Test::More instead of Test (thanks to Nicholas Clark for the * SKIPs the relevant tests on platforms where Socket::SO_TYPE or * uses File::Temp and File::Spec to construct the path for the * SKIPs all IO::Socket::UNIX tests on platforms known to not support If there are other changes that need to be made for this to be |
From dkg@fifthhorseman.netrobustly-refresh-missing-cached-socket-details-v5.patchcommit 5f4a26e9a109d48b971e34df87529cc1e10260f9
Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Date: Fri Feb 17 14:29:14 2012 -0800
[rt.cpan.org #61577] sockdomain and socktype undef on newly accepted sockets
There appears to be a flaw in IO::Socket where some IO::Socket objects
are unable to properly report their socktype, sockdomain, or protocol
(they return undef, even when the underlying socket is sufficiently
initialized to have these properties).
The attached patch should cover IO::Socket objects created via accept(),
new_from_fd(), new(), and anywhere else whose details haven't been
properly cached.
No new code should be executed on IO::Socket objects whose details are
already cached and present.
diff --git a/AUTHORS b/AUTHORS
index 88342aa..1547be2 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -250,6 +250,7 @@ Daniel Chetlin <daniel@chetlin.com>
Daniel Dragan <bulk88@hotmail.com>
Daniel Frederick Crisman <daniel@crisman.org>
Daniel Grisinger <dgris@dimensional.com>
+Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Daniel Lieberman <daniel@bitpusher.com>
Daniel Muiño <dmuino@afip.gov.ar>
Daniel P. Berrange <dan@berrange.com>
diff --git a/MANIFEST b/MANIFEST
index 2be6ea7..1f5219d 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -3259,6 +3259,9 @@ dist/IO/Makefile.PL IO extension makefile writer
dist/IO/poll.c IO poll() emulation using select()
dist/IO/poll.h IO poll() emulation using select()
dist/IO/README IO extension maintenance notice
+dist/IO/t/cachepropagate-tcp.t See if IO::Socket duplication works
+dist/IO/t/cachepropagate-udp.t See if IO::Socket duplication works
+dist/IO/t/cachepropagate-unix.t See if IO::Socket duplication works
dist/IO/t/io_const.t See if constants from IO work
dist/IO/t/io_dir.t See if directory-related methods from IO work
dist/IO/t/io_dup.t See if dup()-related methods from IO work
diff --git a/META.yml b/META.yml
index 9271e61..faa01d5 100644
--- a/META.yml
+++ b/META.yml
@@ -78,6 +78,9 @@ no_index:
- dist/IO/poll.c
- dist/IO/poll.h
- dist/IO/README
+ - dist/IO/t/cachepropagate-tcp.t
+ - dist/IO/t/cachepropagate-udp.t
+ - dist/IO/t/cachepropagate-unix.t
- dist/IO/t/IO.t
- dist/IO/t/io_const.t
- dist/IO/t/io_dir.t
diff --git a/dist/IO/lib/IO/Socket.pm b/dist/IO/lib/IO/Socket.pm
index 529423b..393f836 100644
--- a/dist/IO/lib/IO/Socket.pm
+++ b/dist/IO/lib/IO/Socket.pm
@@ -24,7 +24,7 @@ require IO::Socket::UNIX if ($^O ne 'epoc' && $^O ne 'symbian');
@ISA = qw(IO::Handle);
-$VERSION = "1.34";
+$VERSION = "1.35";
@EXPORT_OK = qw(sockatmark);
@@ -349,18 +349,27 @@ sub timeout {
sub sockdomain {
@_ == 1 or croak 'usage: $sock->sockdomain()';
my $sock = shift;
+ if (!defined(${*$sock}{'io_socket_domain'})) {
+ my $addr = $sock->sockname();
+ ${*$sock}{'io_socket_domain'} = sockaddr_family($addr)
+ if (defined($addr));
+ }
${*$sock}{'io_socket_domain'};
}
sub socktype {
@_ == 1 or croak 'usage: $sock->socktype()';
my $sock = shift;
+ ${*$sock}{'io_socket_type'} = $sock->sockopt(Socket::SO_TYPE)
+ if (!defined(${*$sock}{'io_socket_type'}) && defined(eval{Socket::SO_TYPE}));
${*$sock}{'io_socket_type'}
}
sub protocol {
@_ == 1 or croak 'usage: $sock->protocol()';
my($sock) = @_;
+ ${*$sock}{'io_socket_proto'} = $sock->sockopt(Socket::SO_PROTOCOL)
+ if (!defined(${*$sock}{'io_socket_proto'}) && defined(eval{Socket::SO_PROTOCOL}));
${*$sock}{'io_socket_proto'};
}
diff --git a/dist/IO/t/cachepropagate-tcp.t b/dist/IO/t/cachepropagate-tcp.t
new file mode 100644
index 0000000..9c26b45
--- /dev/null
+++ b/dist/IO/t/cachepropagate-tcp.t
@@ -0,0 +1,51 @@
+#!/usr/bin/perl
+
+use warnings;
+use strict;
+
+use IO::Socket;
+use IO::Socket::INET;
+use Socket;
+use Test::More;
+
+plan tests => 8;
+
+my $listener = IO::Socket::INET->new(Listen => 1,
+ LocalAddr => '127.0.0.1',
+ Proto => 'tcp');
+ok(defined($listener), 'socket created');
+
+my $port = $listener->sockport();
+
+my $p = $listener->protocol();
+ok(defined($p), 'protocol defined');
+my $d = $listener->sockdomain();
+ok(defined($d), 'domain defined');
+my $s = $listener->socktype();
+ok(defined($s), 'type defined');
+
+my $cpid = fork();
+if (0 == $cpid) {
+ # the child:
+ sleep(1);
+ my $connector = IO::Socket::INET->new(PeerAddr => '127.0.0.1',
+ PeerPort => $port,
+ Proto => 'tcp');
+ exit(0);
+} else {;
+ ok(defined($cpid), 'spawned a child');
+}
+
+my $new = $listener->accept();
+
+is($new->sockdomain(), $d, 'domain match');
+SKIP: {
+ skip "no Socket::SO_PROTOCOL", 1 if !defined(eval { Socket::SO_PROTOCOL });
+ is($new->protocol(), $p, 'protocol match');
+}
+SKIP: {
+ skip "no Socket::SO_TYPE", 1 if !defined(eval { Socket::SO_TYPE });
+ is($new->socktype(), $s, 'type match');
+}
+
+wait();
diff --git a/dist/IO/t/cachepropagate-udp.t b/dist/IO/t/cachepropagate-udp.t
new file mode 100644
index 0000000..91cff37
--- /dev/null
+++ b/dist/IO/t/cachepropagate-udp.t
@@ -0,0 +1,34 @@
+#!/usr/bin/perl
+
+use warnings;
+use strict;
+
+use IO::Socket;
+use IO::Socket::INET;
+use Socket;
+use Test::More;
+
+plan tests => 7;
+
+my $listener = IO::Socket::INET->new(LocalAddr => '127.0.0.1',
+ Proto => 'udp');
+ok(defined($listener), 'socket created');
+
+my $p = $listener->protocol();
+ok(defined($p), 'protocol defined');
+my $d = $listener->sockdomain();
+ok(defined($d), 'domain defined');
+my $s = $listener->socktype();
+ok(defined($s), 'type defined');
+
+my $new = IO::Socket::INET->new_from_fd($listener->fileno(), 'r+');
+
+is($new->sockdomain(), $d, 'domain match');
+SKIP: {
+ skip "no Socket::SO_PROTOCOL", 1 if !defined(eval { Socket::SO_PROTOCOL });
+ is($new->protocol(), $p, 'protocol match');
+}
+SKIP: {
+ skip "no Socket::SO_TYPE", 1 if !defined(eval { Socket::SO_TYPE });
+ is($new->socktype(), $s, 'type match');
+}
diff --git a/dist/IO/t/cachepropagate-unix.t b/dist/IO/t/cachepropagate-unix.t
new file mode 100644
index 0000000..375f28a
--- /dev/null
+++ b/dist/IO/t/cachepropagate-unix.t
@@ -0,0 +1,83 @@
+#!/usr/bin/perl
+
+use warnings;
+use strict;
+
+use File::Temp qw(tempdir);
+use File::Spec::Functions;
+use IO::Socket;
+use IO::Socket::UNIX;
+use Socket;
+use Test::More;
+
+plan tests => 15;
+
+SKIP: {
+ skip "UNIX domain sockets not implemented on $^O", 15 if ($^O =~ m/^(?:qnx|nto|vos|MSWin32)$/);
+
+ my $socketpath = catfile(tempdir( CLEANUP => 1 ), 'testsock');
+
+ # start testing stream sockets:
+
+ my $listener = IO::Socket::UNIX->new(Type => SOCK_STREAM,
+ Listen => 1,
+ Local => $socketpath);
+ ok(defined($listener), 'stream socket created');
+
+ my $p = $listener->protocol();
+ ok(defined($p), 'protocol defined');
+ my $d = $listener->sockdomain();
+ ok(defined($d), 'domain defined');
+ my $s = $listener->socktype();
+ ok(defined($s), 'type defined');
+
+ my $cpid = fork();
+ if (0 == $cpid) {
+ # the child:
+ sleep(1);
+ my $connector = IO::Socket::UNIX->new(Peer => $socketpath);
+ exit(0);
+ } else {
+ ok(defined($cpid), 'spawned a child');
+ }
+
+ my $new = $listener->accept();
+
+ is($new->sockdomain(), $d, 'domain match');
+ SKIP: {
+ skip "no Socket::SO_PROTOCOL", 1 if !defined(eval { Socket::SO_PROTOCOL });
+ is($new->protocol(), $p, 'protocol match');
+ }
+ SKIP: {
+ skip "no Socket::SO_TYPE", 1 if !defined(eval { Socket::SO_TYPE });
+ is($new->socktype(), $s, 'type match');
+ }
+
+ unlink($socketpath);
+ wait();
+
+ # now test datagram sockets:
+ $listener = IO::Socket::UNIX->new(Type => SOCK_DGRAM,
+ Local => $socketpath);
+ ok(defined($listener), 'datagram socket created');
+
+ $p = $listener->protocol();
+ ok(defined($p), 'protocol defined');
+ $d = $listener->sockdomain();
+ ok(defined($d), 'domain defined');
+ $s = $listener->socktype();
+ ok(defined($s), 'type defined');
+
+ $new = IO::Socket::UNIX->new_from_fd($listener->fileno(), 'r+');
+
+ is($new->sockdomain(), $d, 'domain match');
+ SKIP: {
+ skip "no Socket::SO_PROTOCOL", 1 if !defined(eval { Socket::SO_PROTOCOL });
+ is($new->protocol(), $p, 'protocol match');
+ }
+ SKIP: {
+ skip "no Socket::SO_TYPE", 1 if !defined(eval { Socket::SO_TYPE });
+ is($new->socktype(), $s, 'type match');
+ }
+ unlink($socketpath);
+ }
|
From @rjbsThis needs to get done ASAP if it's going to go in. I'd like to get RC0 cut -- |
From @tonycozOn Tue, May 08, 2012 at 10:34:05AM +0100, Nicholas Clark wrote:
The existing tests use a mix of C< print "ok 1\n" >, Test and test.pl. Either a dependency on perl 5.006002 or on Test::More would need to be Tony |
From @jmdhOn Tue, May 08, 2012 at 10:09:12PM -0700, Tony Cook via RT wrote:
Daniel's updated patch (v5) does not seem to contain a mix, unless my -- |
From @jmdhOn Tue, May 08, 2012 at 08:09:43PM -0700, Ricardo Signes via RT wrote:
As I see it the next thing is to smoke the updated patch from I probably won't be able to get this smoking today, and possibly not -- |
From @jmdhOn Tue, May 08, 2012 at 08:09:43PM -0700, Ricardo Signes via RT wrote:
As discussed, an updated set of changes is now in smoke-me/cpan61577-dom. -- |
From @cpansproutOn Wed May 09 11:50:33 2012, dom wrote:
rjbs has applied this to blead as commits 271d04e, b690361 and -- Father Chrysostomos |
|
@cpansprout - Status changed from 'open' to 'resolved' |
From @jmdhOn Thu, May 10, 2012 at 12:44:25PM -0700, Father Chrysostomos via RT wrote:
There are still some new test failures on some platforms relating to The tests are currently known to be failing on HP-UX and AIX: http://perl5.test-smoke.org/report/471 Dominic. -- |
|
@jmdh - Status changed from 'resolved' to 'open' |
From @jmdhOn Mon, May 14, 2012 at 07:04:51PM +0100, Dominic Hargreaves wrote:
Now reverted in the release-5.16 branch by commit TonyC also wondered whether this bug could be redefined as: '$sock->protocol is unreliable and should be documented that way'. -- |
From @tonycozOn Mon, May 14, 2012 at 08:58:07PM +0100, Dominic Hargreaves wrote:
I suspect more cases could be handled by copying the relevant That might be a performance regression though. Tony |
From dkg@fifthhorseman.netOn 05/14/2012 08:35 PM, Tony Cook via RT wrote:
If i'm understanding correctly, this reversion seems like a shame to me.
It's a little late for that -- i believe these socket metadata functions
As discussed in the CPAN ticket, That approach also doesn't cover Regards, --dkg |
From dkg@fifthhorseman.netOn 05/14/2012 02:05 PM, Dominic Hargreaves via RT wrote:
i'm not sure whether this means that the proposed fixes to IO/Socket.pm
AIX: cachepropagate-unix.t non-zero exit status 29 -- it's not clear to
HP-UX: "domain match" test fails, which means that sockdomain on an
These two are both HP-UX failing in the same way as 470, above, fwict. I have neither HP-UX nor AIX platforms to test these myself. --dkg |
From @LeontOn Tue, May 15, 2012 at 6:46 AM, Daniel Kahn Gillmor
IO is dual-lifed, you can easily upgrade it to the fixed version as Leon |
From dkg@fifthhorseman.netOn Tue May 15 04:48:52 2012, dkg wrote:
I was graciously offered access to an HP-UX B.11.23 machine for testing. 0 dkg@debian:~$ perl ./test_sockaddr_family.pl 0 dkg@hpux:~$ perl ./test_sockaddr_family.pl This appears to be the case with perl 5.8.8 (native to this version of I'm still unclear on why this caused problems with the cachepropagate I was unable replicate this behavior in C, however. possibly because i |
From dkg@fifthhorseman.net#define _XOPEN_SOURCE_EXTENDED
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <string.h>
#include <sys/wait.h>
#include <stdio.h>
#include <errno.h>
#define SOCKPATH "./socket"
void report_family(int sock, const char * state) {
int family = 0;
socklen_t retlen;
struct sockaddr_un c;
retlen = sizeof(c);
if (0 != getsockname(sock, (struct sockaddr*)(&c), &retlen)) {
printf("failed to getsockname on socket %s: [%d] (%s)\n", state, errno, strerror(errno));
} else {
family = ((struct sockaddr*)(&c))->sa_family;
printf("getsockname shows family %d (%s)\n", family, state);
}
}
int main() {
int sock, accepted;
struct sockaddr_un a, b, c;
socklen_t retlen;
pid_t child_pid;
char buf[4];
int status;
int family = 0;
memset(buf, 0, sizeof(buf));
unlink(SOCKPATH);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
a.sun_family = AF_UNIX;
strcpy(a.sun_path, SOCKPATH);
bind(sock, (struct sockaddr*)(&a), sizeof(a));
report_family(sock, "listener, early");
child_pid = fork();
if (child_pid) {
listen(sock, 2);
retlen = sizeof(b);
accepted = accept(sock, (struct sockaddr*)(&b), &retlen);
report_family(accepted, "accepted, post-accept");
close(sock);
report_family(accepted, "accepted, after-parent-closed");
read(accepted, buf, sizeof(buf));
report_family(accepted, "accepted, after-read");
printf("got '%s'\n", buf);
close(accepted);
waitpid(child_pid, &status, 0);
unlink(SOCKPATH);
} else {
close(sock);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
sleep(1);
connect(sock, (struct sockaddr*)(&a), sizeof(a));
write(sock, "fo", 2);
/*
sleep(1);
*/
close(sock);
}
return 0;
} |
From @cpansproutWas this resolved by these commits? 93a5d7b -- Father Chrysostomos |
From @jmdhOn Thu, Jul 05, 2012 at 05:13:21PM -0700, Tony Cook via RT wrote:
Thanks very much for following up on this so far Tony - it's good Cheers, -- |
From @cpansproutOn Thu Jul 05 17:13:21 2012, tonyc wrote:
Have you had a chance to do that? -- Father Chrysostomos |
From @tonycozOn Wed Sep 19 22:12:56 2012, sprout wrote:
I can't reproduce the failure on HP-UX with the older commit, so I'm Tony |
|
@tonycoz - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#112736 (status was 'resolved')
Searchable as RT112736$
The text was updated successfully, but these errors were encountered: