added file upload support to param method in Mojolicious::Controller …

…and improved protection from excessively large form values
mojolicious · Feb 17, 2012 · cc91406 · cc91406
1 parent be86dfc
commit cc91406
Show file tree

Hide file tree

Showing 11 changed files with 156 additions and 64 deletions.
diff --git a/Changes b/Changes
@@ -3,7 +3,11 @@ This file documents the revision history for Perl extension Mojolicious.
 2.50    2012-02-17 00:00:00
         - Added EXPERIMENTAL is_running method to Mojo::IOWatcher and
           Mojo::IOWatcher::EV.
+        - Added file upload support to param method in
+          Mojolicious::Controller.
         - Improved Mojo::IOLoop to be controllable from foreign event loops.
+        - Improved protection from excessively large form values in
+          Mojo::Message.
         - Improved resilience of Mojo::IOLoop::Client.
         - Improved documentation.
         - Fixed small bug in makefile command.

diff --git a/lib/Mojo/Message.pm b/lib/Mojo/Message.pm
@@ -76,7 +76,8 @@ sub body_params {
   # "x-application-urlencoded" and "application/x-www-form-urlencoded"
   my $type = $self->headers->content_type || '';
   if ($type =~ m#(?:x-application|application/x-www-form)-urlencoded#i) {
-    $params->parse($self->content->asset->slurp);
+    return $params if (my $asset = $self->content->asset)->is_file;
+    $params->parse($asset->slurp);
   }
 
   # "multipart/formdata"
@@ -258,11 +259,7 @@ sub get_start_line_chunk {
 
 sub has_leftovers { shift->content->has_leftovers }
 
-sub header_size {
-  my $self = shift;
-  $self->fix_headers;
-  return $self->content->header_size;
-}
+sub header_size { shift->fix_headers->content->header_size }
 
 sub headers { shift->content->headers(@_) }
 
@@ -292,8 +289,7 @@ sub max_line_size { shift->headers->max_line_size(@_) }
 
 sub param {
   my $self = shift;
-  $self->{body_params} ||= $self->body_params;
-  return $self->{body_params}->param(@_);
+  return ($self->{body_params} ||= $self->body_params)->param(@_);
 }
 
 sub parse            { shift->_parse(0, @_) }
@@ -484,7 +480,8 @@ sub _parse_formdata {
 
     # Form value
     unless (defined $filename) {
-      $value = $part->asset->slurp;
+      next if (my $asset = $part->asset)->is_file;
+      $value = $asset->slurp;
       $value = decode($charset, $value) // $value
         if $charset && !$part->headers->content_transfer_encoding;
     }

diff --git a/lib/Mojo/Message/Request.pm b/lib/Mojo/Message/Request.pm
@@ -97,8 +97,7 @@ sub is_xhr {
 
 sub param {
   my $self = shift;
-  $self->{params} = $self->params unless $self->{params};
-  return $self->{params}->param(@_);
+  return ($self->{params} ||= $self->params)->param(@_);
 }
 
 sub params {

diff --git a/lib/Mojolicious/Controller.pm b/lib/Mojolicious/Controller.pm
@@ -151,9 +151,11 @@ sub param {
 
   # List
   my $p = $self->stash->{'mojo.captures'} || {};
+  my $req = $self->req;
   unless (defined $name) {
     my %seen;
-    my @keys = grep { !$seen{$_}++ } $self->req->param;
+    my @keys = grep { !$seen{$_}++ } $req->param;
+    push @keys, grep { !$seen{$_}++ } map { $_->name } @{$req->uploads};
     push @keys, grep { !$RESERVED{$_} && !$seen{$_}++ } keys %$p;
     return sort @keys;
   }
@@ -167,8 +169,12 @@ sub param {
   # Captured unreserved value
   return $p->{$name} if !$RESERVED{$name} && exists $p->{$name};
 
+  # Upload
+  my $upload = $req->upload($name);
+  return $upload if $upload;
+
   # Param value
-  return $self->req->param($name);
+  return $req->param($name);
 }
 
 # "Is there an app for kissing my shiny metal ass?
@@ -728,15 +734,18 @@ or L<Mojo::Transaction::WebSocket> object.
   my @foo   = $c->param('foo');
   $c        = $c->param(foo => 'ba;r');
 
-Access GET/POST parameters and route captures that are not reserved stash
-values.
+Access GET/POST parameters, file uploads and route captures that are not
+reserved stash values.
 
   # Only GET parameters
   my $foo = $c->req->url->query->param('foo');
 
   # Only GET and POST parameters
   my $foo = $c->req->param('foo');
 
+  # Only file uploads
+  my $foo = $c->req->upload('foo');
+
 =head2 C<redirect_to>
 
   $c = $c->redirect_to('named');

diff --git a/lib/Mojolicious/Guides/Growing.pod b/lib/Mojolicious/Guides/Growing.pod
@@ -267,7 +267,8 @@ L<Mojolicious> to make our C<model> available to all actions and templates.
   app->start;
 
 The C<param> method of our L<Mojolicious::Controller> instance is used to
-access query parameters, POST parameters and route placeholders, all at once.
+access query parameters, POST parameters, file uploads and route
+placeholders, all at once.
 
 =head2 Testing
 

diff --git a/lib/Mojolicious/Lite.pm b/lib/Mojolicious/Lite.pm
@@ -663,7 +663,7 @@ into a temporary file.
       if $self->req->is_limit_exceeded;
 
     # Process uploaded file
-    if (my $example = $self->req->upload('example')) {
+    if (my $example = $self->param('example')) {
       my $size = $example->size;
       my $name = $example->filename;
       $self->render(text => "Thanks for uploading $size byte file $name.");

diff --git a/t/mojo/cgi.t b/t/mojo/cgi.t
@@ -59,7 +59,7 @@ my $message = '';
 }
 my $res =
   Mojo::Message::Response->new->parse("HTTP/1.1 200 OK\x0d\x0a$message");
-is $res->code, 200, 'rigth status';
+is $res->code, 200, 'right status';
 is $res->headers->status, '200 OK', 'right "Status" value';
 is $res->headers->content_type, 'text/html;charset=UTF-8',
   'right "Content-Type" value';
@@ -81,7 +81,7 @@ $message = '';
   Mojolicious::Command::cgi->new->run('--nph');
 }
 $res = Mojo::Message::Response->new->parse($message);
-is $res->code, 200, 'rigth status';
+is $res->code, 200, 'right status';
 is $res->headers->status, undef, 'no "Status" value';
 is $res->headers->content_type, 'text/html;charset=UTF-8',
   'right "Content-Type" value';
@@ -109,7 +109,7 @@ $message = '';
 }
 like $message, qr/chunked/, 'is chunked';
 $res = Mojo::Message::Response->new->parse("HTTP/1.1 200 OK\x0d\x0a$message");
-is $res->code, 200, 'rigth status';
+is $res->code, 200, 'right status';
 is $res->headers->status, '200 OK', 'right "Status" value';
 is $res->body, '1234567', 'right content';
 
@@ -130,7 +130,7 @@ $message = '';
   Mojolicious::Command::cgi->new->run;
 }
 $res = Mojo::Message::Response->new->parse("HTTP/1.1 200 OK\x0d\x0a$message");
-is $res->code, 200, 'rigth status';
+is $res->code, 200, 'right status';
 is $res->headers->status, '200 OK', 'right "Status" value';
 is $res->headers->content_type, 'application/json',
   'right "Content-Type" value';

diff --git a/t/mojo/dom.t b/t/mojo/dom.t
@@ -1976,16 +1976,16 @@ $dom = Mojo::DOM->new(<<EOF);
   <head><meta http-equiv="content-type" content="text/html"></head>
 </html>
 EOF
-is $dom->find('[http-equiv]')->[0]->{content}, 'text/html', 'rigth attribute';
+is $dom->find('[http-equiv]')->[0]->{content}, 'text/html', 'right attribute';
 is $dom->find('[http-equiv]')->[1], undef, 'no result';
 is $dom->find('[http-equiv="content-type"]')->[0]->{content}, 'text/html',
-  'rigth attribute';
+  'right attribute';
 is $dom->find('[http-equiv="content-type"]')->[1], undef, 'no result';
 is $dom->find('[http-equiv^="content-"]')->[0]->{content}, 'text/html',
-  'rigth attribute';
+  'right attribute';
 is $dom->find('[http-equiv^="content-"]')->[1], undef, 'no result';
 is $dom->find('head > [http-equiv$="-type"]')->[0]->{content}, 'text/html',
-  'rigth attribute';
+  'right attribute';
 is $dom->find('head > [http-equiv$="-type"]')->[1], undef, 'no result';
 
 # Find "0" attribute value

diff --git a/t/mojo/path.t b/t/mojo/path.t
@@ -50,7 +50,7 @@ ok !$path->trailing_slash, 'no trailing slash';
 # Canonicalizing
 $path = Mojo::Path->new(
   '/%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc%2fpasswd');
-is "$path", '//../../../../../../../../../../etc/passwd', 'rigth result';
+is "$path", '//../../../../../../../../../../etc/passwd', 'right result';
 is $path->parts->[0],  '',       'right part';
 is $path->parts->[1],  '..',     'right part';
 is $path->parts->[2],  '..',     'right part';
@@ -66,7 +66,7 @@ is $path->parts->[11], 'etc',    'right part';
 is $path->parts->[12], 'passwd', 'right part';
 is $path->parts->[13], undef,    'no part';
 is $path->canonicalize, '/../../../../../../../../../../etc/passwd',
-  'rigth result';
+  'right result';
 is $path->parts->[0],  '..',     'right part';
 is $path->parts->[1],  '..',     'right part';
 is $path->parts->[2],  '..',     'right part';
@@ -86,7 +86,7 @@ ok !$path->trailing_slash, 'no trailing slash';
 # Canonicalizing (alternative)
 $path = Mojo::Path->new(
   '%2ftest%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc%2fpasswd');
-is "$path", '/test/../../../../../../../../../etc/passwd', 'rigth result';
+is "$path", '/test/../../../../../../../../../etc/passwd', 'right result';
 is $path->parts->[0],  'test',   'right part';
 is $path->parts->[1],  '..',     'right part';
 is $path->parts->[2],  '..',     'right part';
@@ -100,7 +100,7 @@ is $path->parts->[9],  '..',     'right part';
 is $path->parts->[10], 'etc',    'right part';
 is $path->parts->[11], 'passwd', 'right part';
 is $path->parts->[12], undef,    'no part';
-is $path->canonicalize, '/../../../../../../../../etc/passwd', 'rigth result';
+is $path->canonicalize, '/../../../../../../../../etc/passwd', 'right result';
 is $path->parts->[0],  '..',     'right part';
 is $path->parts->[1],  '..',     'right part';
 is $path->parts->[2],  '..',     'right part';
@@ -117,7 +117,7 @@ ok !$path->trailing_slash, 'no trailing slash';
 
 # Canonicalizing (with escaped "%")
 $path = Mojo::Path->new('%2ftest%2f..%252f..%2f..%2f..%2f..%2fetc%2fpasswd');
-is "$path", '/test/..%252f../../../../etc/passwd', 'rigth result';
+is "$path", '/test/..%252f../../../../etc/passwd', 'right result';
 is $path->parts->[0], 'test',    'right part';
 is $path->parts->[1], '..%2f..', 'right part';
 is $path->parts->[2], '..',      'right part';
@@ -126,7 +126,7 @@ is $path->parts->[4], '..',      'right part';
 is $path->parts->[5], 'etc',     'right part';
 is $path->parts->[6], 'passwd',  'right part';
 is $path->parts->[7], undef,     'no part';
-is $path->canonicalize, '/../etc/passwd', 'rigth result';
+is $path->canonicalize, '/../etc/passwd', 'right result';
 is $path->parts->[0], '..',     'right part';
 is $path->parts->[1], 'etc',    'right part';
 is $path->parts->[2], 'passwd', 'right part';
@@ -165,12 +165,12 @@ ok !$path->contains('/♥.html'), 'does not contain path';
 
 # Empty path elements
 $path = Mojo::Path->new('//');
-is "$path", '//', 'rigth result';
+is "$path", '//', 'right result';
 is $path->parts->[0], undef, 'no part';
 ok $path->leading_slash,  'has leading slash';
 ok $path->trailing_slash, 'has trailing slash';
 $path = Mojo::Path->new('/foo//bar/23/');
-is "$path", '/foo//bar/23/', 'rigth result';
+is "$path", '/foo//bar/23/', 'right result';
 is $path->parts->[0], 'foo', 'right part';
 is $path->parts->[1], '',    'right part';
 is $path->parts->[2], 'bar', 'right part';
@@ -179,7 +179,7 @@ is $path->parts->[4], undef, 'no part';
 ok $path->leading_slash,  'has leading slash';
 ok $path->trailing_slash, 'has trailing slash';
 $path = Mojo::Path->new('//foo/bar/23/');
-is "$path", '//foo/bar/23/', 'rigth result';
+is "$path", '//foo/bar/23/', 'right result';
 is $path->parts->[0], '',    'right part';
 is $path->parts->[1], 'foo', 'right part';
 is $path->parts->[2], 'bar', 'right part';
@@ -188,7 +188,7 @@ is $path->parts->[4], undef, 'no part';
 ok $path->leading_slash,  'has leading slash';
 ok $path->trailing_slash, 'has trailing slash';
 $path = Mojo::Path->new('/foo///bar/23/');
-is "$path", '/foo///bar/23/', 'rigth result';
+is "$path", '/foo///bar/23/', 'right result';
 is $path->parts->[0], 'foo', 'right part';
 is $path->parts->[1], '',    'right part';
 is $path->parts->[2], '',    'right part';
@@ -198,7 +198,7 @@ is $path->parts->[5], undef, 'no part';
 ok $path->leading_slash,  'has leading slash';
 ok $path->trailing_slash, 'has trailing slash';
 $path = Mojo::Path->new('///foo/bar/23/');
-is "$path", '///foo/bar/23/', 'rigth result';
+is "$path", '///foo/bar/23/', 'right result';
 is $path->parts->[0], '',    'right part';
 is $path->parts->[1], '',    'right part';
 is $path->parts->[2], 'foo', 'right part';
@@ -208,7 +208,7 @@ is $path->parts->[5], undef, 'no part';
 ok $path->leading_slash,  'has leading slash';
 ok $path->trailing_slash, 'has trailing slash';
 $path = Mojo::Path->new('///foo///bar/23///');
-is "$path", '///foo///bar/23///', 'rigth result';
+is "$path", '///foo///bar/23///', 'right result';
 is $path->parts->[0], '',    'right part';
 is $path->parts->[1], '',    'right part';
 is $path->parts->[2], 'foo', 'right part';
@@ -226,6 +226,6 @@ ok $path->trailing_slash, 'has trailing slash';
 $path = Mojo::Path->new->parts(['foo/bar']);
 is $path->parts->[0], 'foo/bar', 'right part';
 is $path->parts->[1], undef,     'no part';
-is "$path", 'foo%2Fbar', 'rigth result';
-is $path->to_string,     'foo%2Fbar',  'rigth result';
-is $path->to_abs_string, '/foo%2Fbar', 'rigth result';
+is "$path", 'foo%2Fbar', 'right result';
+is $path->to_string,     'foo%2Fbar',  'right result';
+is $path->to_abs_string, '/foo%2Fbar', 'right result';