Skip Menu |

This queue is for tickets about the Net-Async-HTTP CPAN distribution.

Report information
The Basics
Id: 101459
Status: open
Priority: 0/
Queue: Net-Async-HTTP

People
Owner: Nobody in particular
Requestors: TEAM [...] cpan.org
Cc:
AdminCc:

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



Subject: Unhandled exception with bad chunked transfer encoding
(I'm tagging this as 'wishlist' since I'm not sure how far you want to go in protecting against bad responses from an HTTP server) If the server responds with something other than useful chunk data, this can cause a loop-level exception.
Subject: nahttp-badchunks.t
use strict; use warnings; use Test::More; use Test::Fatal; use Test::Warnings 0.005 ':all'; use IO::Async::Loop; use IO::Socket::IP; use Net::Async::HTTP; my $loop = IO::Async::Loop->new; my $s; my ($listener) = $loop->listen( addr => { family => "inet", socktype => "stream", ip => '127.0.0.1', port => 0, }, on_accept => sub { my ($sock) = @_; my $remote = join(':', $sock->peerhost, $sock->peerport); note "Accept from $remote"; use Fcntl qw(F_GETFL F_SETFL O_NONBLOCK); my $flags = fcntl($sock, F_GETFL, 0) or die "Can't get flags for the socket: $!\n"; is($flags & O_NONBLOCK, O_NONBLOCK, 'nonblocking flag is set on accepted socket'); my $stream = IO::Async::Stream->new( handle => $sock, on_read => sub { my ($stream, $buffref, $eof) = @_; while($$buffref =~ s{^(.*)\x0D\x0A}{}) { my $line = $1; # note "HTTP request line: $line"; if(!length $line) { $stream->write( join "\x0D\x0A", "HTTP/1.1 404 OK", "Host: me", "Transfer-Encoding: chunked", "Connection: close", "", "0005", # Partial chunk then CRLF "hell", "o", "" ) } } 0 } ); $loop->add($stream); } )->get; my $sock = $listener->read_handle; my $hostport = join(':', $sock->sockhost, $sock->sockport); note "Server listening on $hostport"; $loop->add( my $ua = Net::Async::HTTP->new( fail_on_error => 1, max_connections_per_host => 0, ) ); is(warnings { is(exception { $ua->GET( 'http://' . $hostport, timeout => 2, )->else(sub { my ($failure) = @_; like($failure, qr/chunk/i, "had some sort of error mentioning chunks"); # i.e. don't raise an exception Future->wrap })->get }, undef, 'no exception'); }, 0, 'no warnings'); done_testing;
On Sun Jan 11 12:13:06 2015, TEAM wrote: Show quoted text
> (I'm tagging this as 'wishlist' since I'm not sure how far you want to > go in protecting against bad responses from an HTTP server) > > If the server responds with something other than useful chunk data, > this can cause a loop-level exception.
I think it's reasonable for a client to attempt to gracefully handle bad responses from a server (or vice versa). This is somewhat different to a client module trying to defend itself from bad data (types/etc..) from the controlling application - this latter, at least in a dynamic language like perl, is quite a never-ending minefield. This is a legitimately-valid bug; I'll see what I can do. -- Paul Evans
Attached patch handles the "undef value in string" warning. As to the "spurious read" failure, that's a more general NaHTTP design question, which I'll move to a new bug (RT105836). -- Paul Evans
Subject: rt101459.patch
=== modified file 'lib/Net/Async/HTTP/Connection.pm' --- lib/Net/Async/HTTP/Connection.pm 2015-07-13 13:37:56 +0000 +++ lib/Net/Async/HTTP/Connection.pm 2015-07-13 14:00:29 +0000 @@ -1,7 +1,7 @@ # You may distribute under the terms of either the GNU General Public License # or the Artistic License (the same terms as Perl itself) # -# (C) Paul Evans, 2008-2014 -- leonerd@leonerd.org.uk +# (C) Paul Evans, 2008-2015 -- leonerd@leonerd.org.uk package Net::Async::HTTP::Connection; @@ -525,7 +525,6 @@ if( defined $chunk_length and length( $$buffref ) >= $chunk_length + 2 ) { # Chunk body my $chunk = substr( $$buffref, 0, $chunk_length, "" ); - undef $chunk_length; unless( $$buffref =~ s/^$CRLF// ) { $self->debug_printf( "ERROR chunk without CRLF" ); @@ -533,6 +532,8 @@ $self->close; } + undef $chunk_length; + return $on_more->( $chunk ); }