Blocking behaviour on Net::HTTP::Methods my_readline()

[lgastevens]

Hi
I'd like to report what I think might be a problem with Net::HTTP::Methods. Or for another pair of eyes to check my conclusions!
I'm using HTTP::Async to write an OpenAPI client.
The client needs to be non-blocking when it fetches the HTTPS API responses, hence the use of HTTP::Async.
For most requests this works well, but on very large API responses HTTP::Async's $async->next_response becomes blocking, which should not happen.
It seems that HTTP::Async is not maintained right now, which is unfortunate, but as far as I can see it is correctly using Net::HTTPS::NB; it uses IO::Select's can_read() before calling Net::HTTPS::NB's read_response_headers(), which ends up calling the same function in Net::HTTP::Methods and there the code blocks in my_readline().
Initially I made a dirty fix in Methods.pm function my_readline():

                # consume all incoming bytes
                $self->blocking(0); # <===================== My hack!
                my $bytes_read = $self->sysread($_, 1024, length);
                if(defined $bytes_read) {
                    $new_bytes += $bytes_read;
                }
                elsif($!{EINTR} || $!{EAGAIN} || $!{EWOULDBLOCK}) {
                    return; # <============================ My hack!
                    redo READ;
                }

• I made the socket non-blocking before the sysread: $self->blocking(0);
• I replaced "redo READ;" with "return;"

Then my application code worked as expected and HTTP::Async's $async->next_response no longer blocked on large API responses.
But the same code written with LWP::UserAgent then was broken; not good!

For the first part, I've come to the conclusion that HTTP::Async should really be making the socket non-blocking itself, and I will try and raise an issue there. Though fortunately I can poke through into the socket from my application code and make that happen without making any changes in HTTP::Async.
But I'm still stuck in that I need a change in Net::HTTP::Methods my_readline() to return immediately rather than "redo READ;".

For now I've made this change:

                # consume all incoming bytes
                my $bytes_read = $self->sysread($_, 1024, length);
                if(defined $bytes_read) {
                    $new_bytes += $bytes_read;
                }
                elsif($!{EINTR} || $!{EAGAIN} || $!{EWOULDBLOCK}) {
                    return if ref($self) =~ /Net::HTTPS?::NB/; # Modified by Ludovico Stevens; need to return for non-blocking use
                    redo READ;
                }

I'm not that familiar with the wider Net::HTTP modules and whether this is the right fix.
But I would like to see a more general fix if possible rolled into Net::HTTP's mainline, rather than myself having to go hack the Methods.pm file every time I port my application code.

Below is the test script which reproduces the problem at hand. To verify the non-blocking behaviour this script simply prints dots while it waits for a response.

use strict;
use warnings;
use HTTP::Async;
use HTTP::Request;
use IO::Socket::SSL;
use Cpanel::JSON::XS;

my $host = "10.8.1.8";

sub httpRequest { # Build the OpenAPI HTTP request object
	my ($host, $port, $protocol, $method, $path, $token, $body) = @_;
	my $url = sprintf("%s://%s:%s/rest/openapi%s",
		$protocol,
		$host,
		$port,
		$path
	);
	my @header = ('Content-Type' => 'application/json; charset=UTF-8', 'Accept' => 'application/json');
	push @header, 'X-Auth-Token' => $token if $token;
	my $encoded_body = defined $body ? encode_json( $body ) : undef;
	return HTTP::Request->new($method, $url, \@header, $encoded_body);
}

sub waitForOpenAPIResponse { # Waits for and returns the next HTTP::Async response, printing progress dots while waiting
	my $async = shift;
	my $count = 0;
	my $response;
	while ( $async->not_empty ) {
		if ( $response = $async->next_response ) {
			print "\nGot: ", $response->code, " ", $response->message, " ", $response->header("Server"),"\n";
			print "Size of response = ", length($response->content), "\n";
		}
		elsif (++$count > 1000) {
			$| = 1;
			print ".";
			$count = 0; 
		}
	}
	if (!$response->is_success) { # HTTP Request failed
		die("HTTP Request failed: ", $response->status_line);
	}
	return $response;
}

# Create HTTP::Async object
my $httpAsync = HTTP::Async->new(
	ssl_options => { SSL_verify_mode => IO::Socket::SSL::SSL_VERIFY_NONE }
);

# Send login request to obtain token
print "OpenAPI login ";
my $request = httpRequest(
	$host, 9443, 'https', 'POST', '/auth/token', undef,
	{username => "rwa", password => "rwa"}
);
$httpAsync->add( $request );
my $response = waitForOpenAPIResponse($httpAsync);

# Get token from response
my $token = decode_json($response->content)->{token};

print "OpenAPI pulling /v0/configuration ";
$request = httpRequest(
	$host, 9443, 'https', 'GET', '/v0/configuration', $token
);
$httpAsync->add( $request );
($httpAsync->_io_select->handles)[0]->blocking(0);
$response = waitForOpenAPIResponse($httpAsync);

The last line but one is me poking the socket to be non-blocking

With the unmodified my_readline() in Net::HTTP::Methods I get this:

C:\Users\lstevens\Scripts\acli\working-dir>http-async-test.pl
OpenAPI login
Got: 200 OK Werkzeug/2.1.2 Python/3.10.13
Size of response = 272
OpenAPI pulling /v0/configuration .<LOOONG WAIT!>........
Got: 200 OK Werkzeug/2.1.2 Python/3.10.13
Size of response = 365603

I inserted "<LOONG WAIT!>"; after the 1st dot basically HTTP::Async's $async->next_response blocks immediately and only returns once almost the entire API response is received.

While with my change in my_readline(), I get the desired behaviour:

C:\Users\lstevens\Scripts\acli\working-dir>http-async-test.pl
OpenAPI login .
Got: 200 OK Werkzeug/2.1.2 Python/3.10.13
Size of response = 272
OpenAPI pulling /v0/configuration .............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Got: 200 OK Werkzeug/2.1.2 Python/3.10.13
Size of response = 365603

And this is the same script using LWP::UserAgent, to make sure I have not broken anything for non users of HTTP::Async :

use strict;
use warnings;
use LWP::UserAgent ();
use HTTP::Request;
use IO::Socket::SSL;
use Cpanel::JSON::XS;

my $host = "10.8.1.8";

sub httpRequest { # Build the OpenAPI HTTP request object
	my ($host, $port, $protocol, $method, $path, $token, $body) = @_;
	my $url = sprintf("%s://%s:%s/rest/openapi%s",
		$protocol,
		$host,
		$port,
		$path
	);
	my @header = ('Content-Type' => 'application/json; charset=UTF-8', 'Accept' => 'application/json');
	push @header, 'X-Auth-Token' => $token if $token;
	my $encoded_body = defined $body ? encode_json( $body ) : undef;
	return HTTP::Request->new($method, $url, \@header, $encoded_body);
}

# Create LWP::UserAgent object
my $ua = LWP::UserAgent->new(
	ssl_opts => { SSL_verify_mode => IO::Socket::SSL::SSL_VERIFY_NONE, verify_hostname => 0 }
);

# Send login request to obtain token
print "OpenAPI login\n";
my $request = httpRequest(
	$host, 9443, 'https', 'POST', '/auth/token', undef,
	{username => "rwa", password => "rwa"}
);
my $response = $ua->request( $request );
print "Got: ", $response->code, " ", $response->message, " ", $response->header("Server"),"\n";
print "Size of response = ", length($response->content), "\n";
if (!$response->is_success) { # HTTP Request failed
	die("HTTP Request failed: ", $response->status_line);
}

# Get token from response
my $token = decode_json($response->content)->{token};

print "OpenAPI pulling /v0/configuration\n";
$request = httpRequest(
	$host, 9443, 'https', 'GET', '/v0/configuration', $token
);
$response = $ua->request( $request );
print "Got: ", $response->code, " ", $response->message, " ", $response->header("Server"),"\n";
print "Size of response = ", length($response->content), "\n";
if (!$response->is_success) { # HTTP Request failed
	die("HTTP Request failed: ", $response->status_line);
}

Which works correctly with or without my changes in Net::HTTP::Methods my_readline()

C:\Users\lstevens\Scripts\acli\working-dir>lwp-useragent-test.pl
OpenAPI login
Got: 200 OK Werkzeug/2.1.2 Python/3.10.13
Size of response = 272
OpenAPI pulling /v0/configuration
Got: 200 OK Werkzeug/2.1.2 Python/3.10.13
Size of response = 365603

Thanks
Ludovico Stevens