Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

psgi.input being undef causes exceptions elsewhere #23

Open
hoytech opened this issue Dec 2, 2016 · 6 comments
Open

psgi.input being undef causes exceptions elsewhere #23

hoytech opened this issue Dec 2, 2016 · 6 comments

Comments

@hoytech
Copy link
Contributor

hoytech commented Dec 2, 2016

In the docs it says

Feersum currently passes undef for psgi.input when there is no body to avoid unnecessary work.

Which makes sense, except it causes HTTP::Entity::Parser to throw an exception:

Can't call method "seek" on an undefined value at /usr/local/share/perl/5.22.1/HTTP/Entity/Parser.pm line 71.

I opened an issue there too: kazeburo/HTTP-Entity-Parser#6

I'm not exactly clear on the PSGI spec's take on this, but it seems to say it must be an IO::Handle-like object?

Thanks,

Doug

@hoytech
Copy link
Contributor Author

hoytech commented Dec 3, 2016

Just to be clear, I am seeing this when I issue a POST request with an empty body (admittedly somewhat of an edge case.

Here is a test-case:

use Feersum;
use AnyEvent;
use Plack::Request;

my $socket = IO::Socket::INET->new(Listen => 5, Proto => 'tcp', LocalPort => 9999, ReuseAddr => 1, Blocking => 0) || die "unable to listen: $!";

my $f = Feersum->endjinn;
$f->use_socket($socket);

$f->psgi_request_handler(sub {
    my $env = shift;
    my $req = Plack::Request->new($env);
    $req->parameters;
    return [200, [], []];
});

AE::cv->recv;

When it's running, hit it with curl:

curl http://localhost:9999 -d ''

And you'll see the error:

(in cleanup) DIED: Can't call method "seek" on an undefined value at /usr/local/share/perl/5.22.1/HTTP/Entity/Parser.pm line 71.

@audreyt
Copy link
Collaborator

audreyt commented Dec 3, 2016

Thanks for the report. Try commenting out lines 1579 and 1582 in Feersum.xs and see if it works for you?

@hoytech
Copy link
Contributor Author

hoytech commented Dec 3, 2016

Thanks! Yes that fixes it (you have to comment out 1583 and 1585 as well so it compiles).

I guess this adds some overhead to GET requests since even then it will create an empty stream. I'm not sure if that's important?

@ltriant
Copy link
Contributor

ltriant commented Sep 21, 2018

This change caused the tests to fail in the latest version.

However, with HTTP::Entity::Parser 0.20, this problem no longer happens and the test script above works without issue.

Would it be appropriate to revert these changes, so that Feersum can be installed without failing the graceful shutdown unit test? And perhaps mandate HTTP::Entity::Parser 0.20 as a dependency?

I can send a PR, if the solution works for you :)

@audreyt
Copy link
Collaborator

audreyt commented Sep 21, 2018

That works, please send the PR and looking forward to a new release with your contribution.

@ltriant
Copy link
Contributor

ltriant commented Sep 24, 2018

Thanks @audreyt :)

The remaining test failures seem to only affect the BSDs... If I can diagnose the issue, I'll send another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants