From db1e817508d9ee08bc6df9474795e2767e7a9f0a Mon Sep 17 00:00:00 2001 From: Masahiro Nagano Date: Mon, 16 Nov 2015 15:34:30 +0900 Subject: [PATCH 1/4] replace HTTP::Body with HTTP::Entity::Parser --- cpanfile | 3 +- lib/Plack/Request.pm | 135 +++++++++++++-------------------------- t/Plack-Request/params.t | 2 +- 3 files changed, 47 insertions(+), 93 deletions(-) diff --git a/cpanfile b/cpanfile index 871172b68..607fa52f5 100644 --- a/cpanfile +++ b/cpanfile @@ -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'; @@ -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.03; +requires 'WWW::Form::UrlEncoded', 0.22; on test => sub { requires 'Test::More', '0.88'; diff --git a/lib/Plack/Request.pm b/lib/Plack/Request.pm index 4304bfaa2..f27021462 100644 --- a/lib/Plack/Request.pm +++ b/lib/Plack/Request.pm @@ -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}) @@ -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; @@ -137,14 +118,27 @@ 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 @@ -152,9 +146,10 @@ 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} + ); }; } @@ -237,78 +232,36 @@ sub new_response { Plack::Response->new(@_); } -sub _parse_request_body { +my $default_parser = HTTP::Entity::Parser->new(); +$default_parser->register('application/x-www-form-urlencoded', 'HTTP::Entity::Parser::UrlEncoded'); +$default_parser->register('multipart/form-data', 'HTTP::Entity::Parser::MultiPart'); + +sub request_body_parser { my $self = shift; + $self->{request_body_parser} ||= $default_parser; +} - 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; +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; } - 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 ($buffer) { - $self->env->{'psgix.input.buffered'} = 1; - $self->env->{'psgi.input'} = $buffer->rewind; - } else { - $input->seek(0, 0); - } - - $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__ diff --git a/t/Plack-Request/params.t b/t/Plack-Request/params.t index 7bf336e11..57bf32730 100644 --- a/t/Plack-Request/params.t +++ b/t/Plack-Request/params.t @@ -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; From 56c0d6af18ec2a75692ebba16e6750246a760e46 Mon Sep 17 00:00:00 2001 From: Masahiro Nagano Date: Tue, 17 Nov 2015 16:16:09 +0900 Subject: [PATCH 2/4] set HTTP::Entitiy::Parser buffer length via an environment variable. related HTTP-Entity-Parser/issues/3 --- cpanfile | 2 +- lib/Plack/Request.pm | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cpanfile b/cpanfile index 607fa52f5..4af54ab9d 100644 --- a/cpanfile +++ b/cpanfile @@ -16,7 +16,7 @@ requires 'URI', '1.59'; requires 'parent'; requires 'Apache::LogFormat::Compiler', '0.12'; requires 'HTTP::Tiny', 0.034; -requires 'HTTP::Entity::Parser', 0.03; +requires 'HTTP::Entity::Parser', 0.15; requires 'WWW::Form::UrlEncoded', 0.22; on test => sub { diff --git a/lib/Plack/Request.pm b/lib/Plack/Request.pm index f27021462..e9c3e95b1 100644 --- a/lib/Plack/Request.pm +++ b/lib/Plack/Request.pm @@ -232,13 +232,15 @@ sub new_response { Plack::Response->new(@_); } -my $default_parser = HTTP::Entity::Parser->new(); -$default_parser->register('application/x-www-form-urlencoded', 'HTTP::Entity::Parser::UrlEncoded'); -$default_parser->register('multipart/form-data', 'HTTP::Entity::Parser::MultiPart'); - sub request_body_parser { my $self = shift; - $self->{request_body_parser} ||= $default_parser; + 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}; } sub _parse_request_body { From f4db58113a54591405102fead4cf435425bc1343 Mon Sep 17 00:00:00 2001 From: Masahiro Nagano Date: Wed, 18 Nov 2015 14:49:47 +0900 Subject: [PATCH 3/4] requries HTTP::Entity::Parser 0.16. improve Content-Disposition header parser --- cpanfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpanfile b/cpanfile index 4af54ab9d..ed0e9d23d 100644 --- a/cpanfile +++ b/cpanfile @@ -16,7 +16,7 @@ requires 'URI', '1.59'; requires 'parent'; requires 'Apache::LogFormat::Compiler', '0.12'; requires 'HTTP::Tiny', 0.034; -requires 'HTTP::Entity::Parser', 0.15; +requires 'HTTP::Entity::Parser', 0.16; requires 'WWW::Form::UrlEncoded', 0.22; on test => sub { From 81e1ce6cbad7b592a16d47c40883c3f7a57d6ee0 Mon Sep 17 00:00:00 2001 From: Masahiro Nagano Date: Fri, 20 Nov 2015 08:47:02 +0900 Subject: [PATCH 4/4] requries HTTP::Entity::Parser 0.17 and WWW::Form::UrlEncoded 0.23. --- cpanfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpanfile b/cpanfile index ed0e9d23d..2460cb3b8 100644 --- a/cpanfile +++ b/cpanfile @@ -16,8 +16,8 @@ requires 'URI', '1.59'; requires 'parent'; requires 'Apache::LogFormat::Compiler', '0.12'; requires 'HTTP::Tiny', 0.034; -requires 'HTTP::Entity::Parser', 0.16; -requires 'WWW::Form::UrlEncoded', 0.22; +requires 'HTTP::Entity::Parser', 0.17; +requires 'WWW::Form::UrlEncoded', 0.23; on test => sub { requires 'Test::More', '0.88';