noxxi / p5-io-socket-ssl

IO::Socket::SSL Perl Module
36 stars 60 forks source link

readline() returns read data incorrectly in wantarray #1

Closed mytram closed 11 years ago

mytram commented 11 years ago

Hi

I ran into an issue with IO::Socket::SSL, and it seemed to be an issue in readline() when called in wantarray, e.g. @lines = <$ssl_socket>

If there are only no greater than 2**16 bytes to be read, this will try to sysread again. However, this second call of sysread will either block in blocking mode or return undef , instead of the data already read, if there is no more to read.

Below is the code snippet for quick ref.

while (1) {
    my $rv = $self->sysread($buf,2**16,length($buf));
    if ( ! defined $rv ) {
    next if $!{EINTR};
    return;
    } elsif ( ! $rv ) {
    last
    }
}

Many thanks and regards,

M

noxxi commented 11 years ago

sysread returns undef only on error, not on eof. An error of EINTR is considered temporally, all other errors are fatal and in this case readline should return undef. On eof $rv will be 0, not undef and the function will return the read bytes.

So I don't see any problems with this behavior.

mytram commented 11 years ago

Thanks for the quick response. And I looked deeper into it.

On a non-blocking handle, an EAGAIN or EWOULDBLOCK will be set to $! whilst one of the read functions returns undef. That was exactly what I encountered: readline on a non-blocking handle,and the first read read all the data, and the second read (actually by Net::SSLeay::read) returned undef setting $! to EAGAIN.

Perl's core readline on a non-blocking handle will return as soon as possible the underlying read operation returns, even though there is no $\ read. Having said that, using readline on a non-blocking handle assuming the line separator is always present in the same read as the data seems a bad idea.

Cheers,

noxxi commented 11 years ago

So I guess IO::Socket::SSL behaves the same as other IO:: stuff. This behavior was actually changed in 1.60 in response to RT#75910

mytram commented 11 years ago

I think IO::Socket::SSL::readline() called in list context should terminate the while loop when the $self->sysread returns undef with ($! == EAGAIN || $! = EWOULDBLOCK)

mytram commented 11 years ago

Looked at the diff between 1.59 and 1.60 just for the while(1) loop concerning readline in list context. I think

                            last if $! == EAGAIN || $! == EWOULDBLOCK

should be betwen "next if $!{EINTR}" and "return"

noxxi commented 11 years ago

Yes, I think this behavior makes sense. I've just released 1.831 which should change this behavior

mytram commented 11 years ago

Cool! Thanks a lot!

Also I have converted the r/readline.t test cases into t/readline_inet.t, which are the same test cases but using IO::Socket::INET to check that both versions of readline() behave the same. There are a suite of nonblocking test cases added in readline_inet.t as well. During which I found IO::Socket::SSL's readline() dealt with $\ = "" differently from what's described in perlvar. So I amended those test cases in readline_inet.t to make them pass.

Anyways, the new test file is here https://github.com/mytram/p5-io-socket-ssl/tree/master/t for your reference.

Cheers,