From: Andy Grundman Date: Fri, 3 Aug 2007 05:11:51 +0000 (+0000) Subject: Fixed a bug where c->read didn't work properly, and added some tests for parse_on_dem... X-Git-Tag: 5.7099_04~172 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=878b821cd4548d9d3f5a6c5aa05fb2f0c29fe3b0;hp=048f45ee23401c9f2fbe241639f2687c1e79f990;p=catagits%2FCatalyst-Runtime.git Fixed a bug where c->read didn't work properly, and added some tests for parse_on_demand mode --- diff --git a/Changes b/Changes index 8983f86..1f5ef4d 100644 --- a/Changes +++ b/Changes @@ -15,6 +15,7 @@ This file documents the revision history for Perl extension Catalyst. properly exit after a write error. (http://rt.cpan.org/Ticket/Display.html?id=27135) - Remove warning for captures that are undef. + - Fixed $c->read and parse_on_demand mode. 5.7007 2007-03-13 14:18:00 - Many performance improvements by not using URI.pm: diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm index 6f1848a..41b54eb 100644 --- a/lib/Catalyst.pm +++ b/lib/Catalyst.pm @@ -1590,8 +1590,14 @@ sub prepare { $c->prepare_cookies; $c->prepare_path; - # On-demand parsing - $c->prepare_body unless $c->config->{parse_on_demand}; + # Prepare the body for reading, either by prepare_body + # or the user, if they are using $c->read + $c->prepare_read; + + # Parse the body unless the user wants it on-demand + unless ( $c->config->{parse_on_demand} ) { + $c->prepare_body; + } } my $method = $c->req->method || ''; @@ -1806,6 +1812,10 @@ C<$maxlength> defaults to the size of the request if not specified. You have to set C<< MyApp->config->{parse_on_demand} >> to use this directly. +Warning: If you use read(), Catalyst will not process the body, +so you will not be able to access POST parameters or file uploads via +$c->request. You must handle all body parsing yourself. + =cut sub read { my $c = shift; return $c->engine->read( $c, @_ ) } @@ -2217,8 +2227,8 @@ This causes C to map to C. =head1 ON-DEMAND PARSER The request body is usually parsed at the beginning of a request, -but if you want to handle input yourself or speed things up a bit, -you can enable on-demand parsing with a config parameter. +but if you want to handle input yourself, you can enable on-demand +parsing with a config parameter. MyApp->config->{parse_on_demand} = 1; diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm index 5902c9c..0f801aa 100644 --- a/lib/Catalyst/Engine.pm +++ b/lib/Catalyst/Engine.pm @@ -279,11 +279,7 @@ sub finalize_headers { } =cut -sub finalize_read { - my ( $self, $c ) = @_; - - undef $self->{_prepared_read}; -} +sub finalize_read { } =head2 $self->finalize_uploads($c) @@ -312,12 +308,8 @@ sets up the L object body using L sub prepare_body { my ( $self, $c ) = @_; - - my $length = $c->request->header('Content-Length') || 0; - $self->read_length( $length ); - - if ( $length > 0 ) { + if ( my $length = $self->read_length ) { unless ( $c->request->{_body} ) { my $type = $c->request->header('Content-Type'); $c->request->{_body} = HTTP::Body->new( $type, $length ); @@ -494,8 +486,11 @@ prepare to read from the engine. sub prepare_read { my ( $self, $c ) = @_; - # Reset the read position + # Initialize the read position $self->read_position(0); + + # Initialize the amount of data we think we need to read + $self->read_length( $c->request->header('Content-Length') || 0 ); } =head2 $self->prepare_request(@arguments) @@ -565,11 +560,6 @@ sub prepare_write { } sub read { my ( $self, $c, $maxlength ) = @_; - unless ( $self->{_prepared_read} ) { - $self->prepare_read($c); - $self->{_prepared_read} = 1; - } - my $remaining = $self->read_length - $self->read_position; $maxlength ||= $CHUNKSIZE; diff --git a/t/lib/TestAppOnDemand.pm b/t/lib/TestAppOnDemand.pm new file mode 100644 index 0000000..6704a8a --- /dev/null +++ b/t/lib/TestAppOnDemand.pm @@ -0,0 +1,20 @@ +package TestAppOnDemand; + +use strict; +use Catalyst qw/ + Test::Errors + Test::Headers +/; +use Catalyst::Utils; + +our $VERSION = '0.01'; + +__PACKAGE__->config( + name => __PACKAGE__, + root => '/some/dir', + parse_on_demand => 1, +); + +__PACKAGE__->setup; + +1; diff --git a/t/lib/TestAppOnDemand/Controller/Body.pm b/t/lib/TestAppOnDemand/Controller/Body.pm new file mode 100644 index 0000000..aca4a5e --- /dev/null +++ b/t/lib/TestAppOnDemand/Controller/Body.pm @@ -0,0 +1,29 @@ +package TestAppOnDemand::Controller::Body; + +use strict; +use base 'Catalyst::Base'; + +use Data::Dump (); + +sub params : Local { + my ( $self, $c ) = @_; + + $c->res->body( Data::Dump::dump( $c->req->body_parameters ) ); +} + +sub read : Local { + my ( $self, $c ) = @_; + + # read some data + my @chunks; + + while ( my $data = $c->read( 10_000 ) ) { + push @chunks, $data; + } + + $c->res->content_type( 'text/plain'); + + $c->res->body( join ( '|', map { length $_ } @chunks ) ); +} + +1; \ No newline at end of file diff --git a/t/live_engine_request_body_demand.t b/t/live_engine_request_body_demand.t new file mode 100644 index 0000000..2444dc8 --- /dev/null +++ b/t/live_engine_request_body_demand.t @@ -0,0 +1,66 @@ +#!perl + +use strict; +use warnings; + +use FindBin; +use lib "$FindBin::Bin/lib"; + +use Test::More tests => 8; +use Catalyst::Test 'TestAppOnDemand'; + +use Catalyst::Request; +use HTTP::Headers; +use HTTP::Request::Common; + +# Test a simple POST request to make sure body parsing +# works in on-demand mode. +SKIP: +{ + if ( $ENV{CATALYST_SERVER} ) { + skip "Using remote server", 8; + } + + { + my $params; + + my $request = POST( + 'http://localhost/body/params', + 'Content-Type' => 'application/x-www-form-urlencoded', + 'Content' => 'foo=bar&baz=quux' + ); + + my $expected = { foo => 'bar', baz => 'quux' }; + + ok( my $response = request($request), 'Request' ); + ok( $response->is_success, 'Response Successful 2xx' ); + + { + no strict 'refs'; + ok( + eval '$params = ' . $response->content, + 'Unserialize params' + ); + } + + is_deeply( $params, $expected, 'Catalyst::Request body parameters' ); + } + + # Test reading chunks of the request body using $c->read + { + my $creq; + + my $request = POST( + 'http://localhost/body/read', + 'Content-Type' => 'text/plain', + 'Content' => 'x' x 105_000 + ); + + my $expected = '10000|10000|10000|10000|10000|10000|10000|10000|10000|10000|5000'; + + ok( my $response = request($request), 'Request' ); + ok( $response->is_success, 'Response Successful 2xx' ); + is( $response->content_type, 'text/plain', 'Response Content-Type' ); + is( $response->content, $expected, 'Response Content' ); + } +}