From: Jay Hannah Date: Tue, 16 Jan 2018 22:43:48 +0000 (-0600) Subject: Merge branch 'pr/154' into release-candidates/rc-5.90116 X-Git-Tag: 5.90116~8 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=dddc7ec8cc0e9112ff51caad562a15e3699e3fab;hp=8098d754062214cb2f951e06b3e5cec8b965b8d8 Merge branch 'pr/154' into release-candidates/rc-5.90116 Conflicts jhannah resolved manually: Makefile.PL lib/Catalyst.pm --- diff --git a/Changes b/Changes index 8014ea8..0fc85ac 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,16 @@ # This file documents the revision history for Perl extension Catalyst. +5.90115 - 2017-05-01 + - fixes for silent bad behavior in Catalyst::ScriptRole and 'ensure_class_loaded' + (hobbs++) + - do not require MXRWO if Moose is new enough to have cored it (ether++) + - documentation improvements (ether++) + - Encoding documentation improvements (colinnewell++) + - Improve documentation and test cases for 'abort_chain_on_error_fix' configuration + option (melmothx++) + - Better debug output when using Hash::MultiValue (tremor69++) + - Fixes for detecting debug terminal size (simonamor++) + 5.90114 - 2016-12-19 - Fixed regression introduced in the last version (5.90113) which caused application to hang when the action private name contained a string diff --git a/Makefile.PL b/Makefile.PL index f4d5a0a..2140deb 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -34,7 +34,7 @@ requires 'Class::Load' => '0.12'; requires 'Data::OptList'; requires 'Moose' => '1.03'; requires 'MooseX::MethodAttributes::Role::AttrContainer::Inheritable' => '0.24'; -requires 'MooseX::Role::WithOverloading' => '0.09'; +requires 'MooseX::Role::WithOverloading' => '0.09' unless can_use('Moose', '2.1300'); requires 'Carp' => '1.25'; requires 'Class::C3::Adopt::NEXT' => '0.07'; requires 'CGI::Simple::Cookie' => '1.109'; @@ -120,7 +120,6 @@ author_requires( @author_requires, map {; $_ => 0 } qw( File::Copy::Recursive - Test::Without::Module Starman MooseX::Daemonize Test::NoTabs @@ -142,6 +141,7 @@ resources( 'license', => 'http://dev.perl.org/licenses/', 'homepage', => 'http://dev.catalyst.perl.org/', # r/w: catagits@git.shadowcat.co.uk:Catalyst-Runtime.git + # web: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits/Catalyst-Runtime.git;a=summary 'repository', => 'git://git.shadowcat.co.uk/catagits/Catalyst-Runtime.git', ); diff --git a/README.mkdn b/README.mkdn index 7889225..203d418 100644 --- a/README.mkdn +++ b/README.mkdn @@ -2,23 +2,11 @@ Catalyst - The Elegant MVC Web Application Framework -
- - CPAN version - Catalyst></a> - <a href=Kwalitee Score -
- # SYNOPSIS See the [Catalyst::Manual](https://metacpan.org/pod/Catalyst::Manual) distribution for comprehensive documentation and tutorials. - # Building Catalyst for development - cpanm --local-lib=~/perl5 local::lib && eval $(perl -I ~/perl5/lib/perl5/ -Mlocal::lib) - cpanm --installdeps --with-develop . - perl Makefile.PL - # Install Catalyst::Devel for helpers and other development tools # use the helper to create a new application catalyst.pl MyApp @@ -296,7 +284,7 @@ the relationship between `$c->go` will perform a full dispatch on the specified action or method, with localized `$c->action` and `$c->namespace`. Like `detach`, `go` escapes the processing of the current request chain on completion, and -does not return to its cunless blessed $cunless blessed $caller. +does not return to its caller. @arguments are arguments to the final destination of $action. @captures are arguments to the intermediate steps, if any, on the way to the final sub of @@ -362,6 +350,11 @@ Contains the return value of the last executed action. Note that << $c->state >> operates in a scalar context which means that all values it returns are scalar. +Please note that if an action throws an exception, the value of state +should no longer be considered the return if the last action. It is generally +going to be 0, which indicates an error state. Examine $c->error for error +details. + ## $c->clear\_errors Clear errors. You probably don't want to clear the errors unless you are @@ -378,11 +371,17 @@ Returns true if you have errors ## $c->last\_error Returns the most recent error in the stack (the one most recently added...) -or nothing if there are no errors. +or nothing if there are no errors. This does not modify the contents of the +error stack. ## shift\_errors -shifts the most recently added error off the error stack and returns if. Returns +shifts the most recently added error off the error stack and returns it. Returns +nothing if there are no more errors. + +## pop\_errors + +pops the most recently added error off the error stack and returns it. Returns nothing if there are no more errors. ## COMPONENT ACCESSORS @@ -637,11 +636,11 @@ Example: ## do stuff here.. }; -## $c->uri\_for( $path?, @args?, \\%query\_values? ) +## $c->uri\_for( $path?, @args?, \\%query\_values?, \\$fragment? ) -## $c->uri\_for( $action, \\@captures?, @args?, \\%query\_values? ) +## $c->uri\_for( $action, \\@captures?, @args?, \\%query\_values?, \\$fragment? ) -## $c->uri\_for( $action, \[@captures, @args\], \\%query\_values? ) +## $c->uri\_for( $action, \[@captures, @args\], \\%query\_values?, \\$fragment? ) Constructs an absolute [URI](https://metacpan.org/pod/URI) object based on the application root, the provided path, and the additional arguments and query parameters provided. @@ -659,6 +658,15 @@ relative to the application root (if it does). It is then merged with `$c->request->base`; any `@args` are appended as additional path components; and any `%query_values` are appended as `?foo=bar` parameters. +**NOTE** If you are using this 'stringy' first argument, we skip encoding and +allow you to declare something like: + + $c->uri_for('/foo/bar#baz') + +Where 'baz' is a URI fragment. We consider this first argument string to be +'expert' mode where you are expected to create a valid URL and we for the most +part just pass it through without a lot of internal effort to escape and encode. + If the first argument is a [Catalyst::Action](https://metacpan.org/pod/Catalyst::Action) it represents an action which will have its path resolved using `$c->dispatcher->uri_for_action`. The optional `\@captures` argument (an arrayref) allows passing the captured @@ -976,7 +984,26 @@ Returns or sets the request class. Defaults to [Catalyst::Request](https://metac ## $app->request\_class\_traits -An arrayref of [Moose::Role](https://metacpan.org/pod/Moose::Role)s which are applied to the request class. +An arrayref of [Moose::Role](https://metacpan.org/pod/Moose::Role)s which are applied to the request class. You can +name the full namespace of the role, or a namespace suffix, which will then +be tried against the following standard namespace prefixes. + + $MyApp::TraitFor::Request::$trait_suffix + Catalyst::TraitFor::Request::$trait_suffix + +So for example if you set: + + MyApp->request_class_traits(['Foo']); + +We try each possible role in turn (and throw an error if none load) + + Foo + MyApp::TraitFor::Request::Foo + Catalyst::TraitFor::Request::Foo + +The namespace part 'TraitFor::Request' was chosen to assist in backwards +compatibility with [CatalystX::RoleApplicator](https://metacpan.org/pod/CatalystX::RoleApplicator) which previously provided +these features in a stand alone package. ## $app->composed\_request\_class @@ -988,7 +1015,26 @@ Returns or sets the response class. Defaults to [Catalyst::Response](https://met ## $app->response\_class\_traits -An arrayref of [Moose::Role](https://metacpan.org/pod/Moose::Role)s which are applied to the response class. +An arrayref of [Moose::Role](https://metacpan.org/pod/Moose::Role)s which are applied to the response class. You can +name the full namespace of the role, or a namespace suffix, which will then +be tried against the following standard namespace prefixes. + + $MyApp::TraitFor::Response::$trait_suffix + Catalyst::TraitFor::Response::$trait_suffix + +So for example if you set: + + MyApp->response_class_traits(['Foo']); + +We try each possible role in turn (and throw an error if none load) + + Foo + MyApp::TraitFor::Response::Foo + Catalyst::TraitFor::Responset::Foo + +The namespace part 'TraitFor::Response' was chosen to assist in backwards +compatibility with [CatalystX::RoleApplicator](https://metacpan.org/pod/CatalystX::RoleApplicator) which previously provided +these features in a stand alone package. ## $app->composed\_response\_class @@ -1191,15 +1237,44 @@ Sets up the input/output encoding. See [ENCODING](https://metacpan.org/pod/ENCOD ## handle\_unicode\_encoding\_exception -Hook to let you customize how encoding errors are handled. By default -we just throw an exception. Receives a hashref of debug information. -Example: +Hook to let you customize how encoding errors are handled. By default +we just throw an exception and the default error page will pick it up. +Receives a hashref of debug information. Example of call (from the +Catalyst internals): - $c->handle_unicode_encoding_exception({ - param_value => $value, - error_msg => $_, - encoding_step => 'params', - }); + my $decoded_after_fail = $c->handle_unicode_encoding_exception({ + param_value => $value, + error_msg => $_, + encoding_step => 'params', + }); + +The calling code expects to receive a decoded string or an exception. + +You can override this for custom handling of unicode errors. By +default we just die. If you want a custom response here, one approach +is to throw an HTTP style exception, instead of returning a decoded +string or throwing a generic exception. + + sub handle_unicode_encoding_exception { + my ($c, $params) = @_; + HTTP::Exception::BAD_REQUEST->throw(status_message=>$params->{error_msg}); + } + +Alternatively you can 'catch' the error, stash it and write handling code later +in your application: + + sub handle_unicode_encoding_exception { + my ($c, $params) = @_; + $c->stash(BAD_UNICODE_DATA=>$params); + # return a dummy string. + return 1; + } + +NOTE:</b> Please keep in mind that once an error like this occurs, +the request setup is still ongoing, which means the state of `$c` and +related context parts like the request and response may not be setup +up correctly (since we haven't finished the setup yet). If you throw +an exception the setup is aborted. ## $c->setup\_log @@ -1365,7 +1440,26 @@ A arrayref of [Moose::Role](https://metacpan.org/pod/Moose::Role)s that are appl ## $app->composed\_stats\_class -this is the stats\_class composed with any 'stats\_class\_traits'. +this is the stats\_class composed with any 'stats\_class\_traits'. You can +name the full namespace of the role, or a namespace suffix, which will then +be tried against the following standard namespace prefixes. + + $MyApp::TraitFor::Stats::$trait_suffix + Catalyst::TraitFor::Stats::$trait_suffix + +So for example if you set: + + MyApp->stats_class_traits(['Foo']); + +We try each possible role in turn (and throw an error if none load) + + Foo + MyApp::TraitFor::Stats::Foo + Catalyst::TraitFor::Stats::Foo + +The namespace part 'TraitFor::Stats' was chosen to assist in backwards +compatibility with [CatalystX::RoleApplicator](https://metacpan.org/pod/CatalystX::RoleApplicator) which previously provided +these features in a stand alone package. ## $c->use\_stats @@ -1445,7 +1539,7 @@ variable should be used for determining the request path. decoded, this means that applications using this mode can correctly handle URIs including the %2F character (i.e. with `AllowEncodedSlashes` set to `On` in Apache). - Given that this method of path resolution is probably more correct, it is recommended that you use + Given that this method of path resolution is provably more correct, it is recommended that you use this unless you have a specific need to deploy your application in a non-standard environment, and you are aware of the implications of not being able to handle encoded URI paths correctly. @@ -1459,7 +1553,7 @@ variable should be used for determining the request path. - `using_frontend_proxy_path` - Enabled [Plack::Middleware::ReverseProxyPath](https://metacpan.org/pod/Plack::Middleware::ReverseProxyPath) on your application (if installed, otherwise log an error). This is useful if your application is not running on the 'root' (or /) of your host server. **NOTE** if you use this feature you should add the required -middleware to your project dependency list since it's not automatically a dependency of [Catalyst](https://metacpan.org/pod/Catalyst). +middleware to your project dependency list since its not automatically a dependency of [Catalyst](https://metacpan.org/pod/Catalyst). This has been done since not all people need this feature and we wish to restrict the growth of [Catalyst](https://metacpan.org/pod/Catalyst) dependencies. - `encoding` - See ["ENCODING"](#encoding) @@ -1469,18 +1563,19 @@ This has been done since not all people need this feature and we wish to restric - `abort_chain_on_error_fix` - When there is an error in an action chain, the default behavior is to continue - processing the remaining actions and then catch the error upon chain end. This - can lead to running actions when the application is in an unexpected state. If - you have this issue, setting this config value to true will promptly exit a - chain when there is an error raised in any action (thus terminating the chain - early.) + Defaults to true. - use like: + When there is an error in an action chain, the default behavior is to + abort the processing of the remaining actions to avoid running them + when the application is in an unexpected state. - __PACKAGE__->config(abort_chain_on_error_fix => 1); + Before version 5.90070, the default used to be false. To keep the old + behaviour, you can explicitly set the value to false. E.g. - In the future this might become the default behavior. + __PACKAGE__->config(abort_chain_on_error_fix => 0); + + If this setting is set to false, then the remaining actions are + performed and the error is caught at the end of the chain. - `use_hash_multivalue_in_request` @@ -1527,8 +1622,14 @@ This has been done since not all people need this feature and we wish to restric in Catalyst version 5.90080+ break some of your query code, you may disable the UTF-8 decoding globally using this configuration. - This setting takes precedence over `default_query_encoding` and - `decode_query_using_global_encoding` + This setting takes precedence over `default_query_encoding` + +- `do_not_check_query_encoding` + + Catalyst versions 5.90080 - 5.90106 would decode query parts of an incoming + request but would not raise an exception when the decoding failed due to + incorrect unicode. It now does, but if this change is giving you trouble + you may disable it by setting this configuration to true. - `default_query_encoding` @@ -1537,13 +1638,6 @@ This has been done since not all people need this feature and we wish to restric specify a fixed value for how to decode your query. You might need this if you are doing a lot of custom encoding of your URLs and not using UTF-8. - This setting take precedence over `decode_query_using_global_encoding`. - -- `decode_query_using_global_encoding` - - Setting this to true will default your query decoding to whatever your - general global encoding is (the default is UTF-8). - - `use_chained_args_0_special_case` In older versions of Catalyst, when more than one action matched the same path @@ -1904,6 +1998,11 @@ the encoding configuration to undef. This is recommended for temporary backwards compatibility only. +To turn it off for a single request use the [clear\_encoding](https://metacpan.org/pod/clear_encoding) +method to turn off encoding for this request. This can be useful +when you are setting the body to be an arbitrary block of bytes, +especially if that block happens to be a block of UTF8 text. + Encoding is automatically applied when the content-type is set to a type that can be encoded. Currently we encode when the content type matches the following regular expression: @@ -2032,7 +2131,7 @@ Caelum: Rafael Kitover chansen: Christian Hansen -Chase Venters +Chase Venters `chase.venters@gmail.com` chicks: Christopher Hicks @@ -2096,8 +2195,6 @@ konobi: Scott McWhirter marcus: Marcus Ramberg -Mischa Spiegelmock - miyagawa: Tatsuhiko Miyagawa mgrimes: Mark Grimes diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm index 4bc7460..d4701cc 100644 --- a/lib/Catalyst.pm +++ b/lib/Catalyst.pm @@ -205,7 +205,7 @@ sub composed_stats_class { __PACKAGE__->_encode_check(Encode::FB_CROAK | Encode::LEAVE_SRC); # Remember to update this in Catalyst::Runtime as well! -our $VERSION = '5.90114'; +our $VERSION = '5.90115'; $VERSION = eval $VERSION if $VERSION =~ /_/; # numify for warning-free dev releases sub import { @@ -247,11 +247,6 @@ sub _application { $_[0] } Catalyst - The Elegant MVC Web Application Framework -=for html -CPAN version -Catalyst></a> -<a href=Kwalitee Score - =head1 SYNOPSIS See the L distribution for comprehensive @@ -475,7 +470,7 @@ or stash it like so: and access it from the stash. -Keep in mind that the C method used is that of the caller action. So a C<$c-Edetach> inside a forwarded action would run the C method from the original action requested. +Keep in mind that the C method used is that of the caller action. So a C<< $c->detach >> inside a forwarded action would run the C method from the original action requested. =cut @@ -1706,23 +1701,20 @@ sub uri_for { # somewhat lifted from URI::_query's query_form $query = '?'.join('&', map { my $val = $params->{$_}; - #s/([;\/?:@&=+,\$\[\]%])/$URI::Escape::escapes{$1}/go; ## Commented out because seems to lead to double encoding - JNAP - s/ /+/g; - my $key = $_; + my $key = encode_utf8($_); + # using the URI::Escape pattern here so utf8 chars survive + $key =~ s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go; + $key =~ s/ /+/g; + $val = '' unless defined $val; (map { - my $param = "$_"; - $param = encode_utf8($param); + my $param = encode_utf8($_); # using the URI::Escape pattern here so utf8 chars survive $param =~ s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go; $param =~ s/ /+/g; - $key = encode_utf8($key); - # using the URI::Escape pattern here so utf8 chars survive - $key =~ s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go; - $key =~ s/ /+/g; - - "${key}=$param"; } ( ref $val eq 'ARRAY' ? @$val : $val )); + "${key}=$param"; + } ( ref $val eq 'ARRAY' ? @$val : $val )); } @keys); } @@ -2322,6 +2314,10 @@ sub finalize_encoding { (defined($res->body)) and (ref(\$res->body) eq 'SCALAR') ) { + # if you are finding yourself here and your body is already encoded correctly + # and you want to turn this off, use $c->clear_encoding to prevent encoding + # at this step, or set encoding to undef in the config to do so for the whole + # application. See the ENCODING documentaiton for better notes. $c->res->body( $c->encoding->encode( $c->res->body, $c->_encode_check ) ); # Set the charset if necessary. This might be a bit bonkers since encodable response @@ -2741,9 +2737,16 @@ sub log_request_parameters { next if ! keys %$params; my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [ $column_width, 'Value' ] ); for my $key ( sort keys %$params ) { - my $param = $params->{$key}; - my $value = defined($param) ? $param : ''; - $t->row( $key, ref $value eq 'ARRAY' ? ( join ', ', @$value ) : $value ); + my @values = (); + if(ref $params eq 'Hash::MultiValue') { + @values = $params->get_all($key); + } else { + my $param = $params->{$key}; + if( defined($param) ) { + @values = ref $param eq 'ARRAY' ? @$param : $param; + } + } + $t->row( $key.( scalar @values > 1 ? ' [multiple]' : ''), join(', ', @values) ); } $c->log->debug( ucfirst($type) . " Parameters are:\n" . $t->draw ); } @@ -4300,18 +4303,20 @@ value to undef. C -When there is an error in an action chain, the default behavior is to continue -processing the remaining actions and then catch the error upon chain end. This -can lead to running actions when the application is in an unexpected state. If -you have this issue, setting this config value to true will promptly exit a -chain when there is an error raised in any action (thus terminating the chain -early.) +Defaults to true. + +When there is an error in an action chain, the default behavior is to +abort the processing of the remaining actions to avoid running them +when the application is in an unexpected state. + +Before version 5.90070, the default used to be false. To keep the old +behaviour, you can explicitly set the value to false. E.g. -use like: + __PACKAGE__->config(abort_chain_on_error_fix => 0); - __PACKAGE__->config(abort_chain_on_error_fix => 1); +If this setting is set to false, then the remaining actions are +performed and the error is caught at the end of the chain. -In the future this might become the default behavior. =item * @@ -4765,6 +4770,11 @@ the encoding configuration to undef. This is recommended for temporary backwards compatibility only. +To turn it off for a single request use the L +method to turn off encoding for this request. This can be useful +when you are setting the body to be an arbitrary block of bytes, +especially if that block happens to be a block of UTF8 text. + Encoding is automatically applied when the content-type is set to a type that can be encoded. Currently we encode when the content type matches the following regular expression: @@ -4887,7 +4897,7 @@ andrewalker: André Walker Andrew Bramble -Andrew Ford EA.Ford@ford-mason.co.ukE +Andrew Ford Andrew Ruthven @@ -4901,19 +4911,19 @@ Caelum: Rafael Kitover chansen: Christian Hansen -Chase Venters C +Chase Venters chicks: Christopher Hicks -Chisel Wright C +Chisel Wright -Danijel Milicevic C +Danijel Milicevic davewood: David Schmidt -David Kamholz Edkamholz@cpan.orgE +David Kamholz -David Naughton, C +David Naughton David E. Wheeler @@ -4935,7 +4945,7 @@ gabb: Danijel Milicevic Gary Ashton Jones -Gavin Henry C +Gavin Henry Geoff Richards @@ -4947,7 +4957,7 @@ ilmari: Dagfinn Ilmari Mannsåker jcamacho: Juan Camacho -jester: Jesse Sheidlower C +jester: Jesse Sheidlower jhannah: Jay Hannah @@ -4957,9 +4967,9 @@ Johan Lindstrom jon: Jon Schutz -Jonathan Rockway C<< >> +Jonathan Rockway -Kieren Diment C +Kieren Diment konobi: Scott McWhirter @@ -4995,7 +5005,9 @@ rafl: Florian Ragwitz random: Roland Lammel -Robert Sedlacek C<< >> +revmischa: Mischa Spiegelmock + +Robert Sedlacek SpiceMan: Marcel Montes @@ -5009,17 +5021,17 @@ Ulf Edvinsson vanstyn: Henry Van Styn -Viljo Marrandi C +Viljo Marrandi -Will Hawes C +Will Hawes willert: Sebastian Willert wreis: Wallace Reis -Yuval Kogman, C +Yuval Kogman -rainboxx: Matthias Dietrich, C +rainboxx: Matthias Dietrich dd070: Dhaval Dhanani diff --git a/lib/Catalyst/Component/ContextClosure.pm b/lib/Catalyst/Component/ContextClosure.pm index 9c3139a..b25cc46 100644 --- a/lib/Catalyst/Component/ContextClosure.pm +++ b/lib/Catalyst/Component/ContextClosure.pm @@ -67,7 +67,7 @@ L =head1 AUTHOR -Florian Ragwitz Erafl@debian.orgE +Florian Ragwitz =end stopwords diff --git a/lib/Catalyst/Controller.pm b/lib/Catalyst/Controller.pm index f478ddf..f65ee10 100644 --- a/lib/Catalyst/Controller.pm +++ b/lib/Catalyst/Controller.pm @@ -861,9 +861,9 @@ The following is exactly the same: package MyApp::Controller::Zoo; - sub foo : Local Does('Moo') { ... } # Catalyst::ActionRole:: - sub bar : Local Does('~Moo') { ... } # MyApp::ActionRole::Moo - sub baz : Local Does('+MyApp::ActionRole::Moo') { ... } + sub foo : Local Does('Buzz') { ... } # Catalyst::ActionRole:: + sub bar : Local Does('~Buzz') { ... } # MyApp::ActionRole::Buzz + sub baz : Local Does('+MyApp::ActionRole::Buzz') { ... } =head2 GET diff --git a/lib/Catalyst/DispatchType/Chained.pm b/lib/Catalyst/DispatchType/Chained.pm index cb1dfed..138727c 100644 --- a/lib/Catalyst/DispatchType/Chained.pm +++ b/lib/Catalyst/DispatchType/Chained.pm @@ -702,7 +702,7 @@ controller. For Example: # in MyApp::Controller::Foo sub bar : Chained CaptureArgs(1) { ... } - # in MyApp::Controller::Foo::Moo + # in MyApp::Controller::Foo::Bar sub bar : ChainedParent Args(1) { ... } This builds a chain like C. @@ -715,7 +715,7 @@ parts of the path (separated by C) this action wants to capture as its arguments. If it doesn't expect any, just specify C<:CaptureArgs(0)>. The captures get passed to the action's C<@_> right after the context, but you can also find them as array references in -C<$c-Erequest-Ecaptures-E[$level]>. The C<$level> is the +C<< $c->request->captures->[$level] >>. The C<$level> is the level of the action in the chain that captured the parts of the path. An action that is part of a chain (that is, one that has a C<:Chained> @@ -764,7 +764,7 @@ of path parts after the endpoint. Just as with C<:CaptureArgs>, the arguments get passed to the action in C<@_> after the context object. They can also be reached through -C<$c-Erequest-Earguments>. +C<< $c->request->arguments >>. You should see 'Args' in L for more details on using type constraints in your Args declarations. diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm index 150c269..70f49fb 100644 --- a/lib/Catalyst/Engine.pm +++ b/lib/Catalyst/Engine.pm @@ -592,22 +592,28 @@ sub prepare_query_parameters { ? $env->{QUERY_STRING} : ''; - # Check for keywords (no = signs) - # (yes, index() is faster than a regex :)) - if ( index( $query_string, '=' ) < 0 ) { - my $keywords = $self->unescape_uri($query_string); - $keywords = $decoder->($keywords); - $c->request->query_keywords($keywords); - return; - } - $query_string =~ s/\A[&;]+//; - my $p = Hash::MultiValue->new( - map { defined $_ ? $decoder->($self->unescape_uri($_)) : $_ } - map { ( split /=/, $_, 2 )[0,1] } # slice forces two elements - split /[&;]+/, $query_string - ); + my @unsplit_pairs = split /[&;]+/, $query_string; + my $p = Hash::MultiValue->new(); + + my $is_first_pair = 1; + for my $pair (@unsplit_pairs) { + my ($name, $value) + = map { defined $_ ? $decoder->($self->unescape_uri($_)) : $_ } + ( split /=/, $pair, 2 )[0,1]; # slice forces two elements + + if ($is_first_pair) { + # If the first pair has no equal sign, then it means the isindex + # flag is set. + $c->request->query_keywords($name) unless defined $value; + + $is_first_pair = 0; + } + + $p->add( $name => $value ); + } + $c->encoding($old_encoding) if $old_encoding; $c->request->query_parameters( $c->request->_use_hash_multivalue ? $p : $p->mixed ); diff --git a/lib/Catalyst/Exception/Basic.pm b/lib/Catalyst/Exception/Basic.pm index 713bb5f..253b6a8 100644 --- a/lib/Catalyst/Exception/Basic.pm +++ b/lib/Catalyst/Exception/Basic.pm @@ -1,6 +1,8 @@ package Catalyst::Exception::Basic; -use MooseX::Role::WithOverloading; +use Moose::Role; +use if !eval { require Moose; Moose->VERSION('2.1300') }, + 'MooseX::Role::WithOverloading'; use Carp; use namespace::clean -except => 'meta'; diff --git a/lib/Catalyst/Exception/Interface.pm b/lib/Catalyst/Exception/Interface.pm index aae67f2..73e4cc0 100644 --- a/lib/Catalyst/Exception/Interface.pm +++ b/lib/Catalyst/Exception/Interface.pm @@ -1,6 +1,8 @@ package Catalyst::Exception::Interface; -use MooseX::Role::WithOverloading; +use Moose::Role; +use if !eval { require Moose; Moose->VERSION('2.1300') }, + 'MooseX::Role::WithOverloading'; use namespace::clean -except => 'meta'; use overload diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm index 1306b94..e4cb5c2 100644 --- a/lib/Catalyst/Request.pm +++ b/lib/Catalyst/Request.pm @@ -123,7 +123,7 @@ sub _build_body_data { # 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'); + return unless ($self->method eq 'POST' || $self->method eq 'PUT' || $self->method eq 'PATCH'); my ($match) = grep { $content_type =~/$_/i } keys(%{$self->data_handlers}); diff --git a/lib/Catalyst/Runtime.pm b/lib/Catalyst/Runtime.pm index 6f5a59f..24ddcc7 100644 --- a/lib/Catalyst/Runtime.pm +++ b/lib/Catalyst/Runtime.pm @@ -7,7 +7,7 @@ BEGIN { require 5.008003; } # Remember to update this in Catalyst as well! -our $VERSION = '5.90114'; +our $VERSION = '5.90115'; $VERSION = eval $VERSION if $VERSION =~ /_/; # numify for warning-free dev releases =head1 NAME diff --git a/lib/Catalyst/Test.pm b/lib/Catalyst/Test.pm index e121b35..ef2ff18 100644 --- a/lib/Catalyst/Test.pm +++ b/lib/Catalyst/Test.pm @@ -256,6 +256,15 @@ header configuration; currently only supports setting 'host' value. my $res = request('foo/bar?test=1'); my $virtual_res = request('foo/bar?test=1', {host => 'virtualhost.com'}); +Alternately, you can pass in an L object to set arbitrary +request headers. + + my $res = request(GET '/foo/bar', + X-Foo => 'Bar', + Authorization => 'Bearer JWT_HERE', + ... + ); + =head2 ($res, $c) = ctx_request( ... ); Works exactly like L, except it also returns the Catalyst context object, diff --git a/lib/Catalyst/Utils.pm b/lib/Catalyst/Utils.pm index 9fb1e92..fc877d4 100644 --- a/lib/Catalyst/Utils.pm +++ b/lib/Catalyst/Utils.pm @@ -413,11 +413,14 @@ sub term_width { } else { warn "There was an error trying to detect your terminal size: $@\n"; } + }; + + unless ($width) { warn 'Trouble trying to detect your terminal size, looking at $ENV{COLUMNS}'."\n"; $width = $ENV{COLUMNS} if exists($ENV{COLUMNS}) && $ENV{COLUMNS} =~ m/^\d+$/; - }; + } do { warn "Cannot determine desired terminal width, using default of 80 columns\n"; diff --git a/t/abort-chain-1.t b/t/abort-chain-1.t new file mode 100644 index 0000000..dba8593 --- /dev/null +++ b/t/abort-chain-1.t @@ -0,0 +1,50 @@ +#!perl + +use strict; +use warnings; +use Test::More tests => 1; +use HTTP::Request::Common; + +BEGIN { + package TestApp::Controller::Root; + $INC{'TestApp/Controller/Root.pm'} = __FILE__; + use Moose; + use MooseX::MethodAttributes; + extends 'Catalyst::Controller'; + + has counter => (is => 'rw', isa => 'Int', default => sub { 0 }); + sub increment { + my $self = shift; + $self->counter($self->counter + 1); + } + sub root :Chained('/') :PathPart('') :CaptureArgs(0) { + my ($self, $c, $arg) = @_; + die "Died in root"; + } + sub main :Chained('root') :PathPart('') :Args(0) { + my ($self, $c, $arg) = @_; + $self->increment; + die "Died in main"; + } + sub hits :Path('hits') :Args(0) { + my ($self, $c, $arg) = @_; + $c->response->body($self->counter); + } + __PACKAGE__->config(namespace => ''); +} +{ + package TestApp; + $INC{'TestApp.pm'} = __FILE__; + use Catalyst; + __PACKAGE__->setup; +} + +use Catalyst::Test 'TestApp'; + +{ + my $res = request('/'); +} +{ + my $res = request('/hits'); + is $res->content, 0, "main action not touched on crash with no explicit setting"; +} diff --git a/t/abort-chain-2.t b/t/abort-chain-2.t new file mode 100644 index 0000000..370868c --- /dev/null +++ b/t/abort-chain-2.t @@ -0,0 +1,51 @@ +#!perl + +use strict; +use warnings; +use Test::More tests => 1; +use HTTP::Request::Common; + +BEGIN { + package TestApp::Controller::Root; + $INC{'TestApp/Controller/Root.pm'} = __FILE__; + use Moose; + use MooseX::MethodAttributes; + extends 'Catalyst::Controller'; + + has counter => (is => 'rw', isa => 'Int', default => sub { 0 }); + sub increment { + my $self = shift; + $self->counter($self->counter + 1); + } + sub root :Chained('/') :PathPart('') :CaptureArgs(0) { + my ($self, $c, $arg) = @_; + die "Died in root"; + } + sub main :Chained('root') :PathPart('') :Args(0) { + my ($self, $c, $arg) = @_; + $self->increment; + die "Died in main"; + } + sub hits :Path('hits') :Args(0) { + my ($self, $c, $arg) = @_; + $c->response->body($self->counter); + } + __PACKAGE__->config(namespace => ''); +} +{ + package TestApp; + $INC{'TestApp.pm'} = __FILE__; + use Catalyst; + __PACKAGE__->config(abort_chain_on_error_fix => 1); + __PACKAGE__->setup; +} + +use Catalyst::Test 'TestApp'; + +{ + my $res = request('/'); +} +{ + my $res = request('/hits'); + is $res->content, 0, "main action not touched on crash with explicit setting to true"; +} diff --git a/t/abort-chain-3.t b/t/abort-chain-3.t new file mode 100644 index 0000000..1b0f928 --- /dev/null +++ b/t/abort-chain-3.t @@ -0,0 +1,51 @@ +#!perl + +use strict; +use warnings; +use Test::More tests => 1; +use HTTP::Request::Common; + +BEGIN { + package TestApp::Controller::Root; + $INC{'TestApp/Controller/Root.pm'} = __FILE__; + use Moose; + use MooseX::MethodAttributes; + extends 'Catalyst::Controller'; + + has counter => (is => 'rw', isa => 'Int', default => sub { 0 }); + sub increment { + my $self = shift; + $self->counter($self->counter + 1); + } + sub root :Chained('/') :PathPart('') :CaptureArgs(0) { + my ($self, $c, $arg) = @_; + die "Died in root"; + } + sub main :Chained('root') :PathPart('') :Args(0) { + my ($self, $c, $arg) = @_; + $self->increment; + die "Died in main"; + } + sub hits :Path('hits') :Args(0) { + my ($self, $c, $arg) = @_; + $c->response->body($self->counter); + } + __PACKAGE__->config(namespace => ''); +} +{ + package TestApp; + $INC{'TestApp.pm'} = __FILE__; + use Catalyst; + __PACKAGE__->config(abort_chain_on_error_fix => 0); + __PACKAGE__->setup; +} + +use Catalyst::Test 'TestApp'; + +{ + my $res = request('/'); +} +{ + my $res = request('/hits'); + is $res->content, 1, "main action performed on crash with explicit setting to false"; +} diff --git a/t/aggregate/live_component_controller_actionroles.t b/t/aggregate/live_component_controller_actionroles.t index 0bf1b0c..a78fbec 100644 --- a/t/aggregate/live_component_controller_actionroles.t +++ b/t/aggregate/live_component_controller_actionroles.t @@ -8,9 +8,9 @@ use lib "$FindBin::Bin/../lib"; use Catalyst::Test 'TestApp'; my %roles = ( - foo => 'TestApp::ActionRole::Moo', - bar => 'TestApp::ActionRole::Moo', - baz => 'Moo', + foo => 'TestApp::ActionRole::Guff', + bar => 'TestApp::ActionRole::Guff', + baz => 'Guff', quux => 'Catalyst::ActionRole::Zoo', ); @@ -24,9 +24,9 @@ while (my ($path, $role) = each %roles) { { my $resp = request("/actionroles/corge"); ok($resp->is_success); - is($resp->content, 'TestApp::ActionRole::Moo'); + is($resp->content, 'TestApp::ActionRole::Guff'); is($resp->header('X-Affe'), 'Tiger'); - is($resp->header('X-Action-After'), 'moo'); + is($resp->header('X-Action-After'), 'moo'); } { my $resp = request("/actionroles/frew"); diff --git a/t/aggregate/live_engine_request_parameters.t b/t/aggregate/live_engine_request_parameters.t index c7b4611..d2a91f0 100644 --- a/t/aggregate/live_engine_request_parameters.t +++ b/t/aggregate/live_engine_request_parameters.t @@ -4,7 +4,7 @@ use warnings; use FindBin; use lib "$FindBin::Bin/../lib"; -use Test::More tests => 54; +use Test::More tests => 56; use Catalyst::Test 'TestApp'; use Catalyst::Request; @@ -115,14 +115,21 @@ use HTTP::Request::Common; { my $creq; - my $parameters = { + my $body_parameters = { a => 1, blank => '', }; + my $query_parameters = { + 'query string' => undef + }; + my $parameters = { + %$body_parameters, + %$query_parameters + }; my $request = POST( 'http://localhost/dump/request/a/b?query+string', - 'Content' => $parameters, + 'Content' => $body_parameters, 'Content-Type' => 'application/x-www-form-urlencoded' ); @@ -130,6 +137,8 @@ use HTTP::Request::Common; ok( eval '$creq = ' . $response->content, 'Unserialize Catalyst::Request' ); is( $creq->uri->query, 'query+string', 'Catalyst::Request POST query_string' ); is( $creq->query_keywords, 'query string', 'Catalyst::Request query_keywords' ); + is_deeply( $creq->query_parameters, $query_parameters, 'Catalyst::Request query_parameters' ); + is_deeply( $creq->body_parameters, $body_parameters, 'Catalyst::Request body_parameters' ); is_deeply( $creq->parameters, $parameters, 'Catalyst::Request parameters' ); ok( $response = request('http://localhost/dump/request/a/b?x=1&y=1&z=1'), 'Request' ); diff --git a/t/aggregate/unit_core_log_autoflush.t b/t/aggregate/unit_core_log_autoflush.t index 530d475..bb2aae5 100755 --- a/t/aggregate/unit_core_log_autoflush.t +++ b/t/aggregate/unit_core_log_autoflush.t @@ -35,7 +35,7 @@ like $MESSAGES[0], qr/^\[info\] hello there!$/, { - package Catalyst::Log::Subclass; + package Catalyst::Log::SubclassAutoflush; use base qw/Catalyst::Log/; sub _send_to_log { @@ -47,9 +47,9 @@ like $MESSAGES[0], qr/^\[info\] hello there!$/, @MESSAGES = (); # clear the message log -my $SUBCLASS = 'Catalyst::Log::Subclass'; +my $SUBCLASS = 'Catalyst::Log::SubclassAutoflush'; can_ok $SUBCLASS, 'new'; -ok $log = Catalyst::Log::Subclass->new, +ok $log = $SUBCLASS->new, '... and the log subclass constructor should return a new object'; isa_ok $log, $SUBCLASS, '... and the object it returns'; isa_ok $log, $LOG, '... and it also'; diff --git a/t/aggregate/unit_core_script_server-without_modules.t b/t/aggregate/unit_core_script_server-without_modules.t index 0fdaa87..d6bbcaf 100644 --- a/t/aggregate/unit_core_script_server-without_modules.t +++ b/t/aggregate/unit_core_script_server-without_modules.t @@ -10,26 +10,19 @@ BEGIN { $ENV{PACKAGE_STASH_IMPLEMENTATION} = 'PP' if $] < '5.008007' } use Test::More; use Try::Tiny; -plan skip_all => "Need Test::Without::Module for this test" - unless try { require Test::Without::Module; 1 }; - -Test::Without::Module->import(qw( +my %hidden = map { (my $m = "$_.pm") =~ s{::}{/}g; $m => 1 } qw( Starman::Server Plack::Handler::Starman MooseX::Daemonize MooseX::Daemonize::Pid::File MooseX::Daemonize::Core -)); - -require "$Bin/../aggregate/unit_core_script_server.t"; +); +local @INC = (sub { + return unless exists $hidden{$_[1]}; + die "Can't locate $_[1] in \@INC (hidden)\n"; +}, @INC); -Test::Without::Module->unimport(qw( - Starman::Server - Plack::Handler::Starman - MooseX::Daemonize - MooseX::Daemonize::Pid::File - MooseX::Daemonize::Core -)); +do "$Bin/../aggregate/unit_core_script_server.t" + or die $@ || 'test returned false'; 1; - diff --git a/t/aggregate/unit_core_uri_for.t b/t/aggregate/unit_core_uri_for.t index ab256f0..887aa4a 100644 --- a/t/aggregate/unit_core_uri_for.t +++ b/t/aggregate/unit_core_uri_for.t @@ -60,6 +60,12 @@ is( ); is( + Catalyst::uri_for( $context, '/bar', 'with space', { 'also with' => 'space here' })->as_string, + 'http://127.0.0.1/foo/bar/with%20space?also+with=space+here', + 'Spaces encoded correctly' +); + +is( Catalyst::uri_for( $context, '/bar#fragment', { param1 => 'value1' } )->as_string, 'http://127.0.0.1/foo/bar?param1=value1#fragment', 'URI for path with fragment and query params 1' @@ -127,6 +133,12 @@ is( 'Plus is not encoded, called with only class name' ); +is( + Catalyst::uri_for( 'TestApp', '/bar', 'with space', { 'also with' => 'space here' })->as_string, + '/bar/with%20space?also+with=space+here', + 'Spaces encoded correctly, called with only class name' +); + TODO: { local $TODO = 'broken by 5.7008'; is( diff --git a/t/author/spelling.t b/t/author/spelling.t index c90f51a..e5b01bc 100644 --- a/t/author/spelling.t +++ b/t/author/spelling.t @@ -60,8 +60,10 @@ add_stopwords(qw( Marrandi McWhirter Milicevic + Mischa Miyagawa Montes + Napiorkowski Naughton Oleg Ragwitz @@ -79,11 +81,13 @@ add_stopwords(qw( Sedlacek Sheidlower SpiceMan + Spiegelmock Styn Szilakszi Tatsuhiko Ulf Upasana + Venters Vilain Viljo Wardley @@ -121,7 +125,6 @@ add_stopwords(qw( miyagawa mst multipart - Napiorkowski naughton ningu nothingmuch @@ -130,6 +133,7 @@ add_stopwords(qw( phaylon rafl rainboxx + revmischa sri szbalint uploadtmp diff --git a/t/class_traits.t b/t/class_traits.t index edccea2..d6a0c50 100644 --- a/t/class_traits.t +++ b/t/class_traits.t @@ -4,6 +4,17 @@ use Test::More; use Class::MOP; BEGIN { + my %hidden = map { (my $m = "$_.pm") =~ s{::}{/}g; $m => 1 } qw( + Foo + Bar + ); + unshift @INC, sub { + return unless exists $hidden{$_[1]}; + die "Can't locate $_[1] in \@INC (hidden)\n"; + }; +} + +BEGIN { package TestRole; $INC{'TestRole'} = __FILE__; use Moose::Role; @@ -30,7 +41,7 @@ BEGIN { sub c { 'c' } - package TestApp::TraitFor::Response::Bar; + package TestApp::TraitFor::Response::Bar; $INC{'TestApp/TraitFor/Response/Bar.pm'} = __FILE__; use Moose::Role; diff --git a/t/class_traits_CAR_bug.t b/t/class_traits_CAR_bug.t index d160e73..cc57bc1 100644 --- a/t/class_traits_CAR_bug.t +++ b/t/class_traits_CAR_bug.t @@ -11,6 +11,17 @@ BEGIN { } BEGIN { + my %hidden = map { (my $m = "$_.pm") =~ s{::}{/}g; $m => 1 } qw( + Foo + Bar + ); + unshift @INC, sub { + return unless exists $hidden{$_[1]}; + die "Can't locate $_[1] in \@INC (hidden)\n"; + }; +} + +BEGIN { package TestRole; $INC{'TestRole'} = __FILE__; use Moose::Role; diff --git a/t/lib/Catalyst/ActionRole/Moo.pm b/t/lib/Catalyst/ActionRole/Guff.pm similarity index 80% rename from t/lib/Catalyst/ActionRole/Moo.pm rename to t/lib/Catalyst/ActionRole/Guff.pm index 3d4aa51..4f8e046 100644 --- a/t/lib/Catalyst/ActionRole/Moo.pm +++ b/t/lib/Catalyst/ActionRole/Guff.pm @@ -1,4 +1,4 @@ -package Catalyst::ActionRole::Moo; +package Catalyst::ActionRole::Guff; use Moose::Role; diff --git a/t/lib/Moo.pm b/t/lib/Guff.pm similarity index 91% rename from t/lib/Moo.pm rename to t/lib/Guff.pm index c28806a..16e558e 100644 --- a/t/lib/Moo.pm +++ b/t/lib/Guff.pm @@ -1,4 +1,4 @@ -package Moo; +package Guff; use Moose::Role; diff --git a/t/lib/TestApp/ActionRole/Moo.pm b/t/lib/TestApp/ActionRole/Guff.pm similarity index 77% rename from t/lib/TestApp/ActionRole/Moo.pm rename to t/lib/TestApp/ActionRole/Guff.pm index d0fd290..3e8fdd9 100644 --- a/t/lib/TestApp/ActionRole/Moo.pm +++ b/t/lib/TestApp/ActionRole/Guff.pm @@ -1,4 +1,4 @@ -package TestApp::ActionRole::Moo; +package TestApp::ActionRole::Guff; use Moose::Role; diff --git a/t/lib/TestApp/Controller/ActionRoles.pm b/t/lib/TestApp/Controller/ActionRoles.pm index 37c24f9..69206bb 100644 --- a/t/lib/TestApp/Controller/ActionRoles.pm +++ b/t/lib/TestApp/Controller/ActionRoles.pm @@ -11,12 +11,12 @@ __PACKAGE__->config( }, ); -sub foo : Local Does('Moo') {} -sub bar : Local Does('~Moo') {} -sub baz : Local Does('+Moo') {} +sub foo : Local Does('Guff') {} +sub bar : Local Does('~Guff') {} +sub baz : Local Does('+Guff') {} sub quux : Local Does('Zoo') {} -sub corge : Local Does('Moo') ActionClass('TestAfter') { +sub corge : Local Does('Guff') ActionClass('TestAfter') { my ($self, $ctx) = @_; $ctx->stash(after_message => 'moo'); } diff --git a/t/query_keywords_and_parameters.t b/t/query_keywords_and_parameters.t new file mode 100644 index 0000000..27e598b --- /dev/null +++ b/t/query_keywords_and_parameters.t @@ -0,0 +1,84 @@ +use warnings; +use strict; +use Test::More; + +# Test case for reported issue when an action consumes JSON but a +# POST sends nothing we get a hard error + +{ + package MyApp::Controller::Root; + $INC{'MyApp/Controller/Root.pm'} = __FILE__; + + use base 'Catalyst::Controller'; + + sub bar :Local Args(0) GET { + my( $self, $c ) = @_; + } + + package MyApp; + use Catalyst; + MyApp->setup; +} + +use HTTP::Request::Common; +use Catalyst::Test 'MyApp'; + +# These tests assume that the decoding that occurs for the query string follows +# the payload decoding algorithm described here: +# https://www.w3.org/TR/html5/forms.html#url-encoded-form-data + +{ + ok my $req = GET 'root/bar'; + + my ($res, $c) = ctx_request($req); + + ok !defined($c->req->query_keywords), 'query_keywords is not defined when no ?'; + is_deeply $c->req->query_parameters, {}, 'query_parameters defined, but empty for no ?'; +} + + +{ + ok my $req = GET 'root/bar?'; + + my ($res, $c) = ctx_request($req); + + ok !defined $c->req->query_keywords, 'query_keywords is not defined when ? with empty query string'; + is_deeply $c->req->query_parameters, {}, 'query_parameters defined, but empty with empty query string'; +} + + +{ + ok my $req = GET 'root/bar?a=b'; + + my ($res, $c) = ctx_request($req); + + ok !defined($c->req->query_keywords), 'query_keywords undefined when isindex not set'; + is_deeply $c->req->query_parameters, { a => 'b' }, 'query_parameters defined for ?a=b'; +} + + +{ + ok my $req = GET 'root/bar?x'; + + my ($res, $c) = ctx_request($req); + + is $c->req->query_keywords, 'x', 'query_keywords defined for ?x'; + # The algorithm reads like 'x' should be treated as a value, not a name. + # Perl does not support undef as a hash key. I feel this would be the best + # alternative as isindex is moving towards complete deprecation. + is_deeply $c->req->query_parameters, { 'x' => undef }, 'query_parameters defined for ?x'; +} + + +{ + ok my $req = GET 'root/bar?x&a=b'; + + my ($res, $c) = ctx_request($req); + + is $c->req->query_keywords, 'x', 'query_keywords defined for ?x&a=b'; + # See comment above about the 'query_parameters defined for ?x' test case. + is_deeply $c->req->query_parameters, { 'x' => undef, a => 'b' }, 'query_parameters defined for ?x&a=b'; +} + + +done_testing();