X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=blobdiff_plain;f=lib%2FCatalyst%2FRequest.pm;h=671dd51e4368360260818c7a7add45624329ef27;hp=87ce66a22119de587f0602f97a7b600ab2dc5927;hb=b94f8e72c1c8e8348efc0eb96660dd977a838ea7;hpb=88ba7793bb132b13ecea722fcc56313756a408b9 diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm index 87ce66a..671dd51 100644 --- a/lib/Catalyst/Request.pm +++ b/lib/Catalyst/Request.pm @@ -10,14 +10,14 @@ use HTTP::Headers; use Stream::Buffered; use Hash::MultiValue; use Scalar::Util; - +use Catalyst::Exception; use Moose; use namespace::clean -except => 'meta'; with 'MooseX::Emulate::Class::Accessor::Fast'; -has env => (is => 'ro', writer => '_set_env', predicate => 'has_env'); +has env => (is => 'ro', writer => '_set_env', predicate => '_has_env'); # XXX Deprecated crap here - warn? has action => (is => 'rw'); # XXX: Deprecated in docs ages ago (2006), deprecated with warning in 5.8000 due @@ -60,7 +60,7 @@ has query_keywords => (is => 'rw'); has match => (is => 'rw'); has method => (is => 'rw'); has protocol => (is => 'rw'); -has query_parameters => (is => 'rw', default => sub { {} }); +has query_parameters => (is => 'rw', lazy=>1, default => sub { shift->_use_hash_multivalue ? Hash::MultiValue->new : +{} }); has secure => (is => 'rw', default => 0); has captures => (is => 'rw', default => sub { [] }); has uri => (is => 'rw', predicate => 'has_uri'); @@ -96,7 +96,7 @@ has _log => ( has io_fh => ( is=>'ro', - predicate=>'has_io_fh', + predicate=>'_has_io_fh', lazy=>1, builder=>'_build_io_fh'); @@ -118,7 +118,11 @@ has body_data => ( sub _build_body_data { my ($self) = @_; - my $content_type = $self->content_type; + + # Not sure if these returns should not be exceptions... + my $content_type = $self->content_type || return; + return unless ($self->method eq 'POST' || $self->method eq 'PUT'); + my ($match) = grep { $content_type =~/$_/i } keys(%{$self->data_handlers}); @@ -127,7 +131,7 @@ sub _build_body_data { local $_ = $fh; return $self->data_handlers->{$match}->($fh, $self); } else { - return undef; + Catalyst::Exception->throw("$content_type is does not have an available data handler"); } } @@ -210,15 +214,8 @@ sub _build_parameters { my $body_parameters = $self->body_parameters; my $query_parameters = $self->query_parameters; - ## setup for downstream plack - $self->env->{'plack.request.merged'} ||= do { - my $query = $self->env->{'plack.request.query'} || Hash::MultiValue->new; - my $body = $self->env->{'plack.request.body'} || Hash::MultiValue->new; - Hash::MultiValue->new($query->flatten, $body->flatten); - }; - if($self->_use_hash_multivalue) { - return $self->env->{'plack.request.merged'}->clone; # We want a copy, in case your App is evil + return Hash::MultiValue->new($query_parameters->flatten, $body_parameters->flatten); } # We copy, no references @@ -250,19 +247,12 @@ sub prepare_body { # If previously applied middleware created the HTTP::Body object, then we # just use that one. - if(my $plack_body = $self->env->{'plack.request.http.body'}) { + if(my $plack_body = $self->_has_env ? $self->env->{'plack.request.http.body'} : undef) { $self->_body($plack_body); $self->_body->cleanup(1); return; } - # Define PSGI ENV placeholders, or for empty should there be no content - # body (typical in HEAD or GET). Looks like from Plack::Request that - # middleware would probably expect to see this, even if empty - - $self->env->{'plack.request.body'} = Hash::MultiValue->new; - $self->env->{'plack.request.upload'} = Hash::MultiValue->new; - # If there is nothing to read, set body to naught and return. This # will cause all body code to be skipped @@ -312,9 +302,6 @@ sub prepare_body { $self->env->{'psgi.input'}->seek(0, 0); # Reset the buffer for downstream middleware or apps } - $self->env->{'plack.request.http.body'} = $self->_body; - $self->env->{'plack.request.body'} = Hash::MultiValue->from_mixed($self->_body->param); - # paranoia against wrong Content-Length header my $remaining = $length - $self->_read_position; if ( $remaining > 0 ) { @@ -332,10 +319,13 @@ sub prepare_body_parameters { my ( $self ) = @_; $self->prepare_body if ! $self->_has_body; - return {} unless $self->_body; + + unless($self->_body) { + return $self->_use_hash_multivalue ? Hash::MultiValue->new : {}; + } return $self->_use_hash_multivalue ? - $self->env->{'plack.request.body'}->clone : + Hash::MultiValue->from_mixed($self->_body->param) : $self->_body->param; } @@ -453,6 +443,7 @@ Catalyst::Request - provides information about the current client request $req->uri; $req->user; $req->user_agent; + $req->env; See also L, L. @@ -515,6 +506,13 @@ data of the type 'application/json' and return access to that data via this method. You may define addition data_handlers via a global configuration setting. See L for more information. +If the POST is malformed in some way (such as undefined or not content that +matches the content-type) we raise a L with the error +text as the message. + +If the POSTed content type does not match an availabled data handler, this +will also raise an exception. + =head2 $req->body_parameters Returns a reference to a hash containing body (POST) parameters. Values can @@ -649,6 +647,56 @@ If multiple C parameters are provided this code might corrupt data or cause a hash initialization error. For a more straightforward interface see C<< $c->req->parameters >>. +B Interfaces like this, which are based on L and the C method +are now known to cause demonstrated exploits. It is highly recommended that you +avoid using this method, and migrate existing code away from it. Here's the +whitepaper of the exploit: + +L + +Basically this is an exploit that takes advantage of how L<\param> will do one thing +in scalar context and another thing in list context. This is combined with how Perl +chooses to deal with duplicate keys in a hash definition by overwriting the value of +existing keys with a new value if the same key shows up again. Generally you will be +vulnerale to this exploit if you are using this method in a direct assignment in a +hash, such as with a L create statement. For example, if you have +parameters like: + + user?user=123&foo=a&foo=user&foo=456 + +You could end up with extra parameters injected into your method calls: + + $c->model('User')->create({ + user => $c->req->param('user'), + foo => $c->req->param('foo'), + }); + +Which would look like: + + $c->model('User')->create({ + user => 123, + foo => qw(a user 456), + }); + +(or to be absolutely clear if you are not seeing it): + + $c->model('User')->create({ + user => 456, + foo => 'a', + }); + +Possible remediations include scrubbing your parameters with a form validator like +L or being careful to force scalar context using the scalar +keyword: + + $c->model('User')->create({ + user => scalar($c->req->param('user')), + foo => scalar($c->req->param('foo')), + }); + +Upcoming versions of L will disable this interface by default and require +you to positively enable it should you require it for backwards compatibility reasons. + =cut sub param { @@ -658,9 +706,15 @@ sub param { return keys %{ $self->parameters }; } - if ( @_ == 1 ) { + # If anything in @_ is undef, carp about that, and remove it from + # the list; + + my @params = grep { defined($_) ? 1 : do {carp "You called ->params with an undefined value"; 0} } @_; + + if ( @params == 1 ) { - my $param = shift; + defined(my $param = shift @params) || + carp "You called ->params with an undefined value 2"; unless ( exists $self->parameters->{$param} ) { return wantarray ? () : undef; @@ -677,9 +731,9 @@ sub param { : $self->parameters->{$param}; } } - elsif ( @_ > 1 ) { - my $field = shift; - $self->parameters->{$field} = [@_]; + elsif ( @params > 1 ) { + my $field = shift @params; + $self->parameters->{$field} = [@params]; } } @@ -1003,6 +1057,9 @@ combined from those in the request and those in the body. If parameters have already been set will clear the parameters and build them again. +=head2 $self->env + +Access to the raw PSGI env. =head2 meta