diff options
Diffstat (limited to 'actionpack')
85 files changed, 1221 insertions, 1301 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 9a13e88ec9..be911b147c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,765 +1,2 @@ -* Add `ActionController::Parameters#dig` on Ruby 2.3 and greater, which - behaves the same as `Hash#dig`. - *Sean Griffin* - -* Add request headers in the payload of the `start_processing.action_controller` - and `process_action.action_controller` notifications. - - *Gareth du Plooy* - -* Add `action_dispatch_integration_test` load hook. The hook can be used to - extend `ActionDispatch::IntegrationTest` once it has been loaded. - - *Yuichiro Kaneko* - -* Update default rendering policies when the controller action did - not explicitly indicate a response. - - For API controllers, the implicit render always renders "204 No Content" - and does not account for any templates. - - For other controllers, the following conditions are checked: - - First, if a template exists for the controller action, it is rendered. - This template lookup takes into account the action name, locales, format, - variant, template handlers, etc. (see +render+ for details). - - Second, if other templates exist for the controller action but is not in - the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> - is raised. The list of available templates is assumed to be a complete - enumeration of all the possible formats (or variants, etc.); that is, - having only HTML and JSON templates indicate that the controller action is - not meant to handle XML requests. - - Third, if the current request is an "interactive" browser request (the user - navigated here by entering the URL in the address bar, submiting a form, - clicking on a link, etc. as opposed to an XHR or non-browser API request), - <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error - message. - - Finally, it falls back to the same "204 No Content" behavior as API controllers. - - *Godfrey Chan*, *Jon Moss*, *Kasper Timm Hansen*, *Mike Clark*, *Matthew Draper* - -## Rails 5.0.0.beta3 (February 24, 2016) ## - -* Add application/gzip as a default mime type. - - *Mehmet Emin İNAÇ* - -* Add request encoding and response parsing to integration tests. - - What previously was: - - ```ruby - require 'test_helper' - - class ApiTest < ActionDispatch::IntegrationTest - test 'creates articles' do - assert_difference -> { Article.count } do - post articles_path(format: :json), - params: { article: { title: 'Ahoy!' } }.to_json, - headers: { 'Content-Type' => 'application/json' } - end - - assert_equal({ 'id' => Article.last.id, 'title' => 'Ahoy!' }, JSON.parse(response.body)) - end - end - ``` - - Can now be written as: - - ```ruby - require 'test_helper' - - class ApiTest < ActionDispatch::IntegrationTest - test 'creates articles' do - assert_difference -> { Article.count } do - post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json - end - - assert_equal({ 'id' => Article.last.id, 'title' => 'Ahoy!' }, response.parsed_body) - end - end - ``` - - Passing `as: :json` to integration test request helpers will set the format, - content type and encode the parameters as JSON. - - Then on the response side, `parsed_body` will parse the body according to the - content type the response has. - - Currently JSON is the only supported MIME type. Add your own with - `ActionDispatch::IntegrationTest.register_encoder`. - - *Kasper Timm Hansen* - -* Add image/svg+xml as a default mime type. - - *DHH* - -## Rails 5.0.0.beta2 (February 01, 2016) ## - -* Add `-g` and `-c` options to `bin/rails routes`. These options return the url `name`, `verb` and - `path` field that match the pattern or match a specific controller. - - Deprecate `CONTROLLER` env variable in `bin/rails routes`. - - See #18902. - - *Anton Davydov* & *Vipul A M* - -* Response etags to always be weak: Prefixes 'W/' to value returned by - `ActionDispatch::Http::Cache::Response#etag=`, such that etags set in - `fresh_when` and `stale?` are weak. - - Fixes #17556. - - *Abhishek Yadav* - -* Provide the name of HTTP Status code in assertions. - - *Sean Collins* - -* More explicit error message when running `rake routes`. `CONTROLLER` argument - can now be supplied in different ways: - `Rails::WelcomeController`, `Rails::Welcome`, `rails/welcome`. - - Fixes #22918. - - *Edouard Chin* - -* Allow `ActionController::Parameters` instances as an argument to URL - helper methods. An `ArgumentError` will be raised if the passed parameters - are not secure. - - Fixes #22832. - - *Prathamesh Sonpatki* - -* Add option for per-form CSRF tokens. - - *Greg Ose & Ben Toews* - -* Add tests and documentation for `ActionController::Renderers::use_renderers`. - - *Benjamin Fleischer* - -* Fix `ActionController::Parameters#convert_parameters_to_hashes` to return filtered - or unfiltered values based on from where it is called, `to_h` or `to_unsafe_h` - respectively. - - Fixes #22841. - - *Prathamesh Sonpatki* - -* Add `ActionController::Parameters#include?` - - *Justin Coyne* - -## Rails 5.0.0.beta1 (December 18, 2015) ## - -* Deprecate `redirect_to :back` in favor of `redirect_back`, which accepts a - required `fallback_location` argument, thus eliminating the possibility of a - `RedirectBackError`. - - *Derek Prior* - -* Add `redirect_back` method to `ActionController::Redirecting` to provide a - way to safely redirect to the `HTTP_REFERER` if it is present, falling back - to a provided redirect otherwise. - - *Derek Prior* - -* `ActionController::TestCase` will be moved to it's own gem in Rails 5.1 - - With the speed improvements made to `ActionDispatch::IntegrationTest` we no - longer need to keep two separate code bases for testing controllers. In - Rails 5.1 `ActionController::TestCase` will be deprecated and moved into a - gem outside of Rails source. - - This is a documentation deprecation so that going forward so new tests will use - `ActionDispatch::IntegrationTest` instead of `ActionController::TestCase`. - - *Eileen M. Uchitelle* - -* Add a `response_format` option to `ActionDispatch::DebugExceptions` - to configure the format of the response when errors occur in - development mode. - - If `response_format` is `:default` the debug info will be rendered - in an HTML page. In the other hand, if the provided value is `:api` - the debug info will be rendered in the original response format. - - *Jorge Bejar* - -* Change the `protect_from_forgery` prepend default to `false`. - - Per this comment - https://github.com/rails/rails/pull/18334#issuecomment-69234050 we want - `protect_from_forgery` to default to `prepend: false`. - - `protect_from_forgery` will now be inserted into the callback chain at the - point it is called in your application. This is useful for cases where you - want to `protect_from_forgery` after you perform required authentication - callbacks or other callbacks that are required to run after forgery protection. - - If you want `protect_from_forgery` callbacks to always run first, regardless of - position they are called in your application then you can add `prepend: true` - to your `protect_from_forgery` call. - - Example: - - ```ruby - protect_from_forgery prepend: true - ``` - - *Eileen M. Uchitelle* - -* In url_for, never append a question mark to the URL when the query string - is empty anyway. (It used to do that when called like `url_for(controller: - 'x', action: 'y', q: {})`.) - - *Paul Grayson* - -* Catch invalid UTF-8 querystring values and respond with BadRequest - - Check querystring params for invalid UTF-8 characters, and raise an - ActionController::BadRequest error if present. Previously these strings - would typically trigger errors further down the stack. - - *Grey Baker* - -* Parse RSS/ATOM responses as XML, not HTML. - - *Alexander Kaupanin* - -* Show helpful message in `BadRequest` exceptions due to invalid path - parameter encodings. - - Fixes #21923. - - *Agis Anastasopoulos* - -* Add the ability of returning arbitrary headers to `ActionDispatch::Static`. - - Now ActionDispatch::Static can accept HTTP headers so that developers - will have control of returning arbitrary headers like - 'Access-Control-Allow-Origin' when a response is delivered. They can be - configured with `#config`: - - Example: - - config.public_file_server.headers = { - "Cache-Control" => "public, max-age=60", - "Access-Control-Allow-Origin" => "http://rubyonrails.org" - } - - *Yuki Nishijima* - -* Allow multiple `root` routes in same scope level. Example: - - Example: - - root 'blog#show', constraints: ->(req) { Hostname.blog_site?(req.host) } - root 'landing#show' - - *Rafael Sales* - -* Fix regression in mounted engine named routes generation for app deployed to - a subdirectory. `relative_url_root` was prepended to the path twice (e.g. - "/subdir/subdir/engine_path" instead of "/subdir/engine_path") - - Fixes #20920. Fixes #21459. - - *Matthew Erhard* - -* `ActionDispatch::Response#new` no longer applies default headers. If you want - default headers applied to the response object, then call - `ActionDispatch::Response.create`. This change only impacts people who are - directly constructing an `ActionDispatch::Response` object. - -* Accessing mime types via constants like `Mime::HTML` is deprecated. Please - change code like this: - - Mime::HTML - - To this: - - Mime[:html] - - This change is so that Rails will not manage a list of constants, and fixes - an issue where if a type isn't registered you could possibly get the wrong - object. - - `Mime[:html]` is available in older versions of Rails, too, so you can - safely change libraries and plugins and maintain compatibility with - multiple versions of Rails. - -* `url_for` does not modify its arguments when generating polymorphic URLs. - - *Bernerd Schaefer* - -* Make it easier to opt in to `config.force_ssl` and `config.ssl_options` by - making them less dangerous to try and easier to disable. - - SSL redirect: - * Move `:host` and `:port` options within `redirect: { … }`. Deprecate. - * Introduce `:status` and `:body` to customize the redirect response. - The 301 permanent default makes it difficult to test the redirect and - back out of it since browsers remember the 301. Test with a 302 or 307 - instead, then switch to 301 once you're confident that all is well. - - HTTP Strict Transport Security (HSTS): - * Shorter max-age. Shorten the default max-age from 1 year to 180 days, - the low end for https://www.ssllabs.com/ssltest/ grading and greater - than the 18-week minimum to qualify for browser preload lists. - * Disabling HSTS. Setting `hsts: false` now sets `hsts { expires: 0 }` - instead of omitting the header. Omitting does nothing to disable HSTS - since browsers hang on to your previous settings until they expire. - Sending `{ hsts: { expires: 0 }}` flushes out old browser settings and - actually disables HSTS: - http://tools.ietf.org/html/rfc6797#section-6.1.1 - * HSTS Preload. Introduce `preload: true` to set the `preload` flag, - indicating that your site may be included in browser preload lists, - including Chrome, Firefox, Safari, IE11, and Edge. Submit your site: - https://hstspreload.appspot.com - - *Jeremy Daer* - -* Update `ActionController::TestSession#fetch` to behave more like - `ActionDispatch::Request::Session#fetch` when using non-string keys. - - *Jeremy Friesen* - -* Using strings or symbols for middleware class names is deprecated. Convert - things like this: - - middleware.use "Foo::Bar" - - to this: - - middleware.use Foo::Bar - -* `ActionController::TestSession` now accepts a default value as well as - a block for generating a default value based off the key provided. - - This fixes calls to `session#fetch` in `ApplicationController` instances that - take more two arguments or a block from raising `ArgumentError: wrong - number of arguments (2 for 1)` when performing controller tests. - - *Matthew Gerrior* - -* Fix `ActionController::Parameters#fetch` overwriting `KeyError` returned by - default block. - - *Jonas Schuber Erlandsson*, *Roque Pinel* - -* `ActionController::Parameters` no longer inherits from - `HashWithIndifferentAccess` - - Inheriting from `HashWithIndifferentAccess` allowed users to call any - enumerable methods on `Parameters` object, resulting in a risk of losing the - `permitted?` status or even getting back a pure `Hash` object instead of - a `Parameters` object with proper sanitization. - - By not inheriting from `HashWithIndifferentAccess`, we are able to make - sure that all methods that are defined in `Parameters` object will return - a proper `Parameters` object with a correct `permitted?` flag. - - *Prem Sichanugrist* - -* Replaced `ActiveSupport::Concurrency::Latch` with `Concurrent::CountDownLatch` - from the concurrent-ruby gem. - - *Jerry D'Antonio* - -* Add ability to filter parameters based on parent keys. - - # matches {credit_card: {code: "xxxx"}} - # doesn't match {file: { code: "xxxx"}} - config.filter_parameters += [ "credit_card.code" ] - - See #13897. - - *Guillaume Malette* - -* Deprecate passing first parameter as `Hash` and default status code for `head` method. - - *Mehmet Emin İNAÇ* - -* Adds`Rack::Utils::ParameterTypeError` and `Rack::Utils::InvalidParameterError` - to the rescue_responses hash in `ExceptionWrapper` (Rack recommends - integrators serve 400s for both of these). - - *Grey Baker* - -* Add support for API only apps. - `ActionController::API` is added as a replacement of - `ActionController::Base` for this kind of applications. - - *Santiago Pastorino*, *Jorge Bejar* - -* Remove `assigns` and `assert_template`. Both methods have been extracted - into a gem at https://github.com/rails/rails-controller-testing. - - See #18950. - - *Alan Guo Xiang Tan* - -* `FileHandler` and `Static` middleware initializers accept `index` argument - to configure the directory index file name. Defaults to `index` (as in - `index.html`). - - See #20017. - - *Eliot Sykes* - -* Deprecate `:nothing` option for `render` method. - - *Mehmet Emin İNAÇ* - -* Fix `rake routes` not showing the right format when - nesting multiple routes. - - See #18373. - - *Ravil Bayramgalin* - -* Add ability to override default form builder for a controller. - - class AdminController < ApplicationController - default_form_builder AdminFormBuilder - end - - *Kevin McPhillips* - -* For actions with no corresponding templates, render `head :no_content` - instead of raising an error. This allows for slimmer API controller - methods that simply work, without needing further instructions. - - See #19036. - - *Stephen Bussey* - -* Provide friendlier access to request variants. - - request.variant = :phone - request.variant.phone? # true - request.variant.tablet? # false - - request.variant = [:phone, :tablet] - request.variant.phone? # true - request.variant.desktop? # false - request.variant.any?(:phone, :desktop) # true - request.variant.any?(:desktop, :watch) # false - - *George Claghorn* - -* Fix regression where a gzip file response would have a Content-type, - even when it was a 304 status code. - - See #19271. - - *Kohei Suzuki* - -* Fix handling of empty `X_FORWARDED_HOST` header in `raw_host_with_port`. - - Previously, an empty `X_FORWARDED_HOST` header would cause - `Actiondispatch::Http:URL.raw_host_with_port` to return `nil`, causing - `Actiondispatch::Http:URL.host` to raise a `NoMethodError`. - - *Adam Forsyth* - -* Allow `Bearer` as token-keyword in `Authorization-Header`. - - Additionally to `Token`, the keyword `Bearer` is acceptable as a keyword - for the auth-token. The `Bearer` keyword is described in the original - OAuth RFC and used in libraries like Angular-JWT. - - See #19094. - - *Peter Schröder* - -* Drop request class from `RouteSet` constructor. - - If you would like to use a custom request class, please subclass and implement - the `request_class` method. - - *tenderlove@ruby-lang.org* - -* Fallback to `ENV['RAILS_RELATIVE_URL_ROOT']` in `url_for`. - - Fixed an issue where the `RAILS_RELATIVE_URL_ROOT` environment variable is not - prepended to the path when `url_for` is called. If `SCRIPT_NAME` (used by Rack) - is set, it takes precedence. - - Fixes #5122. - - *Yasyf Mohamedali* - -* Partitioning of routes is now done when the routes are being drawn. This - helps to decrease the time spent filtering the routes during the first request. - - *Guo Xiang Tan* - -* Fix regression in functional tests. Responses should have default headers - assigned. - - See #18423. - - *Jeremy Kemper*, *Yves Senn* - -* Deprecate `AbstractController#skip_action_callback` in favor of individual skip_callback methods - (which can be made to raise an error if no callback was removed). - - *Iain Beeston* - -* Alias the `ActionDispatch::Request#uuid` method to `ActionDispatch::Request#request_id`. - Due to implementation, `config.log_tags = [:request_id]` also works in substitute - for `config.log_tags = [:uuid]`. - - *David Ilizarov* - -* Change filter on /rails/info/routes to use an actual path regexp from rails - and not approximate javascript version. Oniguruma supports much more - extensive list of features than javascript regexp engine. - - Fixes #18402. - - *Ravil Bayramgalin* - -* Non-string authenticity tokens do not raise NoMethodError when decoding - the masked token. - - *Ville Lautanala* - -* Add `http_cache_forever` to Action Controller, so we can cache a response - that never gets expired. - - *arthurnn* - -* `ActionController#translate` supports symbols as shortcuts. - When a shortcut is given it also performs the lookup without the action - name. - - *Max Melentiev* - -* Expand `ActionController::ConditionalGet#fresh_when` and `stale?` to also - accept a collection of records as the first argument, so that the - following code can be written in a shorter form. - - # Before - def index - @articles = Article.all - fresh_when(etag: @articles, last_modified: @articles.maximum(:updated_at)) - end - - # After - def index - @articles = Article.all - fresh_when(@articles) - end - - *claudiob* - -* Explicitly ignored wildcard verbs when searching for HEAD routes before fallback - - Fixes an issue where a mounted rack app at root would intercept the HEAD - request causing an incorrect behavior during the fall back to GET requests. - - Example: - - draw do - get '/home' => 'test#index' - mount rack_app, at: '/' - end - head '/home' - assert_response :success - - In this case, a HEAD request runs through the routes the first time and fails - to match anything. Then, it runs through the list with the fallback and matches - `get '/home'`. The original behavior would match the rack app in the first pass. - - *Terence Sun* - -* Migrating xhr methods to keyword arguments syntax - in `ActionController::TestCase` and `ActionDispatch::Integration` - - Old syntax: - - xhr :get, :create, params: { id: 1 } - - New syntax example: - - get :create, params: { id: 1 }, xhr: true - - *Kir Shatrov* - -* Migrating to keyword arguments syntax in `ActionController::TestCase` and - `ActionDispatch::Integration` HTTP request methods. - - Example: - - post :create, params: { y: x }, session: { a: 'b' } - get :view, params: { id: 1 } - get :view, params: { id: 1 }, format: :json - - *Kir Shatrov* - -* Preserve default url options when generating URLs. - - Fixes an issue that would cause `default_url_options` to be lost when - generating URLs with fewer positional arguments than parameters in the - route definition. - - *Tekin Suleyman* - -* Deprecate `*_via_redirect` integration test methods. - - Use `follow_redirect!` manually after the request call for the same behavior. - - *Aditya Kapoor* - -* Add `ActionController::Renderer` to render arbitrary templates - outside controller actions. - - Its functionality is accessible through class methods `render` and - `renderer` of `ActionController::Base`. - - *Ravil Bayramgalin* - -* Support `:assigns` option when rendering with controllers/mailers. - - *Ravil Bayramgalin* - -* Default headers, removed in controller actions, are no longer reapplied on - the test response. - - *Jonas Baumann* - -* Deprecate all `*_filter` callbacks in favor of `*_action` callbacks. - - *Rafael Mendonça França* - -* Allow you to pass `prepend: false` to `protect_from_forgery` to have the - verification callback appended instead of prepended to the chain. - This allows you to let the verification step depend on prior callbacks. - - Example: - - class ApplicationController < ActionController::Base - before_action :authenticate - protect_from_forgery prepend: false, unless: -> { @authenticated_by.oauth? } - - private - def authenticate - if oauth_request? - # authenticate with oauth - @authenticated_by = 'oauth'.inquiry - else - # authenticate with cookies - @authenticated_by = 'cookie'.inquiry - end - end - end - - *Josef Šimánek* - -* Remove `ActionController::HideActions`. - - *Ravil Bayramgalin* - -* Remove `respond_to`/`respond_with` placeholder methods, this functionality - has been extracted to the `responders` gem. - - *Carlos Antonio da Silva* - -* Remove deprecated assertion files. - - *Rafael Mendonça França* - -* Remove deprecated usage of string keys in URL helpers. - - *Rafael Mendonça França* - -* Remove deprecated `only_path` option on `*_path` helpers. - - *Rafael Mendonça França* - -* Remove deprecated `NamedRouteCollection#helpers`. - - *Rafael Mendonça França* - -* Remove deprecated support to define routes with `:to` option that doesn't contain `#`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::Response#to_ary`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::Request#deep_munge`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::Http::Parameters#symbolized_path_parameters`. - - *Rafael Mendonça França* - -* Remove deprecated option `use_route` in controller tests. - - *Rafael Mendonça França* - -* Ensure `append_info_to_payload` is called even if an exception is raised. - - Fixes an issue where when an exception is raised in the request the additional - payload data is not available. - - See #14903. - - *Dieter Komendera*, *Margus Pärt* - -* Correctly rely on the response's status code to handle calls to `head`. - - *Robin Dupret* - -* Using `head` method returns empty response_body instead - of returning a single space " ". - - The old behavior was added as a workaround for a bug in an early - version of Safari, where the HTTP headers are not returned correctly - if the response body has a 0-length. This is been fixed since and - the workaround is no longer necessary. - - Fixes #18253. - - *Prathamesh Sonpatki* - -* Fix how polymorphic routes works with objects that implement `to_model`. - - *Travis Grathwell* - -* Stop converting empty arrays in `params` to `nil`. - - This behavior was introduced in response to CVE-2012-2660, CVE-2012-2694 - and CVE-2013-0155 - - ActiveRecord now issues a safe query when passing an empty array into - a where clause, so there is no longer a need to defend against this type - of input (any nils are still stripped from the array). - - *Chris Sinjakli* - -* Fixed usage of optional scopes in url helpers. - - *Alex Robbin* - -* Fixed handling of positional url helper arguments when `format: false`. - - Fixes #17819. - - *Andrew White*, *Tatiana Soukiassian* - -Please check [4-2-stable](https://github.com/rails/rails/blob/4-2-stable/actionpack/CHANGELOG.md) for previous changes. +Please check [5-0-stable](https://github.com/rails/rails/blob/5-0-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 37a269cffd..4a05c067a9 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -6,7 +6,6 @@ desc "Default Task" task :default => :test task :package -task "package:clean" # Run the unit tests Rake::TestTask.new do |t| diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 28d8bc3091..f912a72efe 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -13,7 +13,7 @@ Gem::Specification.new do |s| s.author = 'David Heinemeier Hansson' s.email = 'david@loudthinking.com' - s.homepage = 'http://www.rubyonrails.org' + s.homepage = 'http://rubyonrails.org' s.files = Dir['CHANGELOG.md', 'README.rdoc', 'MIT-LICENSE', 'lib/**/*'] s.require_path = 'lib' @@ -21,10 +21,10 @@ Gem::Specification.new do |s| s.add_dependency 'activesupport', version - s.add_dependency 'rack', '~> 2.x' + s.add_dependency 'rack', '~> 2.0' s.add_dependency 'rack-test', '~> 0.6.3' s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.2' - s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.5' + s.add_dependency 'rails-dom-testing', '~> 2.0' s.add_dependency 'actionview', version s.add_development_dependency 'activemodel', version diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 8edea0f52b..aa06f70433 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -1,13 +1,11 @@ require 'erubis' +require 'abstract_controller/error' require 'active_support/configurable' require 'active_support/descendants_tracker' require 'active_support/core_ext/module/anonymous' require 'active_support/core_ext/module/attr_internal' module AbstractController - class Error < StandardError #:nodoc: - end - # Raised when a non-existing controller action is triggered. class ActionNotFound < StandardError end @@ -78,7 +76,7 @@ module AbstractController end end - # action_methods are cached and there is sometimes need to refresh + # action_methods are cached and there is sometimes a need to refresh # them. ::clear_action_methods! allows you to do that, so next time # you run action_methods, they will be recalculated. def clear_action_methods! @@ -152,6 +150,13 @@ module AbstractController _find_action_name(action_name) end + # Tests if a response body is set. Used to determine if the + # +process_action+ callback needs to be terminated in + # +AbstractController::Callbacks+. + def performed? + response_body + end + # Returns true if the given controller is capable of rendering # a path. A subclass of +AbstractController::Base+ # may return false. An Email controller for example does not diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index d63ce9c1c3..3ef8da86fa 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -9,7 +9,7 @@ module AbstractController included do define_callbacks :process_action, - terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.response_body }, + terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.performed? }, skip_after_callbacks_if_terminated: true end diff --git a/actionpack/lib/abstract_controller/error.rb b/actionpack/lib/abstract_controller/error.rb new file mode 100644 index 0000000000..7fafce4dd4 --- /dev/null +++ b/actionpack/lib/abstract_controller/error.rb @@ -0,0 +1,4 @@ +module AbstractController + class Error < StandardError #:nodoc: + end +end diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index e765d73ce4..4ba2c26949 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -1,3 +1,4 @@ +require 'abstract_controller/error' require 'active_support/concern' require 'active_support/core_ext/class/attribute' require 'action_view' @@ -59,9 +60,7 @@ module AbstractController end DEFAULT_PROTECTED_INSTANCE_VARIABLES = Set.new %i( - @_action_name @_response_body @_formats @_prefixes @_config - @_view_context_class @_view_renderer @_lookup_context - @_routes @_db_runtime + @_action_name @_response_body @_formats @_prefixes ) # This method should return a hash with assigns. @@ -123,7 +122,7 @@ module AbstractController def _normalize_render(*args, &block) options = _normalize_args(*args, &block) #TODO: remove defined? when we restore AP <=> AV dependency - if defined?(request) && request.variant.present? + if defined?(request) && !request.nil? && request.variant.present? options[:variant] = request.variant end _normalize_options(options) diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index ff12705abe..6bbebb7b4c 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -14,22 +14,22 @@ module ActionController # flash, assets, and so on. This makes the entire controller stack thinner, # suitable for API applications. It doesn't mean you won't have such # features if you need them: they're all available for you to include in - # your application, they're just not part of the default API Controller stack. + # your application, they're just not part of the default API controller stack. # - # By default, only the ApplicationController in a \Rails application inherits - # from <tt>ActionController::API</tt>. All other controllers in turn inherit - # from ApplicationController. + # Normally, +ApplicationController+ is the only controller that inherits from + # <tt>ActionController::API</tt>. All other controllers in turn inherit from + # +ApplicationController+. # # A sample controller could look like this: # # class PostsController < ApplicationController # def index - # @posts = Post.all - # render json: @posts + # posts = Post.all + # render json: posts # end # end # - # Request, response and parameters objects all work the exact same way as + # Request, response, and parameters objects all work the exact same way as # <tt>ActionController::Base</tt>. # # == Renders @@ -37,18 +37,18 @@ module ActionController # The default API Controller stack includes all renderers, which means you # can use <tt>render :json</tt> and brothers freely in your controllers. Keep # in mind that templates are not going to be rendered, so you need to ensure - # your controller is calling either <tt>render</tt> or <tt>redirect</tt> in - # all actions, otherwise it will return 204 No Content response. + # your controller is calling either <tt>render</tt> or <tt>redirect_to</tt> in + # all actions, otherwise it will return 204 No Content. # # def show - # @post = Post.find(params[:id]) - # render json: @post + # post = Post.find(params[:id]) + # render json: post # end # # == Redirects # # Redirects are used to move from one action to another. You can use the - # <tt>redirect</tt> method in your controllers in the same way as + # <tt>redirect_to</tt> method in your controllers in the same way as in # <tt>ActionController::Base</tt>. For example: # # def create @@ -56,7 +56,7 @@ module ActionController # # do stuff here # end # - # == Adding new behavior + # == Adding New Behavior # # In some scenarios you may want to add back some functionality provided by # <tt>ActionController::Base</tt> that is not present by default in @@ -72,18 +72,19 @@ module ActionController # # class PostsController < ApplicationController # def index - # @posts = Post.all + # posts = Post.all # # respond_to do |format| - # format.json { render json: @posts } - # format.xml { render xml: @posts } + # format.json { render json: posts } + # format.xml { render xml: posts } # end # end # end # - # Quite straightforward. Make sure to check <tt>ActionController::Base</tt> - # available modules if you want to include any other functionality that is - # not provided by <tt>ActionController::API</tt> out of the box. + # Quite straightforward. Make sure to check the modules included in + # <tt>ActionController::Base</tt> if you want to use any other + # functionality that is not provided by <tt>ActionController::API</tt> + # out of the box. class API < Metal abstract! diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 04e5922ce8..251289d4bb 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -32,7 +32,7 @@ module ActionController # new post), it initiates a redirect instead. This redirect works by returning an external # "302 Moved" HTTP response that takes the user to the index action. # - # These two methods represent the two basic action archetypes used in Action Controllers. Get-and-show and do-and-redirect. + # These two methods represent the two basic action archetypes used in Action Controllers: Get-and-show and do-and-redirect. # Most actions are variations on these themes. # # == Requests @@ -51,8 +51,8 @@ module ActionController # == Parameters # # All request parameters, whether they come from a query string in the URL or form data submitted through a POST request are - # available through the params method which returns a hash. For example, an action that was performed through - # <tt>/posts?category=All&limit=5</tt> will include <tt>{ "category" => "All", "limit" => "5" }</tt> in params. + # available through the <tt>params</tt> method which returns a hash. For example, an action that was performed through + # <tt>/posts?category=All&limit=5</tt> will include <tt>{ "category" => "All", "limit" => "5" }</tt> in <tt>params</tt>. # # It's also possible to construct multi-dimensional parameter hashes by specifying keys using brackets, such as: # @@ -60,7 +60,7 @@ module ActionController # <input type="text" name="post[address]" value="hyacintvej"> # # A request stemming from a form holding these inputs will include <tt>{ "post" => { "name" => "david", "address" => "hyacintvej" } }</tt>. - # If the address input had been named <tt>post[address][street]</tt>, the params would have included + # If the address input had been named <tt>post[address][street]</tt>, the <tt>params</tt> would have included # <tt>{ "post" => { "address" => { "street" => "hyacintvej" } } }</tt>. There's no limit to the depth of the nesting. # # == Sessions @@ -229,7 +229,7 @@ module ActionController HttpAuthentication::Digest::ControllerMethods, HttpAuthentication::Token::ControllerMethods, - # Before callbacks should also be executed the earliest as possible, so + # Before callbacks should also be executed as early as possible, so # also include them at the bottom. AbstractController::Callbacks, @@ -251,9 +251,10 @@ module ActionController setup_renderer! # Define some internal variables that should not be propagated to the view. - PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + [ - :@_params, :@_response, :@_request, - :@_view_runtime, :@_stream, :@_url_options, :@_action_has_layout ] + PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + %i( + @_params @_response @_request @_config @_url_options @_action_has_layout @_view_context_class + @_view_renderer @_lookup_context @_routes @_view_runtime @_db_runtime @_helper_proxy + ) def _protected_ivars # :nodoc: PROTECTED_IVARS diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 35befc05e1..e21449f376 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -36,8 +36,23 @@ module ActionController # # === Parameters: # - # * <tt>:etag</tt>. - # * <tt>:last_modified</tt>. + # * <tt>:etag</tt> Sets a "weak" ETag validator on the response. See the + # +:weak_etag+ option. + # * <tt>:weak_etag</tt> Sets a "weak" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A weak ETag indicates semantic + # equivalence, not byte-for-byte equality, so they're good for caching + # HTML pages in browser caches. They can't be used for responses that + # must be byte-identical, like serving Range requests within a PDF file. + # * <tt>:strong_etag</tt> Sets a "strong" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A strong ETag implies exact + # equality: the response must match byte for byte. This is necessary for + # doing Range requests within a large video or PDF file, for example, or + # for compatibility with some CDNs that don't support weak ETags. + # * <tt>:last_modified</tt> Sets a "weak" last-update validator on the + # response. Subsequent requests that set If-Modified-Since may return a + # 304 Not Modified response if last_modified <= If-Modified-Since. # * <tt>:public</tt> By default the Cache-Control header is private, set this to # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current @@ -86,12 +101,16 @@ module ActionController # # before_action { fresh_when @article, template: 'widgets/show' } # - def fresh_when(object = nil, etag: object, last_modified: nil, public: false, template: nil) + def fresh_when(object = nil, etag: nil, weak_etag: nil, strong_etag: nil, last_modified: nil, public: false, template: nil) + weak_etag ||= etag || object unless strong_etag last_modified ||= object.try(:updated_at) || object.try(:maximum, :updated_at) - if etag || template - response.etag = combine_etags(etag: etag, last_modified: last_modified, - public: public, template: template) + if strong_etag + response.strong_etag = combine_etags strong_etag, + last_modified: last_modified, public: public, template: template + elsif weak_etag || template + response.weak_etag = combine_etags weak_etag, + last_modified: last_modified, public: public, template: template end response.last_modified = last_modified if last_modified @@ -107,8 +126,23 @@ module ActionController # # === Parameters: # - # * <tt>:etag</tt>. - # * <tt>:last_modified</tt>. + # * <tt>:etag</tt> Sets a "weak" ETag validator on the response. See the + # +:weak_etag+ option. + # * <tt>:weak_etag</tt> Sets a "weak" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A weak ETag indicates semantic + # equivalence, not byte-for-byte equality, so they're good for caching + # HTML pages in browser caches. They can't be used for responses that + # must be byte-identical, like serving Range requests within a PDF file. + # * <tt>:strong_etag</tt> Sets a "strong" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A strong ETag implies exact + # equality: the response must match byte for byte. This is necessary for + # doing Range requests within a large video or PDF file, for example, or + # for compatibility with some CDNs that don't support weak ETags. + # * <tt>:last_modified</tt> Sets a "weak" last-update validator on the + # response. Subsequent requests that set If-Modified-Since may return a + # 304 Not Modified response if last_modified <= If-Modified-Since. # * <tt>:public</tt> By default the Cache-Control header is private, set this to # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current @@ -180,8 +214,8 @@ module ActionController # super if stale? @article, template: 'widgets/show' # end # - def stale?(object = nil, etag: object, last_modified: nil, public: nil, template: nil) - fresh_when(object, etag: etag, last_modified: last_modified, public: public, template: template) + def stale?(object = nil, **freshness_kwargs) + fresh_when(object, **freshness_kwargs) !request.fresh?(response) end @@ -231,9 +265,8 @@ module ActionController end private - def combine_etags(options) - etags = etaggers.map { |etagger| instance_exec(options, &etagger) }.compact - etags.unshift options[:etag] + def combine_etags(validator, options) + [validator, *etaggers.map { |etagger| instance_exec(options, &etagger) }].compact end end end diff --git a/actionpack/lib/action_controller/metal/cookies.rb b/actionpack/lib/action_controller/metal/cookies.rb index f8efb2b076..44925641a1 100644 --- a/actionpack/lib/action_controller/metal/cookies.rb +++ b/actionpack/lib/action_controller/metal/cookies.rb @@ -3,7 +3,7 @@ module ActionController #:nodoc: extend ActiveSupport::Concern included do - helper_method :cookies + helper_method :cookies if defined?(helper_method) end private diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 957e7a3019..6cd6130032 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -25,14 +25,13 @@ module ActionController #:nodoc: # * <tt>:filename</tt> - suggests a filename for the browser to use. # Defaults to <tt>File.basename(path)</tt>. # * <tt>:type</tt> - specifies an HTTP content type. - # You can specify either a string or a symbol for a registered type register with - # <tt>Mime::Type.register</tt>, for example :json - # If omitted, type will be guessed from the file extension specified in <tt>:filename</tt>. - # If no content type is registered for the extension, default type 'application/octet-stream' will be used. + # You can specify either a string or a symbol for a registered type with <tt>Mime::Type.register</tt>, for example :json. + # If omitted, the type will be inferred from the file extension specified in <tt>:filename</tt>. + # If no content type is registered for the extension, the default type 'application/octet-stream' will be used. # * <tt>:disposition</tt> - specifies whether the file will be shown inline or downloaded. # Valid values are 'inline' and 'attachment' (default). # * <tt>:status</tt> - specifies the status code to send with the response. Defaults to 200. - # * <tt>:url_based_filename</tt> - set to +true+ if you want the browser guess the filename from + # * <tt>:url_based_filename</tt> - set to +true+ if you want the browser to guess the filename from # the URL, which is necessary for i18n filenames on certain browsers # (setting <tt>:filename</tt> overrides this option). # @@ -79,14 +78,14 @@ module ActionController #:nodoc: # <tt>render plain: data</tt>, but also allows you to specify whether # the browser should display the response as a file attachment (i.e. in a # download dialog) or as inline data. You may also set the content type, - # the apparent file name, and other things. + # the file name, and other things. # # Options: # * <tt>:filename</tt> - suggests a filename for the browser to use. - # * <tt>:type</tt> - specifies an HTTP content type. Defaults to 'application/octet-stream'. You can specify - # either a string or a symbol for a registered type register with <tt>Mime::Type.register</tt>, for example :json - # If omitted, type will be guessed from the file extension specified in <tt>:filename</tt>. - # If no content type is registered for the extension, default type 'application/octet-stream' will be used. + # * <tt>:type</tt> - specifies an HTTP content type. Defaults to 'application/octet-stream'. + # You can specify either a string or a symbol for a registered type with <tt>Mime::Type.register</tt>, for example :json. + # If omitted, type will be inferred from the file extension specified in <tt>:filename</tt>. + # If no content type is registered for the extension, the default type 'application/octet-stream' will be used. # * <tt>:disposition</tt> - specifies whether the file will be shown inline or downloaded. # Valid values are 'inline' and 'attachment' (default). # * <tt>:status</tt> - specifies the status code to send with the response. Defaults to 200. diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb index 669cf55bca..75ac996793 100644 --- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb +++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb @@ -39,8 +39,14 @@ module ActionController end end + # Pick the template digest to include in the ETag. If the +:template+ option + # is present, use the named template. If +:template+ is nil or absent, use + # the default controller/action template. If +:template+ is false, omit the + # template digest from the ETag. def pick_template_for_etag(options) - options.fetch(:template) { "#{controller_name}/#{action_name}" } + unless options[:template] == false + options[:template] || "#{controller_name}/#{action_name}" + end end def lookup_and_digest_template(template) diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index e31d65aac2..ea8e91ce24 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -2,17 +2,17 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/hash/slice' module ActionController - # This module provides a method which will redirect browser to use HTTPS + # This module provides a method which will redirect the browser to use HTTPS # protocol. This will ensure that user's sensitive information will be - # transferred safely over the internet. You _should_ always force browser + # transferred safely over the internet. You _should_ always force the browser # to use HTTPS when you're transferring sensitive information such as # user authentication, account information, or credit card information. # # Note that if you are really concerned about your application security, # you might consider using +config.force_ssl+ in your config file instead. # That will ensure all the data transferred via HTTPS protocol and prevent - # user from getting session hijacked when accessing the site under unsecured - # HTTP protocol. + # the user from getting their session hijacked when accessing the site over + # unsecured HTTP protocol. module ForceSSL extend ActiveSupport::Concern include AbstractController::Callbacks diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index d3853e2e83..295f0cb66f 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -5,7 +5,7 @@ module ActionController # # In addition to using the standard template helpers provided, creating custom helpers to # extract complicated logic or reusable functionality is strongly encouraged. By default, each controller - # will include all helpers. These helpers are only accessible on the controller through <tt>.helpers</tt> + # will include all helpers. These helpers are only accessible on the controller through <tt>#helpers</tt> # # In previous versions of \Rails the controller will include a helper which # matches the name of the controller, e.g., <tt>MyController</tt> will automatically @@ -71,7 +71,7 @@ module ActionController attrs.flatten.each { |attr| helper_method(attr, "#{attr}=") } end - # Provides a proxy to access helpers methods from outside the view. + # Provides a proxy to access helper methods from outside the view. def helpers @helper_proxy ||= begin proxy = ActionView::Base.new @@ -113,5 +113,10 @@ module ActionController all_helpers_from_path(helpers_path) end end + + # Provides a proxy to access helper methods from outside the view. + def helpers + @_helper_proxy ||= view_context + end end end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 35be6d9300..4639348509 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -310,9 +310,9 @@ module ActionController end # Might want a shorter timeout depending on whether the request - # is a PATCH, PUT, or POST, and if client is browser or web service. + # is a PATCH, PUT, or POST, and if the client is a browser or web service. # Can be much shorter if the Stale directive is implemented. This would - # allow a user to use new nonce without prompting user again for their + # allow a user to use new nonce without prompting the user again for their # username and password. def validate_nonce(secret_key, request, value, seconds_to_timeout=5*60) return false if value.nil? @@ -347,7 +347,12 @@ module ActionController # private # def authenticate # authenticate_or_request_with_http_token do |token, options| - # token == TOKEN + # # Compare the tokens in a time-constant manner, to mitigate + # # timing attacks. + # ActiveSupport::SecurityUtils.secure_compare( + # ::Digest::SHA256.hexdigest(token), + # ::Digest::SHA256.hexdigest(TOKEN) + # ) # end # end # end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 3a6f784507..6192fc0f9c 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,33 +1,31 @@ require 'active_support/core_ext/string/strip' module ActionController - # Handles implicit rendering for a controller action when it did not - # explicitly indicate an appropiate response via methods such as +render+, - # +respond_to+, +redirect+ or +head+. + # Handles implicit rendering for a controller action that does not + # explicitly respond with +render+, +respond_to+, +redirect+, or +head+. # - # For API controllers, the implicit render always renders "204 No Content" - # and does not account for any templates. + # For API controllers, the implicit response is always 204 No Content. # - # For other controllers, the following conditions are checked: + # For all other controllers, we use these heuristics to decide whether to + # render a template, raise an error for a missing template, or respond with + # 204 No Content: # - # First, if a template exists for the controller action, it is rendered. - # This template lookup takes into account the action name, locales, format, - # variant, template handlers, etc. (see +render+ for details). + # First, if we DO find a template, it's rendered. Template lookup accounts + # for the action name, locales, format, variant, template handlers, and more + # (see +render+ for details). # - # Second, if other templates exist for the controller action but is not in - # the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> - # is raised. The list of available templates is assumed to be a complete - # enumeration of all the possible formats (or variants, etc.); that is, - # having only HTML and JSON templates indicate that the controller action is - # not meant to handle XML requests. + # Second, if we DON'T find a template but the controller action does have + # templates for other formats, variants, etc., then we trust that you meant + # to provide a template for this response, too, and we raise + # <tt>ActionController::UnknownFormat</tt> with an explanation. # - # Third, if the current request is an "interactive" browser request (the user - # navigated here by entering the URL in the address bar, submiting a form, - # clicking on a link, etc. as opposed to an XHR or non-browser API request), - # <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error - # message. + # Third, if we DON'T find a template AND the request is a page load in a web + # browser (technically, a non-XHR GET request for an HTML response) where + # you reasonably expect to have rendered a template, then we raise + # <tt>ActionView::UnknownFormat</tt> with an explanation. # - # Finally, it falls back to the same "204 No Content" behavior as API controllers. + # Finally, if we DON'T find a template AND the request isn't a browser page + # load, then we implicitly respond with 204 No Content. module ImplicitRender # :stopdoc: @@ -37,39 +35,23 @@ module ActionController if template_exists?(action_name.to_s, _prefixes, variants: request.variant) render(*args) elsif any_templates?(action_name.to_s, _prefixes) - message = "#{self.class.name}\##{action_name} does not know how to respond " \ - "to this request. There are other templates available for this controller " \ - "action but none of them were suitable for this request.\n\n" \ - "This usually happens when the client requested an unsupported format " \ - "(e.g. requesting HTML content from a JSON endpoint or vice versa), but " \ - "it might also be failing due to other constraints, such as locales or " \ - "variants.\n" - - if request.formats.any? - message << "\nRequested format(s): #{request.formats.join(", ")}" - end - - if request.variant.any? - message << "\nRequested variant(s): #{request.variant.join(", ")}" - end + message = "#{self.class.name}\##{action_name} is missing a template " \ + "for this request format and variant.\n" \ + "\nrequest.formats: #{request.formats.map(&:to_s).inspect}" \ + "\nrequest.variant: #{request.variant.inspect}" raise ActionController::UnknownFormat, message elsif interactive_browser_request? - message = "You did not define any templates for #{self.class.name}\##{action_name}. " \ - "This is not necessarily a problem (e.g. you might be building an API endpoint " \ - "that does not require any templates), and the controller would usually respond " \ - "with `head :no_content` for your convenience.\n\n" \ - "However, you appear to have navigated here from an interactive browser request – " \ - "such as by navigating to this URL directly, clicking on a link or submitting a form. " \ - "Rendering a `head :no_content` in this case could have resulted in unexpected UI " \ - "behavior in the browser.\n\n" \ - "If you expected the `head :no_content` response, you do not need to take any " \ - "actions – requests coming from an XHR (AJAX) request or other non-browser clients " \ - "will receive the \"204 No Content\" response as expected.\n\n" \ - "If you did not expect this behavior, you can resolve this error by adding a " \ - "template for this controller action (usually `#{action_name}.html.erb`) or " \ - "otherwise indicate the appropriate response in the action using `render`, " \ - "`redirect_to`, `head`, etc.\n" + message = "#{self.class.name}\##{action_name} is missing a template " \ + "for this request format and variant.\n\n" \ + "request.formats: #{request.formats.map(&:to_s).inspect}\n" \ + "request.variant: #{request.variant.inspect}\n\n" \ + "NOTE! For XHR/Ajax or API requests, this action would normally " \ + "respond with 204 No Content: an empty white screen. Since you're " \ + "loading it in a web browser, we assume that you expected to " \ + "actually render a template, not… nothing, so we're showing an " \ + "error to be extra-clear. If you expect 204 No Content, carry on. " \ + "That's what you'll get from an XHR or API request. Give it a shot." raise ActionController::UnknownFormat, message else @@ -85,9 +67,8 @@ module ActionController end private - def interactive_browser_request? - request.format == Mime[:html] && !request.xhr? + request.get? && request.format == Mime[:html] && !request.xhr? end end end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 885ea3fefd..624a6d5b76 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -75,8 +75,8 @@ module ActionController ActiveSupport::Notifications.instrument("halted_callback.action_controller", :filter => filter) end - # A hook which allows you to clean up any time taken into account in - # views wrongly, like database querying time. + # A hook which allows you to clean up any time, wrongly taken into account in + # views, like database querying time. # # def cleanup_view_runtime # super - time_taken_in_something_expensive diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index fc20e7a421..5d395cd8bd 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -3,7 +3,7 @@ require 'delegate' require 'active_support/json' module ActionController - # Mix this module in to your controller, and all actions in that controller + # Mix this module into your controller, and all actions in that controller # will be able to stream data to the client as it's written. # # class MyController < ActionController::Base @@ -20,7 +20,7 @@ module ActionController # end # end # - # There are a few caveats with this use. You *cannot* write headers after the + # There are a few caveats with this module. You *cannot* write headers after the # response has been committed (Response#committed? will return truthy). # Calling +write+ or +close+ on the response stream will cause the response # object to be committed. Make sure all headers are set before calling write @@ -163,14 +163,6 @@ module ActionController end end - def each - @response.sending! - while str = @buf.pop - yield str - end - @response.sent! - end - # Write a 'close' event to the buffer; the producer/writing thread # uses this to notify us that it's finished supplying content. # @@ -210,6 +202,14 @@ module ActionController def call_on_error @error_callback.call end + + private + + def each_chunk(&block) + while str = @buf.pop + yield str + end + end end class Response < ActionDispatch::Response #:nodoc: all diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 173a14a1d2..2e89af1a5e 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -198,7 +198,7 @@ module ActionController #:nodoc: _process_format(format) _set_rendered_content_type format response = collector.response - response ? response.call : render({}) + response.call if response else raise ActionController::UnknownFormat end diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index b13ba06962..3c7cc15627 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -84,7 +84,7 @@ module ActionController # redirect_back fallback_location: proc { edit_post_url(@post) } # # All options that can be passed to <tt>redirect_to</tt> are accepted as - # options and the behavior is indetical. + # options and the behavior is identical. def redirect_back(fallback_location:, **args) if referer = request.headers["Referer"] redirect_to referer, **args diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 90fb34e386..1735609cd9 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -103,7 +103,7 @@ module ActionController # # Both <tt>ActionController::Base</tt> and <tt>ActionController::API</tt> # include <tt>ActionController::Renderers::All</tt>, making all renderers - # avaialable in the controller. See <tt>Renderers::RENDERERS</tt> and <tt>Renderers.add</tt>. + # available in the controller. See <tt>Renderers::RENDERERS</tt> and <tt>Renderers.add</tt>. # # Since <tt>ActionController::Metal</tt> controllers cannot render, the controller # must include <tt>AbstractController::Rendering</tt>, <tt>ActionController::Rendering</tt>, diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index b2f0b382b9..0559fbc6ce 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -213,7 +213,7 @@ module ActionController #:nodoc: if !verified_request? if logger && log_warning_on_csrf_failure - logger.warn "Can't verify CSRF token authenticity" + logger.warn "Can't verify CSRF token authenticity." end handle_unverified_request end @@ -235,7 +235,9 @@ module ActionController #:nodoc: # we aren't serving an unauthorized cross-origin response. def verify_same_origin_request if marked_for_same_origin_verification? && non_xhr_javascript_response? - logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING if logger + if logger && log_warning_on_csrf_failure + logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING + end raise ActionController::InvalidCrossOriginRequest, CROSS_ORIGIN_JAVASCRIPT_WARNING end end @@ -405,7 +407,8 @@ module ActionController #:nodoc: end def normalize_action_path(action_path) - action_path.split('?').first.to_s.chomp('/') + uri = URI.parse(action_path) + uri.path.chomp('/') end end end diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 81b9a7b9ed..17f4030f25 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -1,18 +1,11 @@ module ActionController #:nodoc: - # This module is responsible to provide `rescue_from` helpers - # to controllers and configure when detailed exceptions must be + # This module is responsible for providing `rescue_from` helpers + # to controllers and configuring when detailed exceptions must be # shown. module Rescue extend ActiveSupport::Concern include ActiveSupport::Rescuable - def rescue_with_handler(exception) - if exception.cause && handler_for_rescue(exception.cause) - exception = exception.cause - end - super(exception) - end - # Override this method if you want to customize when detailed # exceptions must be shown. This method is only called when # consider_all_requests_local is false. By default, it returns @@ -27,7 +20,7 @@ module ActionController #:nodoc: super rescue Exception => exception request.env['action_dispatch.show_detailed_exceptions'] ||= show_detailed_exceptions? - rescue_with_handler(exception) || raise(exception) + rescue_with_handler(exception) || raise end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 6d59e32c68..b326695ce2 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -43,7 +43,7 @@ module ActionController # == Action Controller \Parameters # - # Allows to choose which attributes should be whitelisted for mass updating + # Allows you to choose which attributes should be whitelisted for mass updating # and thus prevent accidentally exposing that which shouldn't be exposed. # Provides two methods for this purpose: #require and #permit. The former is # used to mark parameters as required. The latter is used to set the parameter @@ -58,8 +58,7 @@ module ActionController # }) # # permitted = params.require(:person).permit(:name, :age) - # permitted # => {"name"=>"Francesco", "age"=>22} - # permitted.class # => ActionController::Parameters + # permitted # => <ActionController::Parameters {"name"=>"Francesco", "age"=>22} permitted: true> # permitted.permitted? # => true # # Person.first.update!(permitted) @@ -87,7 +86,7 @@ module ActionController # # params = ActionController::Parameters.new(a: "123", b: "456") # params.permit(:c) - # # => {} + # # => <ActionController::Parameters {} permitted: true> # # ActionController::Parameters.action_on_unpermitted_parameters = :raise # @@ -107,6 +106,8 @@ module ActionController # params["key"] # => "value" class Parameters cattr_accessor :permit_all_parameters, instance_accessor: false + self.permit_all_parameters = false + cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, @@ -184,12 +185,19 @@ module ActionController # Returns an unsafe, unfiltered # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this # parameter. + # + # params = ActionController::Parameters.new({ + # name: 'Senjougahara Hitagi', + # oddity: 'Heavy stone crab' + # }) + # params.to_unsafe_h + # # => {"name"=>"Senjougahara Hitagi", "oddity" => "Heavy stone crab"} def to_unsafe_h convert_parameters_to_hashes(@parameters, :to_unsafe_h) end alias_method :to_unsafe_hash, :to_unsafe_h - # Convert all hashes in values into parameters, then yield each pair like + # Convert all hashes in values into parameters, then yield each pair in # the same way as <tt>Hash#each_pair</tt> def each_pair(&block) @parameters.each_pair do |key, value| @@ -248,7 +256,7 @@ module ActionController # either present or the singleton +false+, returns said value: # # ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person) - # # => {"name"=>"Francesco"} + # # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false> # # Otherwise raises <tt>ActionController::ParameterMissing</tt>: # @@ -269,12 +277,12 @@ module ActionController # returned: # # params = ActionController::Parameters.new(user: { ... }, profile: { ... }) - # user_params, profile_params = params.require(:user, :profile) + # user_params, profile_params = params.require([:user, :profile]) # - # Otherwise, the method reraises the first exception found: + # Otherwise, the method re-raises the first exception found: # # params = ActionController::Parameters.new(user: {}, profile: {}) - # user_params, profile_params = params.require(:user, :profile) + # user_params, profile_params = params.require([:user, :profile]) # # ActionController::ParameterMissing: param is missing or the value is empty: user # # Technically this method can be used to fetch terminal values: @@ -367,13 +375,13 @@ module ActionController # }) # # params.require(:person).permit(:contact) - # # => {} + # # => <ActionController::Parameters {} permitted: true> # # params.require(:person).permit(contact: :phone) - # # => {"contact"=>{"phone"=>"555-1234"}} + # # => <ActionController::Parameters {"contact"=><ActionController::Parameters {"phone"=>"555-1234"} permitted: true>} permitted: true> # # params.require(:person).permit(contact: [ :email, :phone ]) - # # => {"contact"=>{"email"=>"none@test.com", "phone"=>"555-1234"}} + # # => <ActionController::Parameters {"contact"=><ActionController::Parameters {"email"=>"none@test.com", "phone"=>"555-1234"} permitted: true>} permitted: true> def permit(*filters) params = self.class.new @@ -395,7 +403,7 @@ module ActionController # returns +nil+. # # params = ActionController::Parameters.new(person: { name: 'Francesco' }) - # params[:person] # => {"name"=>"Francesco"} + # params[:person] # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false> # params[:none] # => nil def [](key) convert_hashes_to_parameters(key, @parameters[key]) @@ -414,7 +422,7 @@ module ActionController # is given, then that will be run and its result returned. # # params = ActionController::Parameters.new(person: { name: 'Francesco' }) - # params.fetch(:person) # => {"name"=>"Francesco"} + # params.fetch(:person) # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false> # params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none # params.fetch(:none, 'Francesco') # => "Francesco" # params.fetch(:none) { 'Francesco' } # => "Francesco" @@ -434,12 +442,12 @@ module ActionController # Extracts the nested parameter from the given +keys+ by calling +dig+ # at each step. Returns +nil+ if any intermediate step is +nil+. # - # params = ActionController::Parameters.new(foo: { bar: { baz: 1 } }) - # params.dig(:foo, :bar, :baz) # => 1 - # params.dig(:foo, :zot, :xyz) # => nil + # params = ActionController::Parameters.new(foo: { bar: { baz: 1 } }) + # params.dig(:foo, :bar, :baz) # => 1 + # params.dig(:foo, :zot, :xyz) # => nil # - # params2 = ActionController::Parameters.new(foo: [10, 11, 12]) - # params2.dig(:foo, 1) + # params2 = ActionController::Parameters.new(foo: [10, 11, 12]) + # params2.dig(:foo, 1) # => 11 def dig(*keys) convert_value_to_parameters(@parameters.dig(*keys)) end @@ -450,8 +458,8 @@ module ActionController # don't exist, returns an empty hash. # # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) - # params.slice(:a, :b) # => {"a"=>1, "b"=>2} - # params.slice(:d) # => {} + # params.slice(:a, :b) # => <ActionController::Parameters {"a"=>1, "b"=>2} permitted: false> + # params.slice(:d) # => <ActionController::Parameters {} permitted: false> def slice(*keys) new_instance_with_inherited_permitted_status(@parameters.slice(*keys)) end @@ -467,8 +475,8 @@ module ActionController # filters out the given +keys+. # # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) - # params.except(:a, :b) # => {"c"=>3} - # params.except(:d) # => {"a"=>1,"b"=>2,"c"=>3} + # params.except(:a, :b) # => <ActionController::Parameters {"c"=>3} permitted: false> + # params.except(:d) # => <ActionController::Parameters {"a"=>1, "b"=>2, "c"=>3} permitted: false> def except(*keys) new_instance_with_inherited_permitted_status(@parameters.except(*keys)) end @@ -476,8 +484,8 @@ module ActionController # Removes and returns the key/value pairs matching the given keys. # # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) - # params.extract!(:a, :b) # => {"a"=>1, "b"=>2} - # params # => {"c"=>3} + # params.extract!(:a, :b) # => <ActionController::Parameters {"a"=>1, "b"=>2} permitted: false> + # params # => <ActionController::Parameters {"c"=>3} permitted: false> def extract!(*keys) new_instance_with_inherited_permitted_status(@parameters.extract!(*keys)) end @@ -487,7 +495,7 @@ module ActionController # # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) # params.transform_values { |x| x * 2 } - # # => {"a"=>2, "b"=>4, "c"=>6} + # # => <ActionController::Parameters {"a"=>2, "b"=>4, "c"=>6} permitted: false> def transform_values(&block) if block new_instance_with_inherited_permitted_status( @@ -570,7 +578,7 @@ module ActionController # params = ActionController::Parameters.new(a: 1) # params.permit! # params.permitted? # => true - # copy_params = params.dup # => {"a"=>1} + # copy_params = params.dup # => <ActionController::Parameters {"a"=>1} permitted: true> # copy_params.permitted? # => true def dup super.tap do |duplicate| @@ -749,6 +757,10 @@ module ActionController end end + def non_scalar?(value) + value.is_a?(Array) || value.is_a?(Parameters) + end + EMPTY_ARRAY = [] def hash_filter(params, filter) filter = filter.with_indifferent_access @@ -763,7 +775,7 @@ module ActionController array_of_permitted_scalars?(self[key]) do |val| params[key] = val end - else + elsif non_scalar?(value) # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| element.permit(*Array.wrap(filter[key])) @@ -786,7 +798,7 @@ module ActionController # # class PeopleController < ActionController::Base # # Using "Person.create(params[:person])" would raise an - # # ActiveModel::ForbiddenAttributes exception because it'd + # # ActiveModel::ForbiddenAttributesError exception because it'd # # be using mass assignment without an explicit permit step. # # This is the recommended form: # def create @@ -814,7 +826,8 @@ module ActionController # end # # In order to use <tt>accepts_nested_attributes_for</tt> with Strong \Parameters, you - # will need to specify which nested attributes should be whitelisted. + # will need to specify which nested attributes should be whitelisted. You might want + # to allow +:id+ and +:_destroy+, see ActiveRecord::NestedAttributes for more information. # # class Person # has_many :pets @@ -834,7 +847,7 @@ module ActionController # # It's mandatory to specify the nested attributes that should be whitelisted. # # If you use `permit` with just the key that points to the nested attributes hash, # # it will return an empty hash. - # params.require(:person).permit(:name, :age, pets_attributes: [ :name, :category ]) + # params.require(:person).permit(:name, :age, pets_attributes: [ :id, :name, :category ]) # end # end # diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index e4d19e9dba..a8c8d66682 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -1,7 +1,7 @@ require 'active_support/core_ext/hash/keys' module ActionController - # ActionController::Renderer allows to render arbitrary templates + # ActionController::Renderer allows you to render arbitrary templates # without requirement of being in controller actions. # # You get a concrete renderer class by invoking ActionController::Base#renderer. @@ -13,11 +13,11 @@ module ActionController # # ApplicationController.renderer.render template: '...' # - # You can use a shortcut on controller to replace previous example with: + # You can use this shortcut in a controller, instead of the previous example: # # ApplicationController.render template: '...' # - # #render method allows you to use any options as when rendering in controller. + # #render allows you to use the same options that you can use when rendering in a controller. # For example, # # FooController.render :action, locals: { ... }, assigns: { ... } @@ -45,7 +45,7 @@ module ActionController }.freeze # Create a new renderer instance for a specific controller class. - def self.for(controller, env = {}, defaults = DEFAULTS) + def self.for(controller, env = {}, defaults = DEFAULTS.dup) new(controller, env, defaults) end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index ecd21f29ce..b1b3e87934 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -527,32 +527,37 @@ module ActionController @request.set_header k, @controller.config.relative_url_root end - @controller.recycle! - @controller.dispatch(action, @request, @response) - @request = @controller.request - @response = @controller.response - - @request.delete_header 'HTTP_COOKIE' + begin + @controller.recycle! + @controller.dispatch(action, @request, @response) + ensure + @request = @controller.request + @response = @controller.response + + @request.delete_header 'HTTP_COOKIE' + + if @request.have_cookie_jar? + unless @request.cookie_jar.committed? + @request.cookie_jar.write(@response) + self.cookies.update(@request.cookie_jar.instance_variable_get(:@cookies)) + end + end + @response.prepare! - if @request.have_cookie_jar? - unless @request.cookie_jar.committed? - @request.cookie_jar.write(@response) - self.cookies.update(@request.cookie_jar.instance_variable_get(:@cookies)) + if flash_value = @request.flash.to_session_value + @request.session['flash'] = flash_value + else + @request.session.delete('flash') end - end - @response.prepare! - if flash_value = @request.flash.to_session_value - @request.session['flash'] = flash_value - else - @request.session.delete('flash') - end + if xhr + @request.delete_header 'HTTP_X_REQUESTED_WITH' + @request.delete_header 'HTTP_ACCEPT' + end + @request.query_string = '' - if xhr - @request.delete_header 'HTTP_X_REQUESTED_WITH' - @request.delete_header 'HTTP_ACCEPT' + @response.sent! end - @request.query_string = '' @response end diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 4bd727c14e..9fa2e38ae3 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -17,9 +17,7 @@ module ActionDispatch end def if_none_match_etags - (if_none_match ? if_none_match.split(/\s*,\s*/) : []).collect do |etag| - etag.gsub(/^\"|\"$/, "") - end + if_none_match ? if_none_match.split(/\s*,\s*/) : [] end def not_modified?(modified_at) @@ -28,8 +26,8 @@ module ActionDispatch def etag_matches?(etag) if etag - etag = etag.gsub(/^\"|\"$/, "") - if_none_match_etags.include?(etag) + validators = if_none_match_etags + validators.include?(etag) || validators.include?('*') end end @@ -80,27 +78,63 @@ module ActionDispatch set_header DATE, utc_time.httpdate end - # This method allows you to set the ETag for cached content, which - # will be returned to the end user. + # This method sets a weak ETag validator on the response so browsers + # and proxies may cache the response, keyed on the ETag. On subsequent + # requests, the If-None-Match header is set to the cached ETag. If it + # matches the current ETag, we can return a 304 Not Modified response + # with no body, letting the browser or proxy know that their cache is + # current. Big savings in request time and network bandwidth. + # + # Weak ETags are considered to be semantically equivalent but not + # byte-for-byte identical. This is perfect for browser caching of HTML + # pages where we don't care about exact equality, just what the user + # is viewing. # - # By default, Action Dispatch sets all ETags to be weak. - # This ensures that if the content changes only semantically, - # the whole page doesn't have to be regenerated from scratch - # by the web server. With strong ETags, pages are compared - # byte by byte, and are regenerated only if they are not exactly equal. - def etag=(etag) - key = ActiveSupport::Cache.expand_cache_key(etag) - super %(W/"#{Digest::MD5.hexdigest(key)}") + # Strong ETags are considered byte-for-byte identical. They allow a + # browser or proxy cache to support Range requests, useful for paging + # through a PDF file or scrubbing through a video. Some CDNs only + # support strong ETags and will ignore weak ETags entirely. + # + # Weak ETags are what we almost always need, so they're the default. + # Check out `#strong_etag=` to provide a strong ETag validator. + def etag=(weak_validators) + self.weak_etag = weak_validators + end + + def weak_etag=(weak_validators) + set_header 'ETag', generate_weak_etag(weak_validators) + end + + def strong_etag=(strong_validators) + set_header 'ETag', generate_strong_etag(strong_validators) end def etag?; etag; end + # True if an ETag is set and it's a weak validator (preceded with W/) + def weak_etag? + etag? && etag.starts_with?('W/"') + end + + # True if an ETag is set and it isn't a weak validator (not preceded with W/) + def strong_etag? + etag? && !weak_etag? + end + private DATE = 'Date'.freeze LAST_MODIFIED = "Last-Modified".freeze SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate]) + def generate_weak_etag(validators) + "W/#{generate_strong_etag(validators)}" + end + + def generate_strong_etag(validators) + %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(validators))}") + end + def cache_control_segments if cache_control = _cache_control cache_control.delete(' ').split(',') diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 5c3b7245d6..69a934b7cd 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -5,7 +5,7 @@ module ActionDispatch # env = { "CONTENT_TYPE" => "text/plain", "HTTP_USER_AGENT" => "curl/7.43.0" } # headers = ActionDispatch::Http::Headers.new(env) # headers["Content-Type"] # => "text/plain" - # headers["User-Agent"] # => "curl/7/43/0" + # headers["User-Agent"] # => "curl/7.43.0" # # Also note that when headers are mapped to CGI-like variables by the Rack # server, both dashes and underscores are converted to underscores. This diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index e9b25339dc..0a58ce2b96 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -164,7 +164,7 @@ module ActionDispatch end def format_from_path_extension - path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] + path = get_header('action_dispatch.original_path') || get_header('PATH_INFO') if match = path && path.match(/\.(\w+)\z/) Mime[match.captures.first] end diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 66cea88256..8b04174f1f 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -21,7 +21,7 @@ Mime::Type.register "video/mpeg", :mpeg, [], %w(mpg mpeg mpe) Mime::Type.register "application/xml", :xml, %w( text/xml application/x-xml ) Mime::Type.register "application/rss+xml", :rss Mime::Type.register "application/atom+xml", :atom -Mime::Type.register "application/x-yaml", :yaml, %w( text/yaml ) +Mime::Type.register "application/x-yaml", :yaml, %w( text/yaml ), %w(yml yaml) Mime::Type.register "multipart/form-data", :multipart_form Mime::Type.register "application/x-www-form-urlencoded", :url_encoded_form diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 316a9f08b7..b0ed681623 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -337,7 +337,6 @@ module ActionDispatch else self.session = {} end - self.flash = nil end def session=(session) #:nodoc: diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index fa4c54701a..1515d59df3 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -68,7 +68,13 @@ module ActionDispatch # :nodoc: alias_method :headers, :header delegate :[], :[]=, :to => :@header - delegate :each, :to => :@stream + + def each(&block) + sending! + x = @stream.each(&block) + sent! + x + end CONTENT_TYPE = "Content-Type".freeze SET_COOKIE = "Set-Cookie".freeze @@ -97,10 +103,10 @@ module ActionDispatch # :nodoc: def body @str_body ||= begin - buf = '' - each { |chunk| buf << chunk } - buf - end + buf = '' + each { |chunk| buf << chunk } + buf + end end def write(string) @@ -112,10 +118,13 @@ module ActionDispatch # :nodoc: end def each(&block) - @response.sending! - x = @buf.each(&block) - @response.sent! - x + if @str_body + return enum_for(:each) unless block_given? + + yield @str_body + else + each_chunk(&block) + end end def abort @@ -129,6 +138,12 @@ module ActionDispatch # :nodoc: def closed? @closed end + + private + + def each_chunk(&block) + @buf.each(&block) # extract into own method + end end def self.create(status = 200, header = {}, body = [], default_headers: self.default_headers) diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 37f41ae988..7a1350a46d 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -217,7 +217,7 @@ module ActionDispatch @protocol ||= ssl? ? 'https://' : 'http://' end - # Returns the \host for this request, such as "example.com". + # Returns the \host and port for this request, such as "example.com:8080". # # class Request < Rack::Request # include ActionDispatch::Http::URL @@ -226,6 +226,9 @@ module ActionDispatch # req = Request.new 'HTTP_HOST' => 'example.com' # req.raw_host_with_port # => "example.com" # + # req = Request.new 'HTTP_HOST' => 'example.com:80' + # req.raw_host_with_port # => "example.com:80" + # # req = Request.new 'HTTP_HOST' => 'example.com:8080' # req.raw_host_with_port # => "example.com:8080" def raw_host_with_port @@ -236,7 +239,7 @@ module ActionDispatch end end - # Returns the host for this request, such as example.com. + # Returns the host for this request, such as "example.com". # # class Request < Rack::Request # include ActionDispatch::Http::URL @@ -249,12 +252,16 @@ module ActionDispatch end # Returns a \host:\port string for this request, such as "example.com" or - # "example.com:8080". + # "example.com:8080". Port is only included if it is not a default port + # (80 or 443) # # class Request < Rack::Request # include ActionDispatch::Http::URL # end # + # req = Request.new 'HTTP_HOST' => 'example.com' + # req.host_with_port # => "example.com" + # # req = Request.new 'HTTP_HOST' => 'example.com:80' # req.host_with_port # => "example.com" # @@ -347,6 +354,17 @@ module ActionDispatch standard_port? ? '' : ":#{port}" end + # Returns the requested port, such as 8080, based on SERVER_PORT + # + # class Request < Rack::Request + # include ActionDispatch::Http::URL + # end + # + # req = Request.new 'SERVER_PORT' => '80' + # req.server_port # => 80 + # + # req = Request.new 'SERVER_PORT' => '8080' + # req.server_port # => 8080 def server_port get_header('SERVER_PORT').to_i end diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 0323360faa..200477b002 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -32,8 +32,13 @@ module ActionDispatch defaults = route.defaults required_parts = route.required_parts - parameterized_parts.keep_if do |key, value| - (defaults[key].nil? && value.present?) || value.to_s != defaults[key].to_s || required_parts.include?(key) + + route.parts.reverse_each do |key| + break if defaults[key].nil? && parameterized_parts[key].present? + break if parameterized_parts[key].to_s != defaults[key].to_s + break if required_parts.include?(key) + + parameterized_parts.delete(key) end return [route.format(parameterized_parts), params] diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 51a471fb23..8974258cf7 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -36,6 +36,14 @@ module ActionDispatch def debug_hash(object) object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") end + + def render(*) + if logger = ActionView::Base.logger + logger.silence { super } + else + super + end + end end def initialize(app, routes_app = nil, response_format = :default) @@ -67,18 +75,19 @@ module ActionDispatch log_error(request, wrapper) if request.get_header('action_dispatch.show_detailed_exceptions') - case @response_format - when :api - render_for_api_application(request, wrapper) - when :default - render_for_default_application(request, wrapper) + content_type = request.formats.first + + if api_request?(content_type) + render_for_api_request(content_type, wrapper) + else + render_for_browser_request(request, wrapper) end else raise exception end end - def render_for_default_application(request, wrapper) + def render_for_browser_request(request, wrapper) template = create_template(request, wrapper) file = "rescues/#{wrapper.rescue_template}" @@ -92,7 +101,7 @@ module ActionDispatch render(wrapper.status_code, body, format) end - def render_for_api_application(request, wrapper) + def render_for_api_request(content_type, wrapper) body = { status: wrapper.status_code, error: Rack::Utils::HTTP_STATUS_CODES.fetch( @@ -103,7 +112,6 @@ module ActionDispatch traces: wrapper.traces } - content_type = request.formats.first to_format = "to_#{content_type.to_sym}" if content_type && body.respond_to?(to_format) @@ -181,5 +189,9 @@ module ActionDispatch ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes) end end + + def api_request?(content_type) + @response_format == :api && !content_type.html? + end end end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index c51dcd542a..80703940ed 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -70,6 +70,11 @@ module ActionDispatch session.delete('flash') end end + + def reset_session # :nodoc + super + self.flash = nil + end end class FlashNow #:nodoc: @@ -128,7 +133,7 @@ module ActionDispatch def to_session_value #:nodoc: flashes_to_keep = @flashes.except(*@discard) return nil if flashes_to_keep.empty? - {'flashes' => flashes_to_keep} + { 'discard' => [], 'flashes' => flashes_to_keep } end def initialize(flashes = {}, discard = []) #:nodoc: diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 5841c978af..faf3262b8f 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,7 @@ module ActionDispatch # The +parsers+ argument can take Hash of parsers where key is identifying # content mime type, and value is a lambda that is going to process data. def self.new(app, parsers = {}) + ActiveSupport::Deprecation.warn('ActionDispatch::ParamsParser is deprecated and will be removed in Rails 5.1. Configure the parameter parsing in ActionDispatch::Request.parameter_parsers.') parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers) app diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 41c220236a..2c5721dc22 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -27,8 +27,8 @@ module ActionDispatch # in the server's `public/` directory (see Static#call). def match?(path) path = ::Rack::Utils.unescape_path path - return false unless valid_path?(path) - path = Rack::Utils.clean_path_info path + return false unless ::Rack::Utils.valid_path? path + path = ::Rack::Utils.clean_path_info path paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] @@ -94,10 +94,6 @@ module ActionDispatch false end end - - def valid_path?(path) - path.valid_encoding? && !path.include?("\0") - end end # This middleware will attempt to return the contents of a file's body from diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 42890225fa..47568f6ad0 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -9,7 +9,7 @@ module ActionDispatch # Singleton object used to determine if an optional param wasn't specified Unspecified = Object.new - + # Creates a session hash, merging the properties of the previous session if any def self.create(store, req, default_options) session_was = find req @@ -198,6 +198,10 @@ module ActionDispatch @delegate.merge!(other) end + def each(&block) + to_hash.each(&block) + end + private def load_for_read! diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index dcf800b215..61ebd0b8db 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -73,14 +73,14 @@ module ActionDispatch # get 'post/:id' => 'posts#show' # post 'post/:id' => 'posts#create_comment' # + # Now, if you POST to <tt>/posts/:id</tt>, it will route to the <tt>create_comment</tt> action. A GET on the same + # URL will route to the <tt>show</tt> action. + # # If your route needs to respond to more than one HTTP method (or all methods) then using the # <tt>:via</tt> option on <tt>match</tt> is preferable. # # match 'post/:id' => 'posts#show', via: [:get, :post] # - # Now, if you POST to <tt>/posts/:id</tt>, it will route to the <tt>create_comment</tt> action. A GET on the same - # URL will route to the <tt>show</tt> action. - # # == Named routes # # Routes can be named by passing an <tt>:as</tt> option, @@ -89,7 +89,7 @@ module ActionDispatch # # Example: # - # # In routes.rb + # # In config/routes.rb # get '/login' => 'accounts#login', as: 'login' # # # With render, redirect_to, tests, etc. @@ -101,7 +101,7 @@ module ActionDispatch # # Use <tt>root</tt> as a shorthand to name a route for the root path "/". # - # # In routes.rb + # # In config/routes.rb # root to: 'blogs#index' # # # would recognize http://www.example.com/ as @@ -114,15 +114,15 @@ module ActionDispatch # Note: when using +controller+, the route is simply named after the # method you call on the block parameter rather than map. # - # # In routes.rb + # # In config/routes.rb # controller :blog do # get 'blog/show' => :list # get 'blog/delete' => :delete - # get 'blog/edit/:id' => :edit + # get 'blog/edit' => :edit # end # # # provides named routes for show, delete, and edit - # link_to @article.title, show_path(id: @article.id) + # link_to @article.title, blog_show_path(id: @article.id) # # == Pretty URLs # @@ -196,7 +196,7 @@ module ActionDispatch # # Rails.application.reload_routes! # - # This will clear all named routes and reload routes.rb if the file has been modified from + # This will clear all named routes and reload config/routes.rb if the file has been modified from # last load. To absolutely force reloading, use <tt>reload!</tt>. # # == Testing Routes @@ -252,5 +252,14 @@ module ActionDispatch SEPARATORS = %w( / . ? ) #:nodoc: HTTP_METHODS = [:get, :head, :post, :patch, :put, :delete, :options] #:nodoc: + + #:stopdoc: + INSECURE_URL_PARAMETERS_MESSAGE = <<-MSG.squish + Attempting to generate a URL from non-sanitized request parameters! + + An attacker can inject malicious data into the generated URL, such as + changing the host. Whitelist and sanitize passed parameters to be secure. + MSG + #:startdoc: end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 5d30a545a2..2459a45827 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -33,11 +33,11 @@ module ActionDispatch end def controller - requirements[:controller] || ':controller' + parts.include?(:controller) ? ':controller' : requirements[:controller] end def action - requirements[:action] || ':action' + parts.include?(:action) ? ':action' : requirements[:action] end def internal? diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 16b430c36e..e2cf75da3a 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1,4 +1,3 @@ -require 'active_support/core_ext/hash/reverse_merge' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/enumerable' require 'active_support/core_ext/array/extract_options' @@ -107,7 +106,7 @@ module ActionDispatch @ast = ast @anchor = anchor @via = via - @internal = options[:internal] + @internal = options.delete(:internal) path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -121,7 +120,7 @@ module ActionDispatch if options_constraints.is_a?(Hash) @defaults = Hash[options_constraints.find_all { |key, default| - URL_OPTIONS.include?(key) && (String === default || Fixnum === default) + URL_OPTIONS.include?(key) && (String === default || Integer === default) }].merge @defaults @blocks = blocks constraints.merge! options_constraints @@ -138,6 +137,10 @@ module ActionDispatch @conditions = Hash[conditions] @defaults = formats[:defaults].merge(@defaults).merge(normalize_defaults(options)) + if path_params.include?(:action) && !@requirements.key?(:action) + @defaults[:action] ||= 'index' + end + @required_defaults = (split_options[:required_defaults] || []).map(&:first) end @@ -821,10 +824,10 @@ module ActionDispatch if options[:constraints].is_a?(Hash) defaults = options[:constraints].select do |k, v| - URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) + URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Integer)) end - (options[:defaults] ||= {}).reverse_merge!(defaults) + options[:defaults] = defaults.merge(options[:defaults] || {}) else block, options[:constraints] = options[:constraints], {} end @@ -1059,6 +1062,10 @@ module ActionDispatch def merge_shallow_scope(parent, child) #:nodoc: child ? true : false end + + def merge_to_scope(parent, child) + child + end end # Resource routing allows you to quickly declare all of the common routes @@ -1579,6 +1586,10 @@ module ActionDispatch raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end + if @scope[:to] + options[:to] ||= @scope[:to] + end + if @scope[:controller] && @scope[:action] options[:to] ||= "#{@scope[:controller]}##{@scope[:action]}" end @@ -1598,7 +1609,7 @@ module ActionDispatch route_options = options.dup if _path && option_path ActiveSupport::Deprecation.warn <<-eowarn -Specifying strings for both :path and the route path is deprecated. Change things like this: +Specifying strings for both :path and the route path is deprecated. Change things like this: match #{_path.inspect}, :path => #{option_path.inspect} @@ -2018,7 +2029,7 @@ to this: class Scope # :nodoc: OPTIONS = [:path, :shallow_path, :as, :shallow_prefix, :module, :controller, :action, :path_names, :constraints, - :shallow, :blocks, :defaults, :via, :format, :options] + :shallow, :blocks, :defaults, :via, :format, :options, :to] RESOURCE_SCOPES = [:resource, :resources] RESOURCE_METHOD_SCOPES = [:collection, :member, :new] @@ -2085,8 +2096,7 @@ to this: def each node = self - loop do - break if node.equal? NULL + until node.equal? NULL yield node node = node.parent end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 9934f5547a..3156acf615 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -4,7 +4,7 @@ module ActionDispatch # given an Active Record model instance. They are to be used in combination with # ActionController::Resources. # - # These methods are useful when you want to generate correct URL or path to a RESTful + # These methods are useful when you want to generate the correct URL or path to a RESTful # resource without having to know the exact type of the record in question. # # Nested resources and/or namespaces are also supported, as illustrated in the example: @@ -79,7 +79,7 @@ module ActionDispatch # polymorphic_url([blog, post], anchor: 'my_anchor', script_name: "/my_app") # # => "http://example.com/my_app/blogs/1/posts/1#my_anchor" # - # For all of these options, see the documentation for <tt>url_for</tt>. + # For all of these options, see the documentation for {url_for}[rdoc-ref:ActionDispatch::Routing::UrlFor]. # # ==== Functionality # diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 85f202b823..ed7130b58e 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -289,7 +289,7 @@ module ActionDispatch if last.permitted? args.pop.to_h else - raise ArgumentError, "Generating a URL from non sanitized request parameters is insecure!" + raise ArgumentError, ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE end end helper.call self, args, options @@ -517,14 +517,14 @@ module ActionDispatch if route.segment_keys.include?(:controller) ActiveSupport::Deprecation.warn(<<-MSG.squish) Using a dynamic :controller segment in a route is deprecated and - will be removed in Rails 5.1 + will be removed in Rails 5.1. MSG end if route.segment_keys.include?(:action) ActiveSupport::Deprecation.warn(<<-MSG.squish) Using a dynamic :action segment in a route is deprecated and - will be removed in Rails 5.1 + will be removed in Rails 5.1. MSG end @@ -548,12 +548,10 @@ module ActionDispatch @recall = recall @set = set - normalize_recall! normalize_options! normalize_controller_action_id! use_relative_controller! normalize_controller! - normalize_action! end def controller @@ -572,11 +570,6 @@ module ActionDispatch end end - # Set 'index' as default action for recall - def normalize_recall! - @recall[:action] ||= 'index' - end - def normalize_options! # If an explicit :controller was given, always make :action explicit # too, so that action expiry works as expected for things like @@ -630,13 +623,6 @@ module ActionDispatch end end - # Move 'index' action from options to recall - def normalize_action! - if @options[:action] == 'index'.freeze - @recall[:action] = @options.delete(:action) - end - end - # Generates a path from routes, returns [path, params]. # If no route is generated the formatter will raise ActionController::UrlGenerationError def generate diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 28be189f93..5ee138e6c6 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -173,7 +173,7 @@ module ActionDispatch route_name) when ActionController::Parameters unless options.permitted? - raise ArgumentError.new("Generating a URL from non sanitized request parameters is insecure!") + raise ArgumentError.new(ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE) end route_name = options.delete :use_route _routes.url_for(options.to_h.symbolize_keys. diff --git a/actionpack/lib/action_dispatch/testing/assertion_response.rb b/actionpack/lib/action_dispatch/testing/assertion_response.rb index 3fb81ff083..404b96bbcd 100644 --- a/actionpack/lib/action_dispatch/testing/assertion_response.rb +++ b/actionpack/lib/action_dispatch/testing/assertion_response.rb @@ -1,14 +1,7 @@ module ActionDispatch - # This is a class that abstracts away an asserted response. - # It purposely does not inherit from Response, because it doesn't need it. - # That means it does not have headers or a body. - # - # As an input to the initializer, we take a Fixnum, a String, or a Symbol. - # If it's a Fixnum or String, we figure out what its symbolized name. - # If it's a Symbol, we figure out what its corresponding code is. - # The resulting code will be a Fixnum, for real HTTP codes, and it will - # be a String for the pseudo-HTTP codes, such as: - # :success, :missing, :redirect and :error + # This is a class that abstracts away an asserted response. It purposely + # does not inherit from Response because it doesn't need it. That means it + # does not have headers or a body. class AssertionResponse attr_reader :code, :name @@ -19,6 +12,9 @@ module ActionDispatch error: "5XX" } + # Accepts a specific response status code as an Integer (404) or String + # ('404') or a response status range as a Symbol pseudo-code (:success, + # indicating any 200-299 status code). def initialize(code_or_name) if code_or_name.is_a?(Symbol) @name = code_or_name diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 60c562d7cd..a7805edaa2 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -95,7 +95,7 @@ module ActionDispatch ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) xhr and xml_http_request methods are deprecated in favor of - `get "/posts", xhr: true` and `post "/posts/1", xhr: true` + `get "/posts", xhr: true` and `post "/posts/1", xhr: true`. MSG process(request_method, path, params: params, headers: headers, xhr: true) @@ -122,6 +122,7 @@ module ActionDispatch # params: { ref_id: 14 }, # headers: { "X-Test-Header" => "testvalue" } def request_via_redirect(http_method, path, *args) + ActiveSupport::Deprecation.warn('`request_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') process_with_kwargs(http_method, path, *args) follow_redirect! while redirect? @@ -131,35 +132,35 @@ module ActionDispatch # Performs a GET request, following any subsequent redirect. # See +request_via_redirect+ for more information. def get_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`get_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`get_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:get, path, *args) end # Performs a POST request, following any subsequent redirect. # See +request_via_redirect+ for more information. def post_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`post_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`post_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:post, path, *args) end # Performs a PATCH request, following any subsequent redirect. # See +request_via_redirect+ for more information. def patch_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`patch_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`patch_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:patch, path, *args) end # Performs a PUT request, following any subsequent redirect. # See +request_via_redirect+ for more information. def put_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`put_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`put_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:put, path, *args) end # Performs a DELETE request, following any subsequent redirect. # See +request_via_redirect+ for more information. def delete_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`delete_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`delete_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:delete, path, *args) end end @@ -299,7 +300,7 @@ module ActionDispatch end end - REQUEST_KWARGS = %i(params headers env xhr) + REQUEST_KWARGS = %i(params headers env xhr as) def kwarg_request?(args) args[0].respond_to?(:keys) && args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } end @@ -316,7 +317,8 @@ module ActionDispatch params: { id: 1 }, headers: { 'X-Extra-Header' => '123' }, env: { 'action_dispatch.custom' => 'custom' }, - xhr: true + xhr: true, + as: :json MSG end @@ -325,17 +327,17 @@ module ActionDispatch request_encoder = RequestEncoder.encoder(as) if path =~ %r{://} - location = URI.parse(path) - https! URI::HTTPS === location if location.scheme - if url_host = location.host - default = Rack::Request::DEFAULT_PORTS[location.scheme] - url_host += ":#{location.port}" if default != location.port - host! url_host + path = build_expanded_path(path, request_encoder) do |location| + https! URI::HTTPS === location if location.scheme + + if url_host = location.host + default = Rack::Request::DEFAULT_PORTS[location.scheme] + url_host += ":#{location.port}" if default != location.port + host! url_host + end end - path = request_encoder.append_format_to location.path - path = location.query ? "#{path}?#{location.query}" : path - else - path = request_encoder.append_format_to path + elsif as + path = build_expanded_path(path, request_encoder) end hostname, port = host.split(':') @@ -394,6 +396,13 @@ module ActionDispatch "#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}" end + def build_expanded_path(path, request_encoder) + location = URI.parse(path) + yield location if block_given? + path = request_encoder.append_format_to location.path + location.query ? "#{path}?#{location.query}" : path + end + class RequestEncoder # :nodoc: @encoders = {} @@ -414,8 +423,11 @@ module ActionDispatch end def append_format_to(path) - path << @path_format unless @url_encoded_form - path + if @url_encoded_form + path + @path_format + else + path + end end def content_type diff --git a/actionpack/lib/action_dispatch/testing/test_request.rb b/actionpack/lib/action_dispatch/testing/test_request.rb index ad1a7f7109..46523a8600 100644 --- a/actionpack/lib/action_dispatch/testing/test_request.rb +++ b/actionpack/lib/action_dispatch/testing/test_request.rb @@ -22,23 +22,23 @@ module ActionDispatch private_class_method :default_env def request_method=(method) - @env['REQUEST_METHOD'] = method.to_s.upcase + set_header('REQUEST_METHOD', method.to_s.upcase) end def host=(host) - @env['HTTP_HOST'] = host + set_header('HTTP_HOST', host) end def port=(number) - @env['SERVER_PORT'] = number.to_i + set_header('SERVER_PORT', number.to_i) end def request_uri=(uri) - @env['REQUEST_URI'] = uri + set_header('REQUEST_URI', uri) end def path=(path) - @env['PATH_INFO'] = path + set_header('PATH_INFO', path) end def action=(action_name) @@ -46,24 +46,24 @@ module ActionDispatch end def if_modified_since=(last_modified) - @env['HTTP_IF_MODIFIED_SINCE'] = last_modified + set_header('HTTP_IF_MODIFIED_SINCE', last_modified) end def if_none_match=(etag) - @env['HTTP_IF_NONE_MATCH'] = etag + set_header('HTTP_IF_NONE_MATCH', etag) end def remote_addr=(addr) - @env['REMOTE_ADDR'] = addr + set_header('REMOTE_ADDR', addr) end def user_agent=(user_agent) - @env['HTTP_USER_AGENT'] = user_agent + set_header('HTTP_USER_AGENT', user_agent) end def accept=(mime_types) - @env.delete('action_dispatch.request.accepts') - @env['HTTP_ACCEPT'] = Array(mime_types).collect(&:to_s).join(",") + delete_header('action_dispatch.request.accepts') + set_header('HTTP_ACCEPT', Array(mime_types).collect(&:to_s).join(",")) end end end diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 157f401f54..d8f86630b1 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -6,9 +6,9 @@ module ActionPack module VERSION MAJOR = 5 - MINOR = 0 + MINOR = 1 TINY = 0 - PRE = "beta3" + PRE = "alpha" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index fcbbfe8a18..1e1d6f5429 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -34,7 +34,6 @@ require 'action_dispatch' require 'active_support/dependencies' require 'active_model' require 'active_record' -require 'action_controller/caching' require 'pp' # require 'pp' early to prevent hidden_methods from not picking up the pretty-print methods until too late @@ -58,10 +57,6 @@ ActiveSupport::Deprecation.debug = true # Disable available locale checks to avoid warnings running the test suite. I18n.enforce_available_locales = false -# Register danish language for testing -I18n.backend.store_translations 'da', {} -I18n.backend.store_translations 'pt-BR', {} - FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') SharedTestRoutes = ActionDispatch::Routing::RouteSet.new @@ -256,24 +251,6 @@ end class ::ApplicationController < ActionController::Base end -class Workshop - extend ActiveModel::Naming - include ActiveModel::Conversion - attr_accessor :id - - def initialize(id) - @id = id - end - - def persisted? - id.present? - end - - def to_s - id.to_s - end -end - module ActionDispatch class DebugExceptions private @@ -377,37 +354,15 @@ module RoutingTestHelpers end end -class MetalRenderingController < ActionController::Metal - include AbstractController::Rendering - include ActionController::Rendering - include ActionController::Renderers -end - class ResourcesController < ActionController::Base def index() head :ok end alias_method :show, :index end -class ThreadsController < ResourcesController; end -class MessagesController < ResourcesController; end class CommentsController < ResourcesController; end -class ReviewsController < ResourcesController; end - class AccountsController < ResourcesController; end -class AdminController < ResourcesController; end -class ProductsController < ResourcesController; end class ImagesController < ResourcesController; end -module Backoffice - class ProductsController < ResourcesController; end - class ImagesController < ResourcesController; end - - module Admin - class ProductsController < ResourcesController; end - class ImagesController < ResourcesController; end - end -end - # Skips the current run on Rubinius using Minitest::Assertions#skip def rubinius_skip(message = '') skip message if RUBY_ENGINE == 'rbx' diff --git a/actionpack/test/assertions/response_assertions_test.rb b/actionpack/test/assertions/response_assertions_test.rb index 579ce0ed29..57a67a48b5 100644 --- a/actionpack/test/assertions/response_assertions_test.rb +++ b/actionpack/test/assertions/response_assertions_test.rb @@ -35,7 +35,7 @@ module ActionDispatch end end - def test_assert_response_fixnum + def test_assert_response_integer @response = FakeResponse.new 400 assert_response 400 diff --git a/actionpack/test/controller/api/with_cookies_test.rb b/actionpack/test/controller/api/with_cookies_test.rb new file mode 100644 index 0000000000..4491dc9002 --- /dev/null +++ b/actionpack/test/controller/api/with_cookies_test.rb @@ -0,0 +1,21 @@ +require 'abstract_unit' + +class WithCookiesController < ActionController::API + include ActionController::Cookies + + def with_cookies + render plain: cookies[:foobar] + end +end + +class WithCookiesTest < ActionController::TestCase + tests WithCookiesController + + def test_with_cookies + request.cookies[:foobar] = 'bazbang' + + get :with_cookies + + assert_equal 'bazbang', response.body + end +end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 754ac144cc..7faf3cd8c6 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -219,12 +219,15 @@ CACHED end def test_fragment_caching_with_options + time = Time.now get :fragment_cached_with_options assert_response :success expected_body = "<body>\n<p>ERB</p>\n</body>\n" assert_equal expected_body, @response.body - assert_equal "<p>ERB</p>", @store.read("views/with_options") + Time.stub(:now, time + 11) do + assert_nil @store.read("views/with_options") + end end def test_render_inline_before_fragment_caching diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index 081288ef21..f87077dd86 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -57,10 +57,10 @@ module ActionDispatch def test_to_session_value @hash['foo'] = 'bar' - assert_equal({'flashes' => {'foo' => 'bar'}}, @hash.to_session_value) + assert_equal({ 'discard' => [], 'flashes' => { 'foo' => 'bar' } }, @hash.to_session_value) @hash.now['qux'] = 1 - assert_equal({'flashes' => {'foo' => 'bar'}}, @hash.to_session_value) + assert_equal({ 'flashes' => { 'foo' => 'bar' }, 'discard' => [] }, @hash.to_session_value) @hash.discard('foo') assert_equal(nil, @hash.to_session_value) diff --git a/actionpack/test/controller/helper_test.rb b/actionpack/test/controller/helper_test.rb index feb882a2b3..ef85e141a0 100644 --- a/actionpack/test/controller/helper_test.rb +++ b/actionpack/test/controller/helper_test.rb @@ -207,6 +207,22 @@ class HelperTest < ActiveSupport::TestCase assert methods.include?(:foobar) end + def test_helper_proxy_in_instance + methods = AllHelpersController.new.helpers.methods + + # Action View + assert_includes methods, :pluralize + + # abc_helper.rb + assert_includes methods, :bare_a + + # fun/games_helper.rb + assert_includes methods, :stratego + + # fun/pdf_helper.rb + assert_includes methods, :foobar + end + def test_helper_proxy_config AllHelpersController.config.my_var = 'smth' diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index ad7166bafa..3b89531e90 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -35,7 +35,7 @@ class SessionTest < ActiveSupport::TestCase path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue"} assert_called_with @session, :process, [:put, path, params: args, headers: headers] do @session.stub :redirect?, false do - @session.request_via_redirect(:put, path, params: args, headers: headers) + assert_deprecated { @session.request_via_redirect(:put, path, params: args, headers: headers) } end end end @@ -54,7 +54,7 @@ class SessionTest < ActiveSupport::TestCase value_series = [true, true, false] assert_called @session, :follow_redirect!, times: 2 do @session.stub :redirect?, ->{ value_series.shift } do - @session.request_via_redirect(:get, path, params: args, headers: headers) + assert_deprecated { @session.request_via_redirect(:get, path, params: args, headers: headers) } end end end @@ -63,7 +63,9 @@ class SessionTest < ActiveSupport::TestCase path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue"} @session.stub :redirect?, false do @session.stub :status, 200 do - assert_equal 200, @session.request_via_redirect(:get, path, params: args, headers: headers) + assert_deprecated do + assert_equal 200, @session.request_via_redirect(:get, path, params: args, headers: headers) + end end end end @@ -1145,6 +1147,22 @@ class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest end end + def test_standard_json_encoding_works + with_routing do |routes| + routes.draw do + ActiveSupport::Deprecation.silence do + post ':action' => FooController + end + end + + post '/foos_json.json', params: { foo: 'fighters' }.to_json, + headers: { 'Content-Type' => 'application/json' } + + assert_response :success + assert_equal({ 'foo' => 'fighters' }, response.parsed_body) + end + end + def test_encoding_as_json post_to_foos as: :json do assert_response :success @@ -1192,6 +1210,20 @@ class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest end end + def test_get_parameters_with_as_option + with_routing do |routes| + routes.draw do + ActiveSupport::Deprecation.silence do + get ':action' => FooController + end + end + + get '/foos_json?foo=heyo', as: :json + + assert_equal({ 'foo' => 'heyo' }, response.parsed_body) + end + end + private def post_to_foos(as:) with_routing do |routes| diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 0c3884cd38..5977124594 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -205,7 +205,7 @@ module ActionController def overfill_buffer_and_die logger = ActionController::Base.logger || Logger.new($stdout) response.stream.on_error do - logger.warn 'Error while streaming' + logger.warn 'Error while streaming.' error_latch.count_down end @@ -246,7 +246,8 @@ module ActionController def assert_stream_closed assert response.stream.closed?, 'stream should be closed' - assert response.sent?, 'stream should be sent' + assert response.committed?, 'response should be committed' + assert response.sent?, 'response should be sent' end def capture_log_output diff --git a/actionpack/test/controller/metal/renderers_test.rb b/actionpack/test/controller/metal/renderers_test.rb index 007866a559..247e872674 100644 --- a/actionpack/test/controller/metal/renderers_test.rb +++ b/actionpack/test/controller/metal/renderers_test.rb @@ -1,6 +1,12 @@ require 'abstract_unit' require 'active_support/core_ext/hash/conversions' +class MetalRenderingController < ActionController::Metal + include AbstractController::Rendering + include ActionController::Rendering + include ActionController::Renderers +end + class MetalRenderingJsonController < MetalRenderingController class Model def to_json(options = {}) diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index d0c7b2e06a..993f4001de 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -74,6 +74,14 @@ class RespondToController < ActionController::Base end end + def missing_templates + respond_to do |type| + # This test requires a block that is empty + type.json { } + type.xml + end + end + def using_defaults_with_type_list respond_to(:html, :xml) end @@ -624,6 +632,13 @@ class RespondToControllerTest < ActionController::TestCase end end + def test_missing_templates + get :missing_templates, format: :json + assert_response :no_content + get :missing_templates, format: :xml + assert_response :no_content + end + def test_invalid_variant assert_raises(ActionController::UnknownFormat) do get :variant_with_implicit_template_rendering, params: { v: :invalid } diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 96048e2868..2eed2996f6 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -360,4 +360,19 @@ class ParametersPermitTest < ActiveSupport::TestCase assert @params.include? 'person' assert_not @params.include? :gorilla end + + test "scalar values should be filtered when array or hash is specified" do + params = ActionController::Parameters.new(foo: "bar") + + assert params.permit(:foo).has_key?(:foo) + refute params.permit(foo: []).has_key?(:foo) + refute params.permit(foo: [:bar]).has_key?(:foo) + refute params.permit(foo: :bar).has_key?(:foo) + end + + test '#permitted? is false by default' do + params = ActionController::Parameters.new + + assert_equal false, params.permitted? + end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index e10d4449f3..4af3c628d0 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -1,5 +1,23 @@ require 'abstract_unit' +class Workshop + extend ActiveModel::Naming + include ActiveModel::Conversion + attr_accessor :id + + def initialize(id) + @id = id + end + + def persisted? + id.present? + end + + def to_s + id.to_s + end +end + class RedirectController < ActionController::Base # empty method not used anywhere to ensure methods like # `status` and `location` aren't called on `redirect_to` calls @@ -176,7 +194,6 @@ class RedirectTest < ActionController::TestCase assert_equal "http://www.example.com", redirect_to_url end - def test_relative_url_redirect_with_status get :relative_url_redirect_with_status assert_response 302 @@ -313,7 +330,7 @@ class RedirectTest < ActionController::TestCase error = assert_raise(ArgumentError) do get :redirect_to_params end - assert_equal "Generating a URL from non sanitized request parameters is insecure!", error.message + assert_equal ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE, error.message end def test_redirect_to_with_block diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 82fc8b0f8a..652c06af19 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -2,6 +2,9 @@ require 'abstract_unit' require 'controller/fake_models' class TestControllerWithExtraEtags < ActionController::Base + def self.controller_name; 'test'; end + def self.controller_path; 'test'; end + etag { nil } etag { 'ab' } etag { :cde } @@ -16,11 +19,19 @@ class TestControllerWithExtraEtags < ActionController::Base render plain: "stale" if stale?(etag: %w(1 2 3), template: false) end + def strong + render plain: "stale" if stale?(strong_etag: 'strong', template: false) + end + def with_template if stale? template: 'test/hello_world' render plain: 'stale' end end + + def with_implicit_template + fresh_when(etag: '123') + end end class ImplicitRenderTestController < ActionController::Base @@ -385,7 +396,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified_but_etag_differs @request.if_modified_since = @last_modified - @request.if_none_match = "234" + @request.if_none_match = '"234"' get :conditional_hello assert_response :success end @@ -414,7 +425,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified_but_etag_differs_with_record @request.if_modified_since = @last_modified - @request.if_none_match = "234" + @request.if_none_match = '"234"' get :conditional_hello_with_record assert_response :success end @@ -442,7 +453,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified_but_etag_differs_with_collection_of_records @request.if_modified_since = @last_modified - @request.if_none_match = "234" + @request.if_none_match = '"234"' get :conditional_hello_with_collection_of_records assert_response :success end @@ -477,8 +488,26 @@ end class EtagRenderTest < ActionController::TestCase tests TestControllerWithExtraEtags + def test_strong_etag + @request.if_none_match = strong_etag(['strong', 'ab', :cde, [:f]]) + get :strong + assert_response :not_modified + + @request.if_none_match = '*' + get :strong + assert_response :not_modified + + @request.if_none_match = '"strong"' + get :strong + assert_response :ok + + @request.if_none_match = weak_etag(['strong', 'ab', :cde, [:f]]) + get :strong + assert_response :ok + end + def test_multiple_etags - @request.if_none_match = etag(["123", 'ab', :cde, [:f]]) + @request.if_none_match = weak_etag(["123", 'ab', :cde, [:f]]) get :fresh assert_response :not_modified @@ -488,7 +517,7 @@ class EtagRenderTest < ActionController::TestCase end def test_array - @request.if_none_match = etag([%w(1 2 3), 'ab', :cde, [:f]]) + @request.if_none_match = weak_etag([%w(1 2 3), 'ab', :cde, [:f]]) get :array assert_response :not_modified @@ -506,26 +535,49 @@ class EtagRenderTest < ActionController::TestCase get :with_template assert_response :not_modified - # Modify the template digest - path = File.expand_path('../../fixtures/test/hello_world.erb', __FILE__) - old = File.read(path) - - begin - File.write path, 'foo' - ActionView::LookupContext::DetailsKey.clear - + modify_template(:hello_world) do request.if_none_match = etag get :with_template assert_response :ok assert_not_equal etag, @response.etag - ensure - File.write path, old end end - def etag(record) - %(W/"#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") + def test_etag_reflects_implicit_template_digest + get :with_implicit_template + assert_response :ok + assert_not_nil etag = @response.etag + + request.if_none_match = etag + get :with_implicit_template + assert_response :not_modified + + modify_template(:with_implicit_template) do + request.if_none_match = etag + get :with_implicit_template + assert_response :ok + assert_not_equal etag, @response.etag + end end + + private + def weak_etag(record) + "W/#{strong_etag record}" + end + + def strong_etag(record) + %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") + end + + def modify_template(name) + path = File.expand_path("../../fixtures/test/#{name}.erb", __FILE__) + original = File.read(path) + File.write(path, "#{original} Modified!") + ActionView::LookupContext::DetailsKey.clear + yield + ensure + File.write(path, original) + end end class MetalRenderTest < ActionController::TestCase @@ -537,6 +589,13 @@ class MetalRenderTest < ActionController::TestCase end end +class ActionControllerBaseRenderTest < ActionController::TestCase + def test_direct_render_to_string + ac = ActionController::Base.new() + assert_equal "Hello world!", ac.render_to_string(template: 'test/hello_world') + end +end + class ImplicitRenderTest < ActionController::TestCase tests ImplicitRenderTestController @@ -713,20 +772,24 @@ class HttpCacheForeverTest < ActionController::TestCase def test_cache_with_public get :cache_me_forever, params: {public: true} + assert_response :ok assert_equal "max-age=#{100.years}, public", @response.headers["Cache-Control"] assert_not_nil @response.etag + assert @response.weak_etag? end def test_cache_with_private get :cache_me_forever + assert_response :ok assert_equal "max-age=#{100.years}, private", @response.headers["Cache-Control"] assert_not_nil @response.etag - assert_response :success + assert @response.weak_etag? end def test_cache_response_code_with_if_modified_since get :cache_me_forever - assert_response :success + assert_response :ok + @request.if_modified_since = @response.headers['Last-Modified'] get :cache_me_forever assert_response :not_modified @@ -734,13 +797,10 @@ class HttpCacheForeverTest < ActionController::TestCase def test_cache_response_code_with_etag get :cache_me_forever - assert_response :success - @request.if_modified_since = @response.headers['Last-Modified'] - @request.if_none_match = @response.etag + assert_response :ok + @request.if_none_match = @response.etag get :cache_me_forever assert_response :not_modified - @request.if_modified_since = @response.headers['Last-Modified'] - @request.if_none_match = @response.etag end end diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index 16d24fa82a..372c09bc23 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -87,6 +87,14 @@ class RendererTest < ActiveSupport::TestCase assert_equal "<p>1\n<br />2</p>", render[inline: '<%= simple_format "1\n2" %>'] end + test 'rendering with user specified defaults' do + ApplicationController.renderer.defaults.merge!({ hello: 'hello', https: true }) + renderer = ApplicationController.renderer.new + content = renderer.render inline: '<%= request.ssl? %>' + + assert_equal 'true', content + end + private def render @render ||= ApplicationController.renderer.method(:render) diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index f7dcbc1984..d3f2ec6aa1 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -407,6 +407,37 @@ module RequestForgeryProtectionTests end end + def test_should_warn_on_not_same_origin_js + old_logger = ActionController::Base.logger + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + ActionController::Base.logger = logger + + begin + assert_cross_origin_blocked { get :same_origin_js } + + assert_equal 1, logger.logged(:warn).size + assert_match(/<script> tag on another site requested protected JavaScript/, logger.logged(:warn).last) + ensure + ActionController::Base.logger = old_logger + end + end + + def test_should_not_warn_if_csrf_logging_disabled_and_not_same_origin_js + old_logger = ActionController::Base.logger + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + ActionController::Base.logger = logger + ActionController::Base.log_warning_on_csrf_failure = false + + begin + assert_cross_origin_blocked { get :same_origin_js } + + assert_equal 0, logger.logged(:warn).size + ensure + ActionController::Base.logger = old_logger + ActionController::Base.log_warning_on_csrf_failure = true + end + end + # Allow non-GET requests since GET is all a remote <script> tag can muster. def test_should_allow_non_get_js_without_xhr_header session[:_csrf_token] = @token @@ -781,6 +812,19 @@ class PerFormTokensControllerTest < ActionController::TestCase assert_response :success end + def test_ignores_origin_during_generation + get :index, params: {form_path: 'https://example.com/per_form_tokens/post_one/'} + + form_token = assert_presence_and_fetch_form_csrf_token + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + post :post_one, params: {custom_authenticity_token: form_token} + end + assert_response :success + end + def test_ignores_trailing_slash_during_validation get :index diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index f42bef883f..c088e5a043 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -131,20 +131,22 @@ class RescueController < ActionController::Base def missing_template end - def io_error_in_view - begin - raise IOError.new('this is io error') - rescue - raise ActionView::TemplateError.new(nil) - end + def exception_with_more_specific_handler_for_wrapper + raise RecordInvalid + rescue + raise NotAuthorized end - def zero_division_error_in_view - begin - raise ZeroDivisionError.new('this is zero division error') - rescue - raise ActionView::TemplateError.new(nil) - end + def exception_with_more_specific_handler_for_cause + raise NotAuthorized + rescue + raise RecordInvalid + end + + def exception_with_no_handler_for_wrapper + raise RecordInvalid + rescue + raise RangeError end protected @@ -233,17 +235,6 @@ class ControllerInheritanceRescueControllerTest < ActionController::TestCase end class RescueControllerTest < ActionController::TestCase - - def test_io_error_in_view - get :io_error_in_view - assert_equal 'io error', @response.body - end - - def test_zero_division_error_in_view - get :zero_division_error_in_view - assert_equal 'action_view templater error', @response.body - end - def test_rescue_handler get :not_authorized assert_response :forbidden @@ -258,7 +249,6 @@ class RescueControllerTest < ActionController::TestCase get :record_invalid end end - def test_rescue_handler_with_argument_as_string assert_called_with @controller, :show_errors, [Exception] do get :record_invalid_raise_as_string @@ -296,11 +286,25 @@ class RescueControllerTest < ActionController::TestCase get :resource_unavailable assert_equal "RescueController::ResourceUnavailable", @response.body end - def test_block_rescue_handler_with_argument_as_string get :resource_unavailable_raise_as_string assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body end + + test 'rescue when wrapper has more specific handler than cause' do + get :exception_with_more_specific_handler_for_wrapper + assert_response :forbidden + end + + test 'rescue when cause has more specific handler than wrapper' do + get :exception_with_more_specific_handler_for_cause + assert_response :unprocessable_entity + end + + test 'rescue when cause has handler, but wrapper doesnt' do + get :exception_with_no_handler_for_wrapper + assert_response :unprocessable_entity + end end class RescueTest < ActionDispatch::IntegrationTest diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 4490abf7b2..8e38af5025 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -3,8 +3,22 @@ require 'active_support/core_ext/object/try' require 'active_support/core_ext/object/with_options' require 'active_support/core_ext/array/extract_options' -class ResourcesTest < ActionController::TestCase +class AdminController < ResourcesController; end +class MessagesController < ResourcesController; end +class ProductsController < ResourcesController; end +class ThreadsController < ResourcesController; end + +module Backoffice + class ProductsController < ResourcesController; end + class ImagesController < ResourcesController; end + + module Admin + class ProductsController < ResourcesController; end + class ImagesController < ResourcesController; end + end +end +class ResourcesTest < ActionController::TestCase def test_default_restful_routes with_restful_routing :messages do assert_simply_restful_for :messages diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index c477b4156c..03bf8f8295 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -626,7 +626,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal '/pages/boo', url_for(rs, { :controller => 'pages', :action => 'boo' }) end - def test_route_with_fixnum_default + def test_route_with_integer_default rs.draw do get 'page(/:id)' => 'content#show_page', :id => 1 @@ -2064,11 +2064,11 @@ class RackMountIntegrationTests < ActiveSupport::TestCase def test_extras params = {:controller => 'people'} assert_equal [], @routes.extra_keys(params) - assert_equal({:controller => 'people'}, params) + assert_equal({:controller => 'people', :action => 'index'}, params) params = {:controller => 'people', :foo => 'bar'} assert_equal [:foo], @routes.extra_keys(params) - assert_equal({:controller => 'people', :foo => 'bar'}, params) + assert_equal({:controller => 'people', :action => 'index', :foo => 'bar'}, params) params = {:controller => 'people', :action => 'create', :person => { :name => 'Josh'}} assert_equal [:person], @routes.extra_keys(params) diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 2820425c31..9df70dacbf 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -11,6 +11,8 @@ class SendFileController < ActionController::Base include ActionController::Testing layout "layouts/standard" # to make sure layouts don't interfere + before_action :file, only: :file_from_before_action + attr_writer :options def options @options ||= {} @@ -20,6 +22,10 @@ class SendFileController < ActionController::Base send_file(file_path, options) end + def file_from_before_action + raise 'No file sent from before action.' + end + def test_send_file_headers_bang options = { :type => Mime[:png], @@ -192,6 +198,15 @@ class SendFileTest < ActionController::TestCase assert_nil @controller.headers['Content-Disposition'] end + def test_send_file_from_before_action + response = nil + assert_nothing_raised { response = process('file_from_before_action') } + assert_not_nil response + + assert_kind_of String, response.body + assert_equal file_data, response.body + end + %w(file data).each do |method| define_method "test_send_#{method}_status" do @controller.options = { :stream => false, :status => 500 } diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index ebcdda6074..ea59156f65 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -154,6 +154,10 @@ XML render html: '<body class="foo"></body>'.html_safe end + def boom + raise 'boom!' + end + private def generate_url(opts) @@ -553,7 +557,7 @@ XML assert_equal 'created', flash[:notice] end - def test_params_passing_with_fixnums + def test_params_passing_with_integer get :test_params, params: { page: { name: "Page name", month: 4, year: 2004, day: 6 } } @@ -565,7 +569,7 @@ XML ) end - def test_params_passing_with_fixnums_when_not_html_request + def test_params_passing_with_integers_when_not_html_request get :test_params, params: { format: 'json', count: 999 } parsed_params = ::JSON.parse(@response.body) assert_equal( @@ -981,6 +985,26 @@ XML assert_redirected_to 'created resource' end end + + def test_exception_in_action_reaches_test + assert_raise(RuntimeError) do + process :boom, method: "GET" + end + end + + def test_request_state_is_cleared_after_exception + assert_raise(RuntimeError) do + process :boom, + method: "GET", + params: { q: 'test1' } + end + + process :test_query_string, + method: "GET", + params: { q: 'test2' } + + assert_equal "q=test2", @response.body + end end class ResponseDefaultHeadersTest < ActionController::TestCase diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index dfcef14344..c7194cde4a 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -520,7 +520,7 @@ class CookiesTest < ActionController::TestCase def test_accessing_nonexistent_signed_cookie_should_not_raise_an_invalid_signature get :set_signed_cookie - assert_nil @controller.send(:cookies).signed[:non_existant_attribute] + assert_nil @controller.send(:cookies).signed[:non_existent_attribute] end def test_encrypted_cookie_using_default_serializer @@ -638,7 +638,7 @@ class CookiesTest < ActionController::TestCase def test_accessing_nonexistent_encrypted_cookie_should_not_raise_invalid_message get :set_encrypted_cookie - assert_nil @controller.send(:cookies).encrypted[:non_existant_attribute] + assert_nil @controller.send(:cookies).encrypted[:non_existent_attribute] end def test_setting_invalid_encrypted_cookie_should_return_nil_when_accessing_it diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 159bf10545..6f3d30ebf9 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -27,37 +27,37 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest env['action_dispatch.show_detailed_exceptions'] = @detailed req = ActionDispatch::Request.new(env) case req.path - when "/pass" + when %r{/pass} [404, { "X-Cascade" => "pass" }, self] - when "/not_found" + when %r{/not_found} raise AbstractController::ActionNotFound - when "/runtime_error" + when %r{/runtime_error} raise RuntimeError - when "/method_not_allowed" + when %r{/method_not_allowed} raise ActionController::MethodNotAllowed - when "/unknown_http_method" + when %r{/unknown_http_method} raise ActionController::UnknownHttpMethod - when "/not_implemented" + when %r{/not_implemented} raise ActionController::NotImplemented - when "/unprocessable_entity" + when %r{/unprocessable_entity} raise ActionController::InvalidAuthenticityToken - when "/not_found_original_exception" + when %r{/not_found_original_exception} begin raise AbstractController::ActionNotFound.new rescue raise ActionView::Template::Error.new('template') end - when "/missing_template" + when %r{/missing_template} raise ActionView::MissingTemplate.new(%w(foo), 'foo/index', %w(foo), false, 'mailer') - when "/bad_request" + when %r{/bad_request} raise ActionController::BadRequest - when "/missing_keys" + when %r{/missing_keys} raise ActionController::UrlGenerationError, "No route matches" - when "/parameter_missing" + when %r{/parameter_missing} raise ActionController::ParameterMissing, :missing_param_key - when "/original_syntax_error" + when %r{/original_syntax_error} eval 'broke_syntax =' # `eval` need for raise native SyntaxError at runtime - when "/syntax_error_into_view" + when %r{/syntax_error_into_view} begin eval 'broke_syntax =' rescue Exception @@ -67,7 +67,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest {}) raise ActionView::Template::Error.new(template) end - when "/framework_raises" + when %r{/framework_raises} method_that_raises else raise "puke!" @@ -75,13 +75,6 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end - class BoomerAPI < Boomer - def call(env) - env['action_dispatch.show_detailed_exceptions'] = @detailed - raise "puke!" - end - end - RoutesApp = Struct.new(:routes).new(SharedTestRoutes) ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp) DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp) @@ -212,61 +205,60 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/ActionController::ParameterMissing/, body) end - test "rescue with json error for API request" do + test "rescue with JSON error for JSON API request" do @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) - get "/", headers: { 'action_dispatch.show_exceptions' => true } + get "/", headers: { 'action_dispatch.show_exceptions' => true }, as: :json assert_response 500 assert_no_match(/<header>/, body) assert_no_match(/<body>/, body) assert_equal "application/json", response.content_type assert_match(/RuntimeError: puke/, body) - get "/not_found", headers: { 'action_dispatch.show_exceptions' => true } + get "/not_found", headers: { 'action_dispatch.show_exceptions' => true }, as: :json assert_response 404 assert_no_match(/<body>/, body) assert_equal "application/json", response.content_type assert_match(/#{AbstractController::ActionNotFound.name}/, body) - get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true } + get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true }, as: :json assert_response 405 assert_no_match(/<body>/, body) assert_equal "application/json", response.content_type assert_match(/ActionController::MethodNotAllowed/, body) - get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true } + get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true }, as: :json assert_response 405 assert_no_match(/<body>/, body) assert_equal "application/json", response.content_type assert_match(/ActionController::UnknownHttpMethod/, body) - get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true } + get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true }, as: :json assert_response 400 assert_no_match(/<body>/, body) assert_equal "application/json", response.content_type assert_match(/ActionController::BadRequest/, body) - get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true } + get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true }, as: :json assert_response 400 assert_no_match(/<body>/, body) assert_equal "application/json", response.content_type assert_match(/ActionController::ParameterMissing/, body) end - test "rescue with json on API request returns only allowed formats or json as a fallback" do + test "rescue with HTML format for HTML API request" do @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) - get "/index.json", headers: { 'action_dispatch.show_exceptions' => true } - assert_response 500 - assert_equal "application/json", response.content_type - assert_match(/RuntimeError: puke/, body) - get "/index.html", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 - assert_no_match(/<header>/, body) - assert_no_match(/<body>/, body) - assert_equal "application/json", response.content_type - assert_match(/RuntimeError: puke/, body) + assert_match(/<header>/, body) + assert_match(/<body>/, body) + assert_equal "text/html", response.content_type + assert_match(/puke/, body) + end + + test "rescue with XML format for XML API requests" do + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) get "/index.xml", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 @@ -274,6 +266,25 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/RuntimeError: puke/, body) end + test "rescue with JSON format as fallback if API request format is not supported" do + begin + Mime::Type.register 'text/wibble', :wibble + + ActionDispatch::IntegrationTest.register_encoder(:wibble, + param_encoder: -> params { params }) + + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) + + get "/index", headers: { 'action_dispatch.show_exceptions' => true }, as: :wibble + assert_response 500 + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + ensure + Mime::Type.unregister :wibble + end + end + test "does not show filtered parameters" do @app = DevelopmentApp @@ -351,6 +362,29 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/puke/, output.rewind && output.read) end + test 'logs only what is necessary' do + @app = DevelopmentApp + io = StringIO.new + logger = ActiveSupport::Logger.new(io) + + _old, ActionView::Base.logger = ActionView::Base.logger, logger + begin + get "/", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => logger } + ensure + ActionView::Base.logger = _old + end + + output = io.rewind && io.read + lines = output.lines + + # Other than the first three... + assert_equal([" \n", "RuntimeError (puke!):\n", " \n"], lines.slice!(0, 3)) + lines.each do |line| + # .. all the remaining lines should be from the backtrace + assert_match(/:\d+:in /, line) + end + end + test 'uses backtrace cleaner from env' do @app = DevelopmentApp backtrace_cleaner = ActiveSupport::BacktraceCleaner.new diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index e4475f4233..de57c4be1d 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -65,7 +65,7 @@ module ActionController latch = Concurrent::CountDownLatch.new t = Thread.new { - @response.stream.each do + @response.each do latch.count_down end } diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index df27e41997..f6dd9272a6 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -102,6 +102,18 @@ module ActionDispatch assert_equal("PUT", fakeset.routes.first.verb) end + def test_to_scope + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.scope(to: "posts#index") do + mapper.get :all + mapper.post :most + end + + assert_equal "posts#index", fakeset.routes.to_a[0].defaults[:to] + assert_equal "posts#index", fakeset.routes.to_a[1].defaults[:to] + end + def test_map_slash fakeset = FakeSet.new mapper = Mapper.new fakeset @@ -178,6 +190,19 @@ module ActionDispatch mapper.mount as: "exciting" end end + + def test_scope_does_not_destructively_mutate_default_options + fakeset = FakeSet.new + mapper = Mapper.new fakeset + + frozen = { foo: :bar }.freeze + + assert_nothing_raised do + mapper.scope(defaults: frozen) do + # pass + end + end + end end end end diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index d75e31db62..b8f0ffb64a 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -166,7 +166,7 @@ module TestGenerationPrefix assert_equal "/pure-awesomeness/blog/posts/1", last_response.body end - test "[ENGINE] url_helpers from engine have higher priotity than application's url_helpers" do + test "[ENGINE] url_helpers from engine have higher priority than application's url_helpers" do get "/awesome/blog/conflicting_url" assert_equal "engine", last_response.body end @@ -319,14 +319,14 @@ module TestGenerationPrefix path = engine_object.polymorphic_url(Post.new, :host => "www.example.com") assert_equal "http://www.example.com/awesome/blog/posts/1", path end - + private def verify_redirect(url, status = 301) assert_equal status, last_response.status assert_equal url, last_response.headers["Location"] assert_equal expected_redirect_body(url), last_response.body end - + def expected_redirect_body(url) %(<html><body>You are being <a href="#{url}">redirected</a>.</body></html>) end @@ -450,14 +450,14 @@ module TestGenerationPrefix get "/absolute_custom_redirect" verify_redirect "http://example.org/foo" end - + private def verify_redirect(url, status = 301) assert_equal status, last_response.status assert_equal url, last_response.headers["Location"] assert_equal expected_redirect_body(url), last_response.body end - + def expected_redirect_body(url) %(<html><body>You are being <a href="#{url}">redirected</a>.</body></html>) end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index a07138b55e..6ab71ebc81 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -155,7 +155,7 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest test "parses json params after custom json mime type registered" do begin Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) assert_parses( {"user" => {"username" => "meinac"}, "username" => "meinac"}, "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } @@ -169,10 +169,10 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest test "parses json params after custom json mime type registered with synonym" do begin Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) assert_parses( {"user" => {"username" => "meinac"}, "username" => "meinac"}, - "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.rails+json' } ) ensure Mime::Type.unregister :json diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index 7dcbcc5c21..e022e7e21e 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -114,5 +114,31 @@ module ActionDispatch }.new end end + + class SessionIntegrationTest < ActionDispatch::IntegrationTest + class MySessionApp + def call(env) + request = Rack::Request.new(env) + request.session['hello'] = 'Hello from MySessionApp!' + [ 200, {}, ['Hello from MySessionApp!'] ] + end + end + + Router = ActionDispatch::Routing::RouteSet.new + Router.draw do + get '/mysessionapp' => MySessionApp.new + end + + def app + @app ||= RoutedRackApp.new(Router) + end + + def test_session_follows_rack_api_contract_1 + get '/mysessionapp' + assert_response :ok + assert_equal 'Hello from MySessionApp!', @response.body + assert_equal 'Hello from MySessionApp!', session['hello'] + end + end end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 0edad72fd9..8a5d85ab84 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -358,6 +358,17 @@ class RequestPort < BaseRequestTest request = stub_request 'HTTP_HOST' => 'www.example.org:8080' assert_equal ':8080', request.port_string end + + test "server port" do + request = stub_request 'SERVER_PORT' => '8080' + assert_equal 8080, request.server_port + + request = stub_request 'SERVER_PORT' => '80' + assert_equal 80, request.server_port + + request = stub_request 'SERVER_PORT' => '' + assert_equal 0, request.server_port + end end class RequestPath < BaseRequestTest @@ -417,6 +428,11 @@ class RequestPath < BaseRequestTest end class RequestHost < BaseRequestTest + test "host without specifying port" do + request = stub_request 'HTTP_HOST' => 'rubyonrails.org' + assert_equal "rubyonrails.org", request.host_with_port + end + test "host with default port" do request = stub_request 'HTTP_HOST' => 'rubyonrails.org:80' assert_equal "rubyonrails.org", request.host_with_port @@ -427,6 +443,21 @@ class RequestHost < BaseRequestTest assert_equal "rubyonrails.org:81", request.host_with_port end + test "raw without specifying port" do + request = stub_request 'HTTP_HOST' => 'rubyonrails.org' + assert_equal "rubyonrails.org", request.raw_host_with_port + end + + test "raw host with default port" do + request = stub_request 'HTTP_HOST' => 'rubyonrails.org:80' + assert_equal "rubyonrails.org:80", request.raw_host_with_port + end + + test "raw host with non default port" do + request = stub_request 'HTTP_HOST' => 'rubyonrails.org:81' + assert_equal "rubyonrails.org:81", request.raw_host_with_port + end + test "proxy request" do request = stub_request 'HTTP_HOST' => 'glu.ttono.us:80' assert_equal "glu.ttono.us", request.host_with_port @@ -1152,36 +1183,41 @@ class RequestParameterFilter < BaseRequestTest end class RequestEtag < BaseRequestTest - test "if_none_match_etags none" do + test "always matches *" do + request = stub_request('HTTP_IF_NONE_MATCH' => '*') + + assert_equal '*', request.if_none_match + assert_equal ['*'], request.if_none_match_etags + + assert request.etag_matches?('"strong"') + assert request.etag_matches?('W/"weak"') + assert_not request.etag_matches?(nil) + end + + test "doesn't match absent If-None-Match" do request = stub_request assert_equal nil, request.if_none_match assert_equal [], request.if_none_match_etags - assert !request.etag_matches?("foo") - assert !request.etag_matches?(nil) - end - - test "if_none_match_etags single" do - header = 'the-etag' - request = stub_request('HTTP_IF_NONE_MATCH' => header) - assert_equal header, request.if_none_match - assert_equal [header], request.if_none_match_etags - assert request.etag_matches?("the-etag") + assert_not request.etag_matches?("foo") + assert_not request.etag_matches?(nil) end - test "if_none_match_etags quoted single" do + test "matches opaque ETag validators without unquoting" do header = '"the-etag"' request = stub_request('HTTP_IF_NONE_MATCH' => header) assert_equal header, request.if_none_match - assert_equal ['the-etag'], request.if_none_match_etags - assert request.etag_matches?("the-etag") + assert_equal ['"the-etag"'], request.if_none_match_etags + + assert request.etag_matches?('"the-etag"') + assert_not request.etag_matches?("the-etag") end test "if_none_match_etags multiple" do header = 'etag1, etag2, "third etag", "etag4"' - expected = ['etag1', 'etag2', 'third etag', 'etag4'] + expected = ['etag1', 'etag2', '"third etag"', '"etag4"'] request = stub_request('HTTP_IF_NONE_MATCH' => header) assert_equal header, request.if_none_match diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index cd385982d9..aa90433505 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -37,6 +37,39 @@ class ResponseTest < ActiveSupport::TestCase assert_equal "closed stream", e.message end + def test_each_isnt_called_if_str_body_is_written + # Controller writes and reads response body + each_counter = 0 + @response.body = Object.new.tap {|o| o.singleton_class.send(:define_method, :each) { |&block| each_counter += 1; block.call 'foo' } } + @response['X-Foo'] = @response.body + + assert_equal 1, each_counter, "#each was not called once" + + # Build response + status, headers, body = @response.to_a + + assert_equal 200, status + assert_equal "foo", headers['X-Foo'] + assert_equal "foo", body.each.to_a.join + + # Show that #each was not called twice + assert_equal 1, each_counter, "#each was not called once" + end + + def test_set_header_after_read_body_during_action + @response.body + + # set header after the action reads back @response.body + @response['x-header'] = "Best of all possible worlds." + + # the response can be built. + status, headers, body = @response.to_a + assert_equal 200, status + assert_equal "", body.body + + assert_equal "Best of all possible worlds.", headers['x-header'] + end + def test_read_body_during_action @response.body = "Hello, World!" @@ -189,7 +222,7 @@ class ResponseTest < ActiveSupport::TestCase assert_equal({"user_name" => "david", "login" => nil}, @response.cookies) end - test "read cache control" do + test "read ETag and Cache-Control" do resp = ActionDispatch::Response.new.tap { |response| response.cache_control[:public] = true response.etag = '123' @@ -197,6 +230,9 @@ class ResponseTest < ActiveSupport::TestCase } resp.to_a + assert resp.etag? + assert resp.weak_etag? + assert_not resp.strong_etag? assert_equal('W/"202cb962ac59075b964b07152d234b70"', resp.etag) assert_equal({:public => true}, resp.cache_control) @@ -204,6 +240,20 @@ class ResponseTest < ActiveSupport::TestCase assert_equal('W/"202cb962ac59075b964b07152d234b70"', resp.headers['ETag']) end + test "read strong ETag" do + resp = ActionDispatch::Response.new.tap { |response| + response.cache_control[:public] = true + response.strong_etag = '123' + response.body = 'Hello' + } + resp.to_a + + assert resp.etag? + assert_not resp.weak_etag? + assert resp.strong_etag? + assert_equal('"202cb962ac59075b964b07152d234b70"', resp.etag) + end + test "read charset and content type" do resp = ActionDispatch::Response.new.tap { |response| response.charset = 'utf-16' @@ -446,11 +496,19 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end - test "we can set strong ETag by directly adding it as header" do - @response = ActionDispatch::Response.create - @response.add_header "ETag", '"202cb962ac59075b964b07152d234b70"' + test "strong ETag validator" do + @app = lambda { |env| + ActionDispatch::Response.new.tap { |resp| + resp.strong_etag = '123' + resp.body = 'Hello' + resp.request = ActionDispatch::Request.empty + }.to_a + } + + get '/' + assert_response :ok - assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) assert_equal('"202cb962ac59075b964b07152d234b70"', @response.headers['ETag']) + assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) end end diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 7ef513b0c8..6934271846 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -1,5 +1,7 @@ require 'abstract_unit' +class ReviewsController < ResourcesController; end + class RoutingConcernsTest < ActionDispatch::IntegrationTest class Reviewable def self.call(mapper, options = {}) diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index 9d0d23d6de..5aafcb23c2 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -347,7 +347,7 @@ module ActionDispatch end assert_equal ["Prefix Verb URI Pattern Controller#Action", - " GET /:controller(/:action) (?-mix:api\\/[^\\/]+)#:action"], output + " GET /:controller(/:action) :controller#:action"], output end def test_inspect_routes_shows_resources_route_when_assets_disabled diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 09830c0c46..75fdc9469f 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3991,16 +3991,6 @@ class TestUnicodePaths < ActionDispatch::IntegrationTest end class TestMultipleNestedController < ActionDispatch::IntegrationTest - module ::Foo - module Bar - class BazController < ActionController::Base - def index - render :inline => "<%= url_for :controller => '/pooh', :action => 'index' %>" - end - end - end - end - Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| app.draw do namespace :foo do @@ -4012,7 +4002,18 @@ class TestMultipleNestedController < ActionDispatch::IntegrationTest end end - include Routes.url_helpers + module ::Foo + module Bar + class BazController < ActionController::Base + include Routes.url_helpers + + def index + render :inline => "<%= url_for :controller => '/pooh', :action => 'index' %>" + end + end + end + end + APP = build_app Routes def app; APP end @@ -4755,3 +4756,72 @@ class TestPartialDynamicPathSegments < ActionDispatch::IntegrationTest assert_equal(params, request.path_parameters) end end + +class TestPathParameters < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| + app.draw do + scope module: 'test_path_parameters' do + scope ':locale', locale: /en|ar/ do + root to: 'home#index' + get '/about', to: 'pages#about' + end + end + + get ':controller(/:action/(:id))' + end + end + + class HomeController < ActionController::Base + include Routes.url_helpers + + def index + render inline: "<%= root_path %>" + end + end + + class PagesController < ActionController::Base + include Routes.url_helpers + + def about + render inline: "<%= root_path(locale: :ar) %> | <%= url_for(locale: :ar) %>" + end + end + + APP = build_app Routes + def app; APP end + + def test_path_parameters_are_not_mutated + get '/en/about' + assert_equal "/ar | /ar/about", @response.body + end +end + +class TestInternalRoutingParams < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| + app.draw do + get '/test_internal/:internal' => 'internal#internal' + end + end + + class ::InternalController < ActionController::Base + def internal + head :ok + end + end + + APP = build_app Routes + + def app + APP + end + + def test_paths_with_partial_dynamic_segments_are_recognised + get '/test_internal/123' + assert_equal 200, response.status + + assert_equal( + { controller: 'internal', action: 'internal', internal: '123' }, + request.path_parameters + ) + end +end diff --git a/actionpack/test/dispatch/test_request_test.rb b/actionpack/test/dispatch/test_request_test.rb index 51c469a61a..3c19cbd68a 100644 --- a/actionpack/test/dispatch/test_request_test.rb +++ b/actionpack/test/dispatch/test_request_test.rb @@ -88,6 +88,33 @@ class TestRequestTest < ActiveSupport::TestCase assert_equal 'GoogleBot', req.user_agent end + test "setter methods" do + req = ActionDispatch::TestRequest.create({}) + get = 'GET' + + [ + 'request_method=', 'host=', 'request_uri=', 'path=', 'if_modified_since=', 'if_none_match=', + 'remote_addr=', 'user_agent=', 'accept=' + ].each do |method| + req.send(method, get) + end + + req.port = 8080 + req.accept = 'hello goodbye' + + assert_equal(get, req.get_header('REQUEST_METHOD')) + assert_equal(get, req.get_header('HTTP_HOST')) + assert_equal(8080, req.get_header('SERVER_PORT')) + assert_equal(get, req.get_header('REQUEST_URI')) + assert_equal(get, req.get_header('PATH_INFO')) + assert_equal(get, req.get_header('HTTP_IF_MODIFIED_SINCE')) + assert_equal(get, req.get_header('HTTP_IF_NONE_MATCH')) + assert_equal(get, req.get_header('REMOTE_ADDR')) + assert_equal(get, req.get_header('HTTP_USER_AGENT')) + assert_nil(req.get_header('action_dispatch.request.accepts')) + assert_equal('hello goodbye', req.get_header('HTTP_ACCEPT')) + end + private def assert_cookies(expected, cookie_jar) assert_equal(expected, cookie_jar.instance_variable_get("@cookies")) diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb index 01453323ef..951c761995 100644 --- a/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb @@ -1,3 +1,3 @@ <body> -<%= cache 'with_options', skip_digest: true, expires_in: 1.minute do %><p>ERB</p><% end %> +<%= cache 'with_options', skip_digest: true, expires_in: 10 do %><p>ERB</p><% end %> </body> diff --git a/actionpack/test/fixtures/test/with_implicit_template.erb b/actionpack/test/fixtures/test/with_implicit_template.erb new file mode 100644 index 0000000000..474488cd13 --- /dev/null +++ b/actionpack/test/fixtures/test/with_implicit_template.erb @@ -0,0 +1 @@ +Hello explicitly! |