Wrong "sysread" return code processing in Starman::Server::_read_headers about starman HOT 5 CLOSED

Thanks for the report. Can you supply a test case to reproduce this?

from starman.

While writing a test case i have realiazed that problem is not only in sysread, but also in strict CRLF checking.

Starting starman in debug mode:
export STARMAN_DEBUG=1; starman -e 'sub { my $env = shift; return [200 => ["Content-Type: text/html\n"], ["body"]] }'

2012/12/12-11:34:24 Starman::Server (type Net::Server::PreFork) starting! pid(3093)
Resolved []:5000 to [0.0.0.0]:5000, IPv4
Host [] resolved to IPv6 address [::] but IO::Socket::INET6->new fails: Address family not supported by protocol at /usr/local/perl-5.14.2/lib/site_perl/5.14.2/Net/Server/Proto.pm line 133.
Binding to TCP port 5000 on host 0.0.0.0 with IPv4
Setting gid to "30328 30328 496 501 30328 30489 30490"
Setting up serialization via flock
Beginning prefork (5 processes)
Starting "5" children
Child Preforked (3103)
[3103] Initializing the PSGI app
Parent ready for children.
Child Preforked (3104)
[3104] Initializing the PSGI app
Child Preforked (3107)
[3107] Initializing the PSGI app
Child Preforked (3106)
[3106] Initializing the PSGI app
Child Preforked (3105)
[3105] Initializing the PSGI app

Sending empty request:
perl -e 'use Socket; my $p = sockaddr_in(5000, inet_aton("127.0.0.1")); socket(SOCK, PF_INET, SOCK_STREAM, getprotobyname("tcp")); connect(SOCK, $p) or die $!; printf "Bytes sent: %d\n", send(SOCK, "", 0, $p);'

We got error:

2012/12/12-11:35:00 CONNECT TCP Peer: "[127.0.0.1]:45097" Local: "[127.0.0.1]:5000"
[3104] Read error:
[3104] Closing connection

I have expected "Bad request" in debug, but got only "Read error" with empty $!.
Moreover i see "Read error" in requests like this:
send(SOCK, "GET / HTTP/1.0\n\n", 0, $p)

Probably the true problem is not only in sysread's return value checking, but also in strict CRLF checking while parsing headers.
Here is a quote from HTTP 1.1 spec:

The line terminator for message-header fields is the sequence CRLF. However, we recommend that applications, when parsing such headers, recognize a single LF as a line terminator and ignore the leading CR.

In my case starman works as a backend-server, processing requests from nginx (working in reverse proxy mode).
Nginx (and other web servers, ex. apache), follows above recommendation and doesn't drop requests without leading CR.
In fact there is a huge number of different crawlers, robots, bots etc, that do not follow http spec and send only "\n". It would be great if starman could process both CRLF and LF line terminators.

Thanks.

from starman.

Starman is intended to be used in the backend behind frontends like nginx (like your case), so it's arguable whether it should support a broken client like such bots. It makes sense to not at least crash in such situations though.

from starman.

In perfect case it would be great to have starman supporting such recommendations, just because it's a HTTP server in the first place. This will make starman's behaviour more like an apache and nginx. Migration to starman will become more transparent particularly for complex and highly loaded projects where even LF-terminator does matter.

However $read checking after sysread is still incorrect and likely can be fixed in rather simple manner:

if ( !defined $read ) {
    die "Read error: $!\n";
}
elsif ( $read == 0 ) {
    last;
}

Thanks.

from starman.

The line ending issue has become a problem for us at MaxMind too. We are moving our web services our to Web::Machine on top of Starman. We have thousands of clients, and some of them are apparently sending requests with just NL as the line ending. We do have Starman behind haproxy, but that doesn't help. Apparently haproxy doesn't rewrite the entire request. I can't imagine we're the only ones wanting to use Starman behind haproxy.

I suspect this could be fixed fairly simply by looking for /\r?\n/ instead of /\r\n/. Would you take a patch for this?

from starman.