Skip Menu |
 

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

Report information
The Basics
Id: 91989
Status: resolved
Priority: 0/
Queue: IO-Socket-IP

People
Owner: Nobody in particular
Requestors: nick [...] ccl4.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: 0.25



Subject: Net::HTTP fails tests if changed to use IO::Socket::IP
Download (untitled) / with headers
text/plain 1.8k
IO::Socket::IP isn't currently a substitute for IO::Socket::INET in Net::HTTP. If I change Net::HTTP to use it: --- lib/Net/HTTP.pm~ 2013-03-10 23:35:20.000000000 +0100 +++ lib/Net/HTTP.pm 2014-01-09 17:34:17.000000000 +0100 @@ -5,8 +5,8 @@ $VERSION = "6.06"; unless ($SOCKET_CLASS) { - eval { require IO::Socket::INET } || require IO::Socket; - $SOCKET_CLASS = "IO::Socket::INET"; + eval { require IO::Socket::IP; } || require IO::Socket; + $SOCKET_CLASS = "IO::Socket::IP"; } require Net::HTTP::Methods; require Carp; Then tests fail. The failure can be reduced to this $ cat nb.pl #!perl -w use v5.10; require Net::HTTP; # bind a random TCP port for testing my %lopts = ( LocalAddr => "127.0.0.1", LocalPort => 0, Proto => "tcp", ReuseAddr => 1, Listen => 1024 ); my $srv = IO::Socket::INET->new(%lopts); my $host = $srv->sockhost . ':' . $srv->sockport; my $nb = Net::HTTP->new(Host => $host, Blocking => 0); say ref($nb); __END__ which generates no output, because $nb is not defined. (It should be Net::HTTP) The problem seems to be that Net::HTTP is a subclass of the socket module, and has these methods: sub new { my $class = shift; Carp::croak("No Host option provided") unless @_; $class->SUPER::new(@_); } sub configure { my($self, $cnf) = @_; $self->http_configure($cnf); } sub http_connect { my($self, $cnf) = @_; $self->SUPER::configure($cnf); } and relies its configure being called by the SUPER::new. Which is the case for IO::Socket::INET, but isn't for IO::Socket::IP. Given that IO::Socket::IP has this: # IO::Socket may call this one; neaten up the arguments from IO::Socket::INET # before calling our real _configure method sub configure { I assume that this is a bug in emulation of IO::Socket::INET, as it's not documented as an incompatibility. IO::Socket::IP 0.24 with blead, on both OS X and Linux. Nicholas Clark
Download (untitled) / with headers
text/plain 399b
I think configure() is a red herring here, because the code completely passes in the (normal) blocking case; it's only the non-blocking case it doesn't. I expect the trouble here is that Net::HTTP is creating a nonblocking listening socket, and then just blindly presuming this has already succeeded, so goes on with its life. It's going to have to wait for this to actually exist. -- Paul Evans
Download (untitled) / with headers
text/plain 586b
On Thu Jan 09 13:36:39 2014, PEVANS wrote: Show quoted text
> I expect the trouble here is that Net::HTTP is creating a nonblocking > listening socket, and then just blindly presuming this has already > succeeded, so goes on with its life. It's going to have to wait for > this to actually exist.
Actually turns out to be more subtle: IO::Socket::INET->new( PeerHost => "ip:port", PeerPort => "port" ) works, but isn't documented as doing so. IO::Socket::IP didn't like the port specified twice. I've now made it do so. Will be fixed in next version, whenever PAUSE comes back... -- Paul Evans
Subject: rt-91989.patch
Download rt-91989.patch
text/x-diff 3k
=== modified file 'lib/IO/Socket/IP.pm' --- lib/IO/Socket/IP.pm 2013-09-19 13:18:19 +0000 +++ lib/IO/Socket/IP.pm 2014-01-10 20:27:00 +0000 @@ -354,9 +354,12 @@ my $host = $type . 'Host'; my $service = $type . 'Service'; - if (exists $arg->{$host} && !exists $arg->{$service}) { - defined $arg->{$host} or next; + if( defined $arg->{$host} ) { ( $arg->{$host}, my $s ) = $self->split_addr( $arg->{$host} ); + + croak "$service is inconsistent with service specified by $host" + if defined $arg->{$service} and defined $s and $arg->{$service} ne $s; + $arg->{$service} = $s if defined $s; } } @@ -928,8 +931,7 @@ passed in a single string rather than as two separate arguments. If either C<LocalHost> or C<PeerHost> (or their C<...Addr> synonyms) have any -of the following special forms, and C<LocalService> or C<PeerService> (or -their C<...Port> synonyms) are absent, special parsing is applied. +of the following special forms then special parsing is applied. The value of the C<...Host> argument will be split to give both the hostname and port (or service name): @@ -950,6 +952,10 @@ In this case, the name (C<http>) will be tried first, but if the resolver does not understand it then the port number (C<80>) will be used instead. +If the C<...Host> argument is in this special form and the corresponding +C<...Service> or C<...Port> argument is also defined, the two must give the +same value. If they do not, an exception is thrown. + =head2 ( $host, $port ) = IO::Socket::IP->split_addr( $addr ) Utility method that provides the parsing functionality described above. @@ -1048,6 +1054,15 @@ =item * +C<IO::Socket::INET> will attempt to parse a service/port spec from the +C<LocalAddr> or C<PeerAddr> argument and if found this will override the one +given by C<LocalPort> or C<PeerPort> even if that is different. + +C<IO::Socket::IP> will throw an exception if both exist but have different +values. + +=item * + The C<Timeout> constructor argument is currently not recognised. The behaviour enabled by C<MultiHomed> is in fact implemented by === modified file 't/10args.t' --- t/10args.t 2013-04-12 19:17:46 +0000 +++ t/10args.t 2014-01-10 20:27:00 +0000 @@ -47,6 +47,10 @@ [ [ PeerHost => '[::1]' ], { PeerHost => '::1' } ], [ [ LocalHost => '[::1]:80' ], { LocalHost => '::1', LocalService => '80' } ], [ [ LocalHost => undef ], { LocalHost => undef } ], + + # IO::Socket::INET is happy to take port from the *Host argument even if a *Port argument + # exists + [ [ PeerHost => '127.0.0.1:80', PeerPort => '80' ], { PeerHost => '127.0.0.1', PeerService => '80' } ], ); is_deeply( [ IO::Socket::IP->split_addr( "hostname:http" ) ], @@ -83,4 +87,8 @@ arguments_is(@$_) for @tests; +ok( !eval { IO::Socket::IP->new( PeerHost => '127.0.0.1:1234', PeerPort => '5678' ); + 1 }, + 'Inconsistent port in PeerHost and PeerPort dies' ); + done_testing;
Download (untitled) / with headers
text/plain 562b
With this patch in place the Net::HTTP unit tests now pass: PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/apache-https.t .. skipped: Live tests disabled; pass --live-tests to Makefile.PL to enable t/apache.t ........ skipped: Live tests disabled; pass --live-tests to Makefile.PL to enable t/http-nb.t ....... ok t/http.t .......... ok All tests successful. Files=4, Tests=51, 1 wallclock secs ( 0.03 usr 0.01 sys + 0.09 cusr 0.00 csys = 0.13 CPU) Result: PASS -- Paul Evans
Released in 0.25 -- Paul Evans


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

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