Bug #95983 for IO-Socket-IP: IO::Socket::IP should not call $self->connect when retrying connect

On Mon May 26 15:49:16 2014, SULLR wrote: Show quoted text
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

=== 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;

Am Di 08. Jul 2014, 06:35:58, PEVANS schrieb: Show quoted text
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

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;