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

Already on GitHub? Sign in to your account

PSGI Plugin: psgix.input.buffered should *not* be set to whatever post_b... #211

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

avar commented Apr 3, 2013

...uffering is set to

The psgix.input.buffered variable doesn't just mean "hey, I happen to
be buffering things internally in case you were interested". From the
PSGI spec:

 If psgix.input.buffered environment is true, it MUST implement
 the seek method.

While uWSGI technically complies with that with this stub method:

XS(XS_input_seek) {
    dXSARGS;
    psgi_check_args(1);
    XSRETURN(0);
}

It doesn't actually sync, this means that the first thing that reads
psgi.input via uwsgi::input will get content, but everything else
doesn't. This plays very badly with
Plack::Request::_parse_request_body() which does:

my $input = $self->input;

my $buffer;
if ($self->env->{'psgix.input.buffered'}) {
    # Just in case if input is read by middleware/apps beforehand
    $input->seek(0, 0);
} else {
    $buffer = Stream::Buffered->new($cl);
}

And later:

$buffer->print($chunk) if $buffer;

So $buffer never ends up being true if psgix.input.buffered=1, so
Plack::Request doesn't save away what it just read (since it assumes
it can get it back!), so later calling e.g. Plack::Request::raw_body()
does:

$fh->seek(0, 0); # just in case middleware/apps read it without seeking back
$fh->read(my($content), $cl, 0);
$fh->seek(0, 0);

And gets nothing! All of this fail is avoided if we just set
psgix.input.buffered=0 which'll cause Plack itself to buffer things up
via Stream::Buffered. In the future this could be set to true if uWSGI
actually implements the interface psgix.input.buffered requires.

@avar avar PSGI Plugin: psgix.input.buffered should *not* be set to whatever pos…
…t_buffering is set to

The psgix.input.buffered variable doesn't just mean "hey, I happen to
be buffering things internally in case you were interested". From the
PSGI spec:

     If psgix.input.buffered environment is true, it MUST implement
     the seek method.

While uWSGI technically complies with that with this stub method:

    XS(XS_input_seek) {
        dXSARGS;
        psgi_check_args(1);
        XSRETURN(0);
    }

It doesn't actually sync, this means that the *first* thing that reads
psgi.input via uwsgi::input will get content, but everything else
doesn't. This plays very badly with
Plack::Request::_parse_request_body() which does:

    my $input = $self->input;

    my $buffer;
    if ($self->env->{'psgix.input.buffered'}) {
        # Just in case if input is read by middleware/apps beforehand
        $input->seek(0, 0);
    } else {
        $buffer = Stream::Buffered->new($cl);
    }

And later:

    $buffer->print($chunk) if $buffer;

So $buffer never ends up being true if psgix.input.buffered=1, so
Plack::Request doesn't save away what it just read (since it assumes
it can get it back!), so later calling e.g. Plack::Request::raw_body()
does:

    $fh->seek(0, 0); # just in case middleware/apps read it without seeking back
    $fh->read(my($content), $cl, 0);
    $fh->seek(0, 0);

And gets nothing! All of this fail is avoided if we just set
psgix.input.buffered=0 which'll cause Plack itself to buffer things up
via Stream::Buffered. In the future this could be set to true if uWSGI
actually implements the interface psgix.input.buffered requires.
7f74d2c
Owner

unbit commented Apr 3, 2013

This patch f226f29 should correctly implement seek when post_buffering is enabled.

Contributor

avar commented Apr 16, 2013

Thanks, but that's a 1.9.-only feature, as I found trying to backport this stuff to 1.4..

If you're going to release more of the 1.4.* series it would be good to have my patch applied to those, so they won't claim to support a Plack API that they actually don't support.

Owner

unbit commented Apr 16, 2013

yes there will be more 1.4 releases, this patch will go in the next one. If you want to make a pull request for 1.4 branch i will apply (just to track contributors)

Owner

unbit commented Jun 2, 2013

i close this as in 1.4.10 we have added a special condition:

hv_store(env, "psgix.input.buffered", 20, newSViv(wsgi_req->body_as_file), 0))

@unbit unbit closed this Jun 2, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment