This queue is for tickets about the IO-Socket-IP CPAN distribution.

Report information
The Basics
Id:
95983
Status:
resolved
Priority:
Low/Low
Queue:

People
Owner:
Nobody in particular
Requestors:
sullr [...] cpan.org
Cc:
CARNIL [...] cpan.org
gregoa [...] cpan.org
AdminCc:

BugTracker
Severity:
(no value)
Broken in:
(no value)
Fixed in:
0.31



Subject: IO::Socket::IP should not call $self->connect when retrying connect
Hi, currently IO::Socket::IP implements multi-homing by calling $self->connect for all possible combinations of source and destination until it succeeds. But, if IO::Socket::IP is used as a base class for another module, like done with IO::Socket::SSL, this will not result in retrying until it gets an successful connection to an IP, but until the connect of the derived class returns success. In the case of IO::Socket::SSL successful connect will not only mean connection to the IP address, but also SSL handshake, verification of the certificate etc. And in my opinion it should not be the behavior of the IP layer to retry a connection if the SSL layer failed because of certificate validation errors. So I would suggest, that any $self->connect should be changed to $self->IO::Socket::IP::connect, so that the class deals only about successful connection on the IP level. Regards, Steffen
On Mon May 26 15:49:16 2014, SULLR wrote:
Show quoted text
> So I would suggest, that any $self->connect should be changed to > $self->IO::Socket::IP::connect, so that the class deals only about > successful connection on the IP level.
Yeees. I think that sounds a reasonable idea. As an aside: I think it's a shame in general there isn't a neater way to notate that kind of thing. It would be nice if you could subclass an object and provide a new definition for an existing method, but which only applies to "outsiders", and existing calls in base classes still see their base definition. Anyway. Patch attached; will be in next release. -- Paul Evans
Subject: rt95983.patch
=== modified file 'lib/IO/Socket/IP.pm' --- lib/IO/Socket/IP.pm 2014-02-24 16:08:13 +0000 +++ lib/IO/Socket/IP.pm 2014-05-31 17:23:48 +0000 @@ -601,7 +601,7 @@ } if( defined( my $addr = $info->{peeraddr} ) ) { - if( $self->connect( $addr ) ) { + if( $self->IO::Socket::IP::connect( $addr ) ) { $! = 0; return 1; } @@ -651,7 +651,7 @@ # (still in progress). This even works on MSWin32. my $addr = ${*$self}{io_socket_ip_infos}[${*$self}{io_socket_ip_idx}]{peeraddr}; - if( $self->connect( $addr ) or $! == EISCONN ) { + if( $self->IO::Socket::IP::connect( $addr ) or $! == EISCONN ) { delete ${*$self}{io_socket_ip_connect_in_progress}; $! = 0; return 1;
I may have to revert this one because it's causing bad knock-on effects with IO::Socket::SSL: https://rt.cpan.org/Ticket/Display.html?id=97050 Basically: the very thing it was supposed to fix, it has broken. Meh. -- Paul Evans
Am Di 08. Jul 2014, 06:35:58, PEVANS schrieb:
Show quoted text
> I may have to revert this one because it's causing bad knock-on > effects with IO::Socket::SSL: > > https://rt.cpan.org/Ticket/Display.html?id=97050 > > Basically: the very thing it was supposed to fix, it has broken. Meh.
Yes, unfortunately it wasn't as easy as I thought because the calling scheme inside IO::Socket::* (i.e. new -> configure -> connect ) isn't that simple if you have a class hierarchy and also try to implement multi-homing :( But I think I have a working patch (included, against 0.30). The basic idea of the patch is that one has to distinguish between an error at the transport layer which can be solved with IP based multi-homing and an error at the application layer. One could expect the system error to be reflected inside $!, while an application error will probably not set $! (e.g. IO::Socket::SSL sets an $SSL_ERROR variable). So if connect fails, but $! is not set, one can assume error at the application layer and stop trying to fix it with IP based multi-homing. The other difference in the patch is to change $self->IO::Socket::IP::connect($addr) to CORE::connect($self,$addr), because if you have a look at the connect function it simple calls CORE::connect if an $addr argument is given. It was already right to not use $self->connect in this place, it was only a problem if called from inside the new - configure - connect chain. With this patch the tests inside IO::Socket::IP pass and also the tests of IO::Socket::SSL. Regards, Steffen
Subject: IP.pm.patch
diff --git a/lib/IO/Socket/IP.pm b/lib/IO/Socket/IP.pm index 1911145..16eb7c8 100644 --- a/lib/IO/Socket/IP.pm +++ b/lib/IO/Socket/IP.pm @@ -601,7 +601,7 @@ sub setup } if( defined( my $addr = $info->{peeraddr} ) ) { - if( $self->IO::Socket::IP::connect( $addr ) ) { + if( $self->connect( $addr ) ) { $! = 0; return 1; } @@ -611,6 +611,13 @@ sub setup return 0; } + # If connect failed but we have no system error there must be an error + # at the application layer, like a bad certificate with + # IO::Socket::SSL. + # In this case don't continue IP based multi-homing because the problem + # cannot be solved at the IP layer. + return 0 if ! $!; + ${*$self}{io_socket_ip_errors}[0] = $!; next; } @@ -651,7 +658,7 @@ sub connect # (still in progress). This even works on MSWin32. my $addr = ${*$self}{io_socket_ip_infos}[${*$self}{io_socket_ip_idx}]{peeraddr}; - if( $self->IO::Socket::IP::connect( $addr ) or $! == EISCONN ) { + if( CORE::connect( $self, $addr ) or $! == EISCONN ) { delete ${*$self}{io_socket_ip_connect_in_progress}; $! = 0; return 1;
Fixed by effectively reverting the original change :/ -- Paul Evans


This service runs on Request Tracker, is sponsored by The Perl Foundation, and maintained by Best Practical Solutions.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.