Skip to content

replace HTTP::Body with HTTP::Entity::Parser and optimize query_parameters #538

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.

Sign up for GitHub

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

Merged
miyagawa merged 4 commits into from
Nov 25, 2015
Merged

replace HTTP::Body with HTTP::Entity::Parser and optimize query_parameters #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db1e817
replace HTTP::Body with HTTP::Entity::Parser
kazeburo Nov 16, 2015
56c0d6a
set HTTP::Entitiy::Parser buffer length via an environment variable. …
kazeburo Nov 17, 2015
f4db581
requries HTTP::Entity::Parser 0.16. improve Content-Disposition heade…
kazeburo Nov 18, 2015
81e1ce6
requries HTTP::Entity::Parser 0.17 and WWW::Form::UrlEncoded 0.23.
kazeburo Nov 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cpanfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ requires 'Devel::StackTrace', '1.23';
requires 'Devel::StackTrace::AsHTML', '0.11';
requires 'File::ShareDir', '1.00';
requires 'Filesys::Notify::Simple';
requires 'HTTP::Body', '1.06';
requires 'HTTP::Message', '5.814';
requires 'HTTP::Headers::Fast', '0.18';
requires 'Hash::MultiValue', '0.05';
Expand All @@ -17,6 +16,8 @@ requires 'URI', '1.59';
requires 'parent';
requires 'Apache::LogFormat::Compiler', '0.12';
requires 'HTTP::Tiny', 0.034;
requires 'HTTP::Entity::Parser', 0.17;
requires 'WWW::Form::UrlEncoded', 0.23;

on test => sub {
requires 'Test::More', '0.88';
Expand Down
137 changes: 46 additions & 91 deletions lib/Plack/Request.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ our $VERSION = '1.0037';
use HTTP::Headers::Fast;
use Carp ();
use Hash::MultiValue;
use HTTP::Body;

use Plack::Request::Upload;
use Stream::Buffered;
use URI;
use URI::Escape ();

use HTTP::Entity::Parser;
use WWW::Form::UrlEncoded qw/parse_urlencoded_arrayref/;

sub new {
my($class, $env) = @_;
Carp::croak(q{$env is required})
Expand Down Expand Up @@ -75,27 +77,6 @@ sub cookies {
$self->env->{'plack.cookie.parsed'} = \%results;
}

sub query_parameters {
my $self = shift;
$self->env->{'plack.request.query'} ||= $self->_parse_query;
}

sub _parse_query {
my $self = shift;

my @query;
my $query_string = $self->env->{QUERY_STRING};
if (defined $query_string) {
$query_string =~ s/\A[&;]+//;
@query =
map { s/\+/ /g; URI::Escape::uri_unescape($_) }
map { /=/ ? split(/=/, $_, 2) : ($_ => '')}
split(/[&;]+/, $query_string);
}

Hash::MultiValue->new(@query);
}

sub content {
my $self = shift;

Expand Down Expand Up @@ -137,24 +118,38 @@ sub header { shift->headers->header(@_) }
sub referer { shift->headers->referer(@_) }
sub user_agent { shift->headers->user_agent(@_) }

sub body_parameters {
sub _body_parameters {
my $self = shift;

unless ($self->env->{'plack.request.body'}) {
unless ($self->env->{'plack.request.body_parameters'}) {
$self->_parse_request_body;
}
return $self->env->{'plack.request.body_parameters'};
}

sub _query_parameters {
my $self = shift;
$self->env->{'plack.request.query_parameters'} ||= parse_urlencoded_arrayref($self->env->{'QUERY_STRING'});
}

return $self->env->{'plack.request.body'};
sub query_parameters {
my $self = shift;
$self->env->{'plack.request.query'} ||= Hash::MultiValue->new(@{$self->_query_parameters});
}

sub body_parameters {
my $self = shift;
$self->env->{'plack.request.body'} ||= Hash::MultiValue->new(@{$self->_body_parameters});
}

# contains body + query
sub parameters {
my $self = shift;

$self->env->{'plack.request.merged'} ||= do {
my $query = $self->query_parameters;
my $body = $self->body_parameters;
Hash::MultiValue->new($query->flatten, $body->flatten);
Hash::MultiValue->new(
@{$self->_query_parameters},
@{$self->_body_parameters}
);
};
}

Expand Down Expand Up @@ -237,78 +232,38 @@ sub new_response {
Plack::Response->new(@_);
}

sub _parse_request_body {
sub request_body_parser {
my $self = shift;

my $ct = $self->env->{CONTENT_TYPE};
my $cl = $self->env->{CONTENT_LENGTH};
if (!$ct && !$cl) {
# No Content-Type nor Content-Length -> GET/HEAD
$self->env->{'plack.request.body'} = Hash::MultiValue->new;
$self->env->{'plack.request.upload'} = Hash::MultiValue->new;
return;
}

my $body = HTTP::Body->new($ct, $cl);

# HTTP::Body will create temporary files in case there was an
# upload. Those temporary files can be cleaned up by telling
# HTTP::Body to do so. It will run the cleanup when the request
# env is destroyed. That the object will not go out of scope by
# the end of this sub we will store a reference here.
$self->env->{'plack.request.http.body'} = $body;
$body->cleanup(1);

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);
}

my $spin = 0;
while ($cl) {
$input->read(my $chunk, $cl < 8192 ? $cl : 8192);
my $read = length $chunk;
$cl -= $read;
$body->add($chunk);
$buffer->print($chunk) if $buffer;

if ($read == 0 && $spin++ > 2000) {
Carp::croak "Bad Content-Length: maybe client disconnect? ($cl bytes remaining)";
}
if ( !$self->{request_body_parser} ) {
my $default_parser = HTTP::Entity::Parser->new(exists $ENV{PLACK_BUFFER_LENGTH} ? (buffer_length => $ENV{PLACK_BUFFER_LENGTH}) : ());
$default_parser->register('application/x-www-form-urlencoded', 'HTTP::Entity::Parser::UrlEncoded');
$default_parser->register('multipart/form-data', 'HTTP::Entity::Parser::MultiPart');
$self->{request_body_parser} = $default_parser;
}
$self->{request_body_parser};
}

if ($buffer) {
$self->env->{'psgix.input.buffered'} = 1;
$self->env->{'psgi.input'} = $buffer->rewind;
} else {
$input->seek(0, 0);
sub _parse_request_body {
my $self = shift;
if ( !$self->env->{CONTENT_TYPE} ) {
$self->env->{'plack.request.body_parameters'} = [];
$self->env->{'plack.request.upload'} = Hash::MultiValue->new();
return;
}

$self->env->{'plack.request.body'} = Hash::MultiValue->from_mixed($body->param);
my ($params,$uploads) = $self->request_body_parser->parse($self->env);
$self->env->{'plack.request.body_parameters'} = $params;

my @uploads = Hash::MultiValue->from_mixed($body->upload)->flatten;
my @obj;
while (my($k, $v) = splice @uploads, 0, 2) {
push @obj, $k, $self->_make_upload($v);
my $upload_hash = Hash::MultiValue->new();
while ( my ($k,$v) = splice @$uploads, 0, 2 ) {
my %copy = %$v;
$copy{headers} = HTTP::Headers::Fast->new(@{$v->{headers}});
$upload_hash->add($k, Plack::Request::Upload->new(%copy));
}

$self->env->{'plack.request.upload'} = Hash::MultiValue->new(@obj);

$self->env->{'plack.request.upload'} = $upload_hash;
1;
}

sub _make_upload {
my($self, $upload) = @_;
my %copy = %$upload;
$copy{headers} = HTTP::Headers::Fast->new(%{$upload->{headers}});
Plack::Request::Upload->new(%copy);
}

1;
__END__

Expand Down
2 changes: 1 addition & 1 deletion t/Plack-Request/params.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ is_deeply [ $req->param('foo') ] , [ qw(bar baz) ];
is_deeply [ sort $req->param ], [ 'bar', 'foo' ];

$req = Plack::Request->new({ QUERY_STRING => "&&foo=bar&&baz=quux" });
is_deeply $req->parameters, { foo => 'bar', baz => 'quux' };
is_deeply $req->parameters, { "" => "", foo => 'bar', baz => 'quux' };

done_testing;