diff options
Diffstat (limited to 'actionpack')
146 files changed, 2804 insertions, 1776 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index f6ffe45490..be911b147c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,653 +1,2 @@ -* 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 601263bfac..4a05c067a9 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -5,6 +5,8 @@ test_files = Dir.glob('test/**/*_test.rb') desc "Default Task" task :default => :test +task :package + # Run the unit tests Rake::TestTask.new do |t| t.libs << 'test' diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 28d8bc3091..965fafff5f 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' @@ -24,7 +24,7 @@ Gem::Specification.new do |s| s.add_dependency 'rack', '~> 2.x' 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.rb b/actionpack/lib/abstract_controller.rb index 56c4033387..1e57cbaac4 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -6,6 +6,7 @@ module AbstractController extend ActiveSupport::Autoload autoload :Base + autoload :Caching autoload :Callbacks autoload :Collector autoload :DoubleRenderError, "abstract_controller/rendering" @@ -15,4 +16,9 @@ module AbstractController autoload :Translation autoload :AssetPaths autoload :UrlFor + + def self.eager_load! + super + AbstractController::Caching.eager_load! + end end 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/caching.rb b/actionpack/lib/abstract_controller/caching.rb new file mode 100644 index 0000000000..0dea50889a --- /dev/null +++ b/actionpack/lib/abstract_controller/caching.rb @@ -0,0 +1,62 @@ +module AbstractController + module Caching + extend ActiveSupport::Concern + extend ActiveSupport::Autoload + + eager_autoload do + autoload :Fragments + end + + module ConfigMethods + def cache_store + config.cache_store + end + + def cache_store=(store) + config.cache_store = ActiveSupport::Cache.lookup_store(store) + end + + private + def cache_configured? + perform_caching && cache_store + end + end + + include ConfigMethods + include AbstractController::Caching::Fragments + + included do + extend ConfigMethods + + config_accessor :default_static_extension + self.default_static_extension ||= '.html' + + config_accessor :perform_caching + self.perform_caching = true if perform_caching.nil? + + class_attribute :_view_cache_dependencies + self._view_cache_dependencies = [] + helper_method :view_cache_dependencies if respond_to?(:helper_method) + end + + module ClassMethods + def view_cache_dependency(&dependency) + self._view_cache_dependencies += [dependency] + end + end + + def view_cache_dependencies + self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact + end + + protected + # Convenience accessor. + def cache(key, options = {}, &block) + if cache_configured? + cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) + else + yield + end + end + end +end diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index b9ad51a9cf..3257a731ed 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -1,4 +1,4 @@ -module ActionController +module AbstractController module Caching # Fragment caching is used for caching various blocks within # views without caching the entire action as a whole. This is @@ -135,13 +135,8 @@ module ActionController end def instrument_fragment_cache(name, key) # :nodoc: - payload = { - controller: controller_name, - action: action_name, - key: key - } - - ActiveSupport::Notifications.instrument("#{name}.action_controller", payload) { yield } + payload = instrument_payload(key) + ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", payload) { yield } end end end 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/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index d84c238a62..ab4355296b 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -38,7 +38,8 @@ module AbstractController end # Declare a controller method as a helper. For example, the following - # makes the +current_user+ controller method available to the view: + # makes the +current_user+ and +logged_in?+ controller methods available + # to the view: # class ApplicationController < ActionController::Base # helper_method :current_user, :logged_in? # 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.rb b/actionpack/lib/action_controller.rb index 40f33a9de0..62f5905205 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -9,12 +9,15 @@ module ActionController autoload :API autoload :Base - autoload :Caching autoload :Metal autoload :Middleware autoload :Renderer autoload :FormBuilder + eager_autoload do + autoload :Caching + end + autoload_under "metal" do autoload :ConditionalGet autoload :Cookies @@ -47,11 +50,6 @@ module ActionController autoload :TestCase, 'action_controller/test_case' autoload :TemplateAssertions, 'action_controller/test_case' - - def self.eager_load! - super - ActionController::Caching.eager_load! - end end # Common Active Support usage in Action Controller 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..d546d7260c 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -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/caching.rb b/actionpack/lib/action_controller/caching.rb index 0b8fa2ea09..a9a8508abc 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -1,6 +1,3 @@ -require 'fileutils' -require 'uri' - module ActionController # \Caching is a cheap way of speeding up slow applications by keeping the result of # calculations, renderings, and database calls around for subsequent requests. @@ -23,65 +20,25 @@ module ActionController # config.action_controller.cache_store = :mem_cache_store, Memcached::Rails.new('localhost:11211') # config.action_controller.cache_store = MyOwnStore.new('parameter') module Caching - extend ActiveSupport::Concern extend ActiveSupport::Autoload - - eager_autoload do - autoload :Fragments - end - - module ConfigMethods - def cache_store - config.cache_store - end - - def cache_store=(store) - config.cache_store = ActiveSupport::Cache.lookup_store(store) - end - - private - def cache_configured? - perform_caching && cache_store - end - end - - include AbstractController::Callbacks - - include ConfigMethods - include Fragments + extend ActiveSupport::Concern included do - extend ConfigMethods - - config_accessor :default_static_extension - self.default_static_extension ||= '.html' - - config_accessor :perform_caching - self.perform_caching = true if perform_caching.nil? - - class_attribute :_view_cache_dependencies - self._view_cache_dependencies = [] - helper_method :view_cache_dependencies if respond_to?(:helper_method) + include AbstractController::Caching end - module ClassMethods - def view_cache_dependency(&dependency) - self._view_cache_dependencies += [dependency] - end - end + private - def view_cache_dependencies - self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact - end + def instrument_payload(key) + { + controller: controller_name, + action: action_name, + key: key + } + end - protected - # Convenience accessor. - def cache(key, options = {}, &block) - if cache_configured? - cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) - else - yield - end + def instrument_name + "action_controller" end end end diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 4c9f14e409..a0917b4fdb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -25,7 +25,9 @@ module ActionController status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" - message << " (#{additions.join(" | ".freeze)})" unless additions.blank? + message << " (#{additions.join(" | ".freeze)})" unless additions.empty? + message << "\n\n" if defined?(Rails.env) && Rails.env.development? + message end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index f6a93a8940..f6e67b02d7 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -174,10 +174,8 @@ module ActionController def response_body=(body) body = [body] unless body.nil? || body.respond_to?(:each) response.reset_body! - body.each { |part| - next if part.empty? - response.write part - } + return unless body + response.body = body super end diff --git a/actionpack/lib/action_controller/metal/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb index 6c6f8381ff..cef65a362c 100644 --- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb +++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb @@ -1,5 +1,5 @@ module ActionController - module BasicImplicitRender + module BasicImplicitRender # :nodoc: def send_action(method, *args) super.tap { default_render unless performed? } end diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index f8e0d9cf6c..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,12 +214,12 @@ 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 - # Sets a HTTP 1.1 Cache-Control header. Defaults to issuing a +private+ + # Sets an HTTP 1.1 Cache-Control header. Defaults to issuing a +private+ # instruction, so that intermediate caches must not cache the response. # # expires_in 20.minutes @@ -195,7 +229,7 @@ module ActionController # This method will overwrite an existing Cache-Control header. # See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities. # - # The method will also ensure a HTTP Date header for client compatibility. + # The method will also ensure an HTTP Date header for client compatibility. def expires_in(seconds, options = {}) response.cache_control.merge!( :max_age => seconds, @@ -208,7 +242,7 @@ module ActionController response.date = Time.now unless response.date? end - # Sets a HTTP 1.1 Cache-Control header of <tt>no-cache</tt> so no caching should + # Sets an HTTP 1.1 Cache-Control header of <tt>no-cache</tt> so no caching should # occur by the browser or intermediate caches (like caching proxy servers). def expires_now response.cache_control.replace(:no_cache => true) @@ -216,26 +250,23 @@ module ActionController # Cache or yield the block. The cache is supposed to never expire. # - # You can use this method when you have a HTTP response that never changes, + # You can use this method when you have an HTTP response that never changes, # and the browser and proxies should cache it indefinitely. # # * +public+: By default, HTTP responses are private, cached only on the # user's web browser. To allow proxies to cache the response, set +true+ to # indicate that they can serve the cached response to all users. - # - # * +version+: the version passed as a key for the cache. - def http_cache_forever(public: false, version: 'v1') + def http_cache_forever(public: false) expires_in 100.years, public: public - yield if stale?(etag: "#{version}-#{request.fullpath}", + yield if stale?(etag: request.fullpath, last_modified: Time.new(2011, 1, 1).utc, public: public) 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/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 17fcc2fa02..6192fc0f9c 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,29 +1,62 @@ +require 'active_support/core_ext/string/strip' + module ActionController + # Handles implicit rendering for a controller action that does not + # explicitly respond with +render+, +respond_to+, +redirect+, or +head+. + # + # For API controllers, the implicit response is always 204 No Content. + # + # 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 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 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 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, 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: include BasicImplicitRender - # Renders the template corresponding to the controller action, if it exists. - # The action name, format, and variant are all taken into account. - # For example, the "new" action with an HTML format and variant "phone" - # would try to render the <tt>new.html+phone.erb</tt> template. - # - # If no template is found <tt>ActionController::BasicImplicitRender</tt>'s implementation is called, unless - # a block is passed. In that case, it will override the super implementation. - # - # default_render do - # head 404 # No template was found - # end def default_render(*args) 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} 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 = "#{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 - if block_given? - yield(*args) - else - logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger - super - end + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + super end end @@ -32,5 +65,10 @@ module ActionController "default_render" end end + + private + def interactive_browser_request? + 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 3dbf34eb2a..624a6d5b76 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -19,9 +19,10 @@ module ActionController :controller => self.class.name, :action => self.action_name, :params => request.filtered_parameters, - :format => request.format.try(:ref), + :headers => request.headers, + :format => request.format.ref, :method => request.request_method, - :path => (request.fullpath rescue "unknown") + :path => request.fullpath } ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) @@ -74,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 e3c540bf5f..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 @@ -237,39 +237,55 @@ module ActionController # This processes the action in a child thread. It lets us return the # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client - Thread.new { - t2 = Thread.current - t2.abort_on_exception = true - - # Since we're processing the view in a different thread, copy the - # thread locals from the main thread to the child thread. :'( - locals.each { |k,v| t2[k] = v } - - begin - super(name) - rescue => e - if @_response.committed? - begin - @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html - @_response.stream.call_on_error - rescue => exception - log_error(exception) - ensure - log_error(e) - @_response.stream.close + new_controller_thread { + ActiveSupport::Dependencies.interlock.running do + t2 = Thread.current + + # Since we're processing the view in a different thread, copy the + # thread locals from the main thread to the child thread. :'( + locals.each { |k,v| t2[k] = v } + + begin + super(name) + rescue => e + if @_response.committed? + begin + @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html + @_response.stream.call_on_error + rescue => exception + log_error(exception) + ensure + log_error(e) + @_response.stream.close + end + else + error = e end - else - error = e + ensure + @_response.commit! end - ensure - @_response.commit! end } - @_response.await_commit + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + @_response.await_commit + end + raise error if error end + # Spawn a new thread to serve up the controller in. This is to get + # around the fact that Rack isn't based around IOs and we need to use + # a thread to stream data from the response bodies. Nobody should call + # this method except in Rails internals. Seriously! + def new_controller_thread # :nodoc: + Thread.new { + t2 = Thread.current + t2.abort_on_exception = true + yield + } + end + def log_error(exception) logger = ActionController::Base.logger return unless logger 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 91b3403ad5..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 @@ -378,7 +380,9 @@ module ActionController #:nodoc: end def xor_byte_strings(s1, s2) - s1.bytes.zip(s2.bytes).map { |(c1,c2)| c1 ^ c2 }.pack('c*') + s2_bytes = s2.bytes + s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } + s2_bytes.pack('C*') end # The form's authenticity parameter. Override to provide your own. @@ -403,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 d3382ef296..46589901fd 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 @@ -109,7 +109,7 @@ module ActionController cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false - delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :inspect, + delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :as_json, to: :@parameters # By default, never raise an UnpermittedParameters exception if these @@ -122,16 +122,6 @@ module ActionController cattr_accessor :always_permitted_parameters self.always_permitted_parameters = %w( controller action ) - def self.const_missing(const_name) - return super unless const_name == :NEVER_UNPERMITTED_PARAMS - ActiveSupport::Deprecation.warn(<<-MSG.squish) - `ActionController::Parameters::NEVER_UNPERMITTED_PARAMS` has been deprecated. - Use `ActionController::Parameters.always_permitted_parameters` instead. - MSG - - always_permitted_parameters - end - # Returns a new instance of <tt>ActionController::Parameters</tt>. # Also, sets the +permitted+ attribute to the default value of # <tt>ActionController::Parameters.permit_all_parameters</tt>. @@ -154,17 +144,21 @@ module ActionController end # Returns true if another +Parameters+ object contains the same content and - # permitted flag, or other Hash-like object contains the same content. This - # override is in place so you can perform a comparison with `Hash`. - def ==(other_hash) - if other_hash.respond_to?(:permitted?) - super + # permitted flag. + def ==(other) + if other.respond_to?(:permitted?) + self.permitted? == other.permitted? && self.parameters == other.parameters + elsif other.is_a?(Hash) + ActiveSupport::Deprecation.warn <<-WARNING.squish + Comparing equality between `ActionController::Parameters` and a + `Hash` is deprecated and will be removed in Rails 5.1. Please only do + comparisons between instances of `ActionController::Parameters`. If + you need to compare to a hash, first convert it using + `ActionController::Parameters#new`. + WARNING + @parameters == other.with_indifferent_access else - if other_hash.is_a?(Hash) - @parameters == other_hash.with_indifferent_access - else - @parameters == other_hash - end + @parameters == other end end @@ -190,12 +184,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| @@ -277,7 +278,7 @@ module ActionController # params = ActionController::Parameters.new(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) @@ -436,6 +437,21 @@ module ActionController ) end + if Hash.method_defined?(:dig) + # 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 + # + # params2 = ActionController::Parameters.new(foo: [10, 11, 12]) + # params2.dig(:foo, 1) # => 11 + def dig(*keys) + convert_value_to_parameters(@parameters.dig(*keys)) + end + end + # Returns a new <tt>ActionController::Parameters</tt> instance that # includes only the given +keys+. If the given +keys+ # don't exist, returns an empty hash. @@ -584,6 +600,10 @@ module ActionController dup end + def inspect + "<#{self.class} #{@parameters} permitted: #{@permitted}>" + end + def method_missing(method_sym, *args, &block) if @parameters.respond_to?(method_sym) message = <<-DEPRECATE.squish @@ -603,12 +623,14 @@ module ActionController end protected + attr_reader :parameters + def permitted=(new_permitted) @permitted = new_permitted end def fields_for_style? - @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } + @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && (v.is_a?(Hash) || v.is_a?(Parameters)) } end private @@ -734,6 +756,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 @@ -748,7 +774,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])) @@ -771,7 +797,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 @@ -799,7 +825,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 @@ -819,7 +846,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 b43bb9dc17..b1b3e87934 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -12,6 +12,17 @@ module ActionController include Testing::Functional end + module Live + # Disable controller / rendering threads in tests. User tests can access + # the database on the main thread, so they could open a txn, then the + # controller thread will open a new connection and try to access data + # that's only visible to the main thread's txn. This is the problem in #23483 + remove_method :new_controller_thread + def new_controller_thread # :nodoc: + yield + end + end + # ActionController::TestCase will be deprecated and moved to a gem in Rails 5.1. # Please use ActionDispatch::IntegrationTest going forward. class TestRequest < ActionDispatch::TestRequest #:nodoc: @@ -41,7 +52,7 @@ module ActionController self.session = session self.session_options = TestSession::DEFAULT_OPTIONS @custom_param_parsers = { - Mime[:xml] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } + xml: lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } } end @@ -94,7 +105,7 @@ module ActionController when :url_encoded_form data = non_path_parameters.to_query else - @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters } + @custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters } data = non_path_parameters.to_query end end @@ -417,7 +428,7 @@ module ActionController end alias xhr :xml_http_request - # Simulate a HTTP request to +action+ by specifying request method, + # Simulate an HTTP request to +action+ by specifying request method, # parameters and set/volley the response. # # - +action+: The controller action to call. @@ -516,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 + begin + @controller.recycle! + @controller.dispatch(action, @request, @response) + ensure + @request = @controller.request + @response = @controller.response - @request.delete_header 'HTTP_COOKIE' + @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)) + 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 - end - @response.prepare! + @response.prepare! - if flash_value = @request.flash.to_session_value - @request.session['flash'] = flash_value - else - @request.session.delete('flash') - end + 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' + if xhr + @request.delete_header 'HTTP_X_REQUESTED_WITH' + @request.delete_header 'HTTP_ACCEPT' + end + @request.query_string = '' + + @response.sent! end - @request.query_string = '' @response end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 1e4df07d6e..01d49475de 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -51,8 +51,8 @@ module ActionDispatch autoload :Cookies autoload :DebugExceptions autoload :ExceptionWrapper + autoload :Executor autoload :Flash - autoload :LoadInterlock autoload :ParamsParser autoload :PublicExceptions autoload :Reloader diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 0f7898a3f8..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,19 +78,63 @@ module ActionDispatch set_header DATE, utc_time.httpdate end - def etag=(etag) - key = ActiveSupport::Cache.expand_cache_key(etag) - super %(W/"#{Digest::MD5.hexdigest(key)}") + # 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. + # + # 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/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 9dcab79c3a..041eca48ca 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -4,9 +4,11 @@ module ActionDispatch module Http # Allows you to specify sensitive parameters which will be replaced from # the request log by looking in the query string of the request and all - # sub-hashes of the params hash to filter. If a block is given, each key and - # value of the params hash and all sub-hashes is passed to it, the value - # or key can be replaced using String#replace or similar method. + # sub-hashes of the params hash to filter. Filtering only certain sub-keys + # from a hash is possible by using the dot notation: 'credit_card.number'. + # If a block is given, each key and value of the params hash and all + # sub-hashes is passed to it, the value or key can be replaced using + # String#replace or similar method. # # env["action_dispatch.parameter_filter"] = [:password] # => replaces the value to all keys matching /password/i with "[FILTERED]" @@ -14,6 +16,10 @@ module ActionDispatch # env["action_dispatch.parameter_filter"] = [:foo, "bar"] # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]" # + # env["action_dispatch.parameter_filter"] = [ "credit_card.code" ] + # => replaces { credit_card: {code: "xxxx"} } with "[FILTERED]", does not + # change { file: { code: "xxxx"} } + # # env["action_dispatch.parameter_filter"] = -> (k, v) do # v.reverse! if k =~ /secret/i # end diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 12f81dc1a5..69a934b7cd 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -2,9 +2,23 @@ module ActionDispatch module Http # Provides access to the request's HTTP headers from the environment. # - # env = { "CONTENT_TYPE" => "text/plain" } + # 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" + # + # Also note that when headers are mapped to CGI-like variables by the Rack + # server, both dashes and underscores are converted to underscores. This + # ambiguity cannot be resolved at this stage anymore. Both underscores and + # dashes have to be interpreted as if they were originally sent as dashes. + # + # # GET / HTTP/1.1 + # # ... + # # User-Agent: curl/7.43.0 + # # X_Custom_Header: token + # + # headers["X_Custom_Header"] # => nil + # headers["X-Custom-Header"] # => "token" class Headers CGI_VARIABLES = Set.new(%W[ AUTH_TYPE @@ -101,7 +115,7 @@ module ActionDispatch private - # Converts a HTTP header name to an environment variable name if it is + # Converts an HTTP header name to an environment variable name if it is # not contained within the headers hash. def env_name(key) key = key.to_s 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_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 463b5fe405..4672ea7199 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -1,3 +1,5 @@ +# -*- frozen-string-literal: true -*- + require 'singleton' require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/string/starts_ends_with' @@ -108,57 +110,56 @@ module Mime end end - class AcceptList < Array #:nodoc: - def assort! - sort! + class AcceptList #:nodoc: + def self.sort!(list) + list.sort! - text_xml_idx = find_item_by_name self, 'text/xml' - app_xml_idx = find_item_by_name self, Mime[:xml].to_s + text_xml_idx = find_item_by_name list, 'text/xml' + app_xml_idx = find_item_by_name list, Mime[:xml].to_s # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx - app_xml = self[app_xml_idx] - text_xml = self[text_xml_idx] + app_xml = list[app_xml_idx] + text_xml = list[text_xml_idx] app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list - self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml + list[app_xml_idx], list[text_xml_idx] = text_xml, app_xml app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx end - delete_at(text_xml_idx) # delete text_xml from the list + list.delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - self[text_xml_idx].name = Mime[:xml].to_s + list[text_xml_idx].name = Mime[:xml].to_s end # Look for more specific XML-based types and sort them ahead of app/xml if app_xml_idx - app_xml = self[app_xml_idx] + app_xml = list[app_xml_idx] idx = app_xml_idx - while idx < length - type = self[idx] + while idx < list.length + type = list[idx] break if type.q < app_xml.q if type.name.ends_with? '+xml' - self[app_xml_idx], self[idx] = self[idx], app_xml + list[app_xml_idx], list[idx] = list[idx], app_xml app_xml_idx = idx end idx += 1 end end - map! { |i| Mime::Type.lookup(i.name) }.uniq! - to_a + list.map! { |i| Mime::Type.lookup(i.name) }.uniq! + list end - private - def find_item_by_name(array, name) + def self.find_item_by_name(array, name) array.index { |item| item.name == name } end end class << self - TRAILING_STAR_REGEXP = /(text|application)\/\*/ + TRAILING_STAR_REGEXP = /^(text|application)\/\*/ PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/ def register_callback(&block) @@ -198,21 +199,22 @@ module Mime accept_header = accept_header.split(PARAMETER_SEPARATOR_REGEXP).first parse_trailing_star(accept_header) || [Mime::Type.lookup(accept_header)].compact else - list, index = AcceptList.new, 0 + list, index = [], 0 accept_header.split(',').each do |header| params, q = header.split(PARAMETER_SEPARATOR_REGEXP) - if params.present? - params.strip! - params = parse_trailing_star(params) || [params] + next unless params + params.strip! + next if params.empty? + + params = parse_trailing_star(params) || [params] - params.each do |m| - list << AcceptItem.new(index, m.to_s, q) - index += 1 - end + params.each do |m| + list << AcceptItem.new(index, m.to_s, q) + index += 1 end end - list.assort! + AcceptList.sort! list end end diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 87715205d9..8b04174f1f 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -14,20 +14,22 @@ Mime::Type.register "image/jpeg", :jpeg, [], %w(jpg jpeg jpe pjpeg) Mime::Type.register "image/gif", :gif, [], %w(gif) Mime::Type.register "image/bmp", :bmp, [], %w(bmp) Mime::Type.register "image/tiff", :tiff, [], %w(tif tiff) +Mime::Type.register "image/svg+xml", :svg 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 # http://www.ietf.org/rfc/rfc4627.txt # http://www.json.org/JSONRequest.html -Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest application/vnd.api+json ) +Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) Mime::Type.register "application/pdf", :pdf, [], %w(pdf) Mime::Type.register "application/zip", :zip, [], %w(zip) +Mime::Type.register "application/gzip", :gzip, %w(application/x-gzip), %w(gz) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index cca7376ffa..ff5031d7d5 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -1,22 +1,31 @@ module ActionDispatch module Http module Parameters + extend ActiveSupport::Concern + PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { - Mime[:json] => lambda { |raw_post| + Mime[:json].symbol => -> (raw_post) { data = ActiveSupport::JSON.decode(raw_post) data.is_a?(Hash) ? data : {:_json => data} } } - def self.included(klass) - class << klass - attr_accessor :parameter_parsers + included do + class << self + attr_reader :parameter_parsers end - klass.parameter_parsers = DEFAULT_PARSERS + self.parameter_parsers = DEFAULT_PARSERS end + + module ClassMethods + def parameter_parsers=(parsers) # :nodoc: + @parameter_parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } + end + end + # Returns both GET and POST \parameters in a single hash. def parameters params = get_header("action_dispatch.request.parameters") @@ -51,7 +60,7 @@ module ActionDispatch def parse_formatted_parameters(parsers) return yield if content_length.zero? - strategy = parsers.fetch(content_mime_type) { return yield } + strategy = parsers.fetch(content_mime_type.symbol) { return yield } begin strategy.call(raw_post) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 5427425ef7..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: @@ -403,6 +402,10 @@ module ActionDispatch def commit_flash end + def ssl? + super || scheme == 'wss'.freeze + end + private def check_method(name) HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS[0...-1].join(', ')}, and #{HTTP_METHODS[-1]}") diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 14f86c7c07..1515d59df3 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/module/attribute_accessors' require 'action_dispatch/http/filter_redirect' +require 'action_dispatch/http/cache' require 'monitor' module ActionDispatch # :nodoc: @@ -67,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 @@ -96,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) @@ -111,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 @@ -128,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/backwards.rb b/actionpack/lib/action_dispatch/journey/backwards.rb deleted file mode 100644 index 3bd20fdf81..0000000000 --- a/actionpack/lib/action_dispatch/journey/backwards.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Rack # :nodoc: - Mount = ActionDispatch::Journey::Router - Mount::RouteSet = ActionDispatch::Journey::Router - Mount::RegexpWithNamedGroups = ActionDispatch::Journey::Path::Pattern -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/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 35c2b1b86e..cfd6681dd1 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -3,7 +3,7 @@ module ActionDispatch class Route # :nodoc: attr_reader :app, :path, :defaults, :name, :precedence - attr_reader :constraints + attr_reader :constraints, :internal alias :conditions :constraints module VerbMatchers @@ -55,7 +55,7 @@ module ActionDispatch ## # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. - def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence, internal = false) @name = name @app = app @path = path @@ -70,6 +70,7 @@ module ActionDispatch @decorated_ast = nil @precedence = precedence @path_formatter = @path.build_formatter + @internal = internal end def ast @@ -81,7 +82,7 @@ module ActionDispatch end def requirements # :nodoc: - # needed for rails `rake routes` + # needed for rails `rails routes` @defaults.merge(path.requirements).delete_if { |_,v| /.+?/ == v } diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index f649588520..06cdce1724 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -16,9 +16,6 @@ module ActionDispatch class RoutingError < ::StandardError # :nodoc: end - # :nodoc: - VERSION = '2.0.0' - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb index f80df78582..c782779b34 100644 --- a/actionpack/lib/action_dispatch/middleware/callbacks.rb +++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb @@ -7,7 +7,16 @@ module ActionDispatch define_callbacks :call class << self - delegate :to_prepare, :to_cleanup, :to => "ActionDispatch::Reloader" + def to_prepare(*args, &block) + ActiveSupport::Reloader.to_prepare(*args, &block) + end + + def to_cleanup(*args, &block) + ActiveSupport::Reloader.to_complete(*args, &block) + end + + deprecate to_prepare: 'use ActiveSupport::Reloader.to_prepare instead', + to_cleanup: 'use ActiveSupport::Reloader.to_complete instead' def before(*args, &block) set_callback(:call, :before, *args, &block) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 3477aa8b29..f2f3150b56 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/key_generator' require 'active_support/message_verifier' require 'active_support/json' +require 'rack/utils' module ActionDispatch class Request @@ -337,7 +338,7 @@ module ActionDispatch end def to_header - @cookies.map { |k,v| "#{k}=#{v}" }.join ';' + @cookies.map { |k,v| "#{escape(k)}=#{escape(v)}" }.join '; ' end def handle_options(options) #:nodoc: @@ -419,6 +420,10 @@ module ActionDispatch private + def escape(string) + ::Rack::Utils.escape(string) + end + def make_set_cookie_header(header) header = @set_cookies.inject(header) { |m, (k, v)| if write_cookie?(v) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index b55c937e0c..5f758d641a 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -67,18 +67,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 +93,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 +104,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) @@ -156,15 +156,20 @@ module ActionDispatch trace = wrapper.framework_trace if trace.empty? ActiveSupport::Deprecation.silence do - message = "\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << trace.join("\n ") - logger.fatal("#{message}\n\n") + logger.fatal " " + logger.fatal "#{exception.class} (#{exception.message}):" + log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code) + logger.fatal " " + log_array logger, trace end end + def log_array(logger, array) + array.map { |line| logger.fatal line } + end + def logger(request) - request.logger || stderr_logger + request.logger || ActionView::Base.logger || stderr_logger end def stderr_logger @@ -176,5 +181,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/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 3b61824cc9..59edc66086 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -1,4 +1,3 @@ -require 'action_controller/metal/exceptions' require 'active_support/core_ext/module/attribute_accessors' require 'rack/utils' diff --git a/actionpack/lib/action_dispatch/middleware/executor.rb b/actionpack/lib/action_dispatch/middleware/executor.rb new file mode 100644 index 0000000000..06245b403b --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/executor.rb @@ -0,0 +1,19 @@ +require 'rack/body_proxy' + +module ActionDispatch + class Executor + def initialize(app, executor) + @app, @executor = app, executor + end + + def call(env) + state = @executor.run! + begin + response = @app.call(env) + returned = response << ::Rack::BodyProxy.new(response.pop) { state.complete! } + ensure + state.complete! unless returned + end + 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/load_interlock.rb b/actionpack/lib/action_dispatch/middleware/load_interlock.rb deleted file mode 100644 index 07f498319c..0000000000 --- a/actionpack/lib/action_dispatch/middleware/load_interlock.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'active_support/dependencies' -require 'rack/body_proxy' - -module ActionDispatch - class LoadInterlock - def initialize(app) - @app = app - end - - def call(env) - interlock = ActiveSupport::Dependencies.interlock - interlock.start_running - response = @app.call(env) - body = Rack::BodyProxy.new(response[2]) { interlock.done_running } - response[2] = body - response - ensure - interlock.done_running unless body - end - end -end diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index c2a4f46e67..faf3262b8f 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,8 @@ 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 end diff --git a/actionpack/lib/action_dispatch/middleware/reloader.rb b/actionpack/lib/action_dispatch/middleware/reloader.rb index af9a29eb07..112bde6596 100644 --- a/actionpack/lib/action_dispatch/middleware/reloader.rb +++ b/actionpack/lib/action_dispatch/middleware/reloader.rb @@ -23,74 +23,32 @@ module ActionDispatch # middleware stack, but are executed only when <tt>ActionDispatch::Reloader.prepare!</tt> # or <tt>ActionDispatch::Reloader.cleanup!</tt> are called manually. # - class Reloader - include ActiveSupport::Callbacks - include ActiveSupport::Deprecation::Reporting - - define_callbacks :prepare - define_callbacks :cleanup - - # Add a prepare callback. Prepare callbacks are run before each request, prior - # to ActionDispatch::Callback's before callbacks. + class Reloader < Executor def self.to_prepare(*args, &block) - unless block_given? - warn "to_prepare without a block is deprecated. Please use a block" - end - set_callback(:prepare, *args, &block) + ActiveSupport::Reloader.to_prepare(*args, &block) end - # Add a cleanup callback. Cleanup callbacks are run after each request is - # complete (after #close is called on the response body). def self.to_cleanup(*args, &block) - unless block_given? - warn "to_cleanup without a block is deprecated. Please use a block" - end - set_callback(:cleanup, *args, &block) + ActiveSupport::Reloader.to_complete(*args, &block) end - # Execute all prepare callbacks. def self.prepare! - new(nil).prepare! + default_reloader.prepare! end - # Execute all cleanup callbacks. def self.cleanup! - new(nil).cleanup! - end - - def initialize(app, condition=nil) - @app = app - @condition = condition || lambda { true } - @validated = true + default_reloader.reload! end - def call(env) - @validated = @condition.call - prepare! - - response = @app.call(env) - response[2] = ::Rack::BodyProxy.new(response[2]) { cleanup! } + class << self + attr_accessor :default_reloader # :nodoc: - response - rescue Exception - cleanup! - raise + deprecate to_prepare: 'use ActiveSupport::Reloader.to_prepare instead', + to_cleanup: 'use ActiveSupport::Reloader.to_complete instead', + prepare!: 'use Rails.application.reloader.prepare! instead', + cleanup!: 'use Rails.application.reloader.reload! instead of cleanup + prepare' end - def prepare! #:nodoc: - run_callbacks :prepare if validated? - end - - def cleanup! #:nodoc: - run_callbacks :cleanup if validated? - ensure - @validated = true - end - - private - - def validated? #:nodoc: - @validated - end + self.default_reloader = ActiveSupport::Reloader end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 429a98f236..dec9c60ef2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -23,7 +23,7 @@ module ActionDispatch # goes a step further than signed cookies in that encrypted cookies cannot # be altered or read by users. This is the default starting in Rails 4. # - # If you have both secret_token and secret_key base set, your cookies will + # If you have both secret_token and secret_key_base set, your cookies will # be encrypted, and signed cookies generated by Rails 3 will be # transparently read and encrypted to provide a smooth upgrade path. # diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 735b5939dd..ab3077b308 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -23,7 +23,7 @@ module ActionDispatch # preload lists is `18.weeks`. # * `subdomains`: Set to `true` to tell the browser to apply these settings # to all subdomains. This protects your cookies from interception by a - # vulnerable site on a subdomain. Defaults to `false`. + # vulnerable site on a subdomain. Defaults to `true`. # * `preload`: Advertise that this site may be included in browsers' # preloaded HSTS lists. HSTS protects your site on every visit *except the # first visit* since it hasn't seen your HSTS header yet. To close this @@ -34,6 +34,10 @@ module ActionDispatch # original HSTS directive until it expires. Instead, use the header to tell browsers to # expire HSTS immediately. Setting `hsts: false` is a shortcut for # `hsts: { expires: 0 }`. + # + # Requests can opt-out of redirection with `exclude`: + # + # config.ssl_options = { redirect: { exclude: -> request { request.path =~ /healthcheck/ } } } class SSL # Default to 180 days, the low end for https://www.ssllabs.com/ssltest/ # and greater than the 18-week requirement for browser preload lists. @@ -49,14 +53,26 @@ module ActionDispatch if options[:host] || options[:port] ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc The `:host` and `:port` options are moving within `:redirect`: - `config.ssl_options = { redirect: { host: …, port: … }}`. + `config.ssl_options = { redirect: { host: …, port: … } }`. end_warning @redirect = options.slice(:host, :port) else @redirect = redirect end + @exclude = @redirect && @redirect[:exclude] || proc { !@redirect } @secure_cookies = secure_cookies + + if hsts != true && hsts != false && hsts[:subdomains].nil? + hsts[:subdomains] = false + + ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc + In Rails 5.1, The `:subdomains` option of HSTS config will be treated as true if + unspecified. Set `config.ssl_options = { hsts: { subdomains: false } }` to opt out + of this behavior. + end_warning + end + @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) end @@ -69,7 +85,7 @@ module ActionDispatch flag_cookies_as_secure! headers if @secure_cookies end else - return redirect_to_https request if @redirect + return redirect_to_https request unless @exclude.call(request) @app.call(env) end end 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/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index ddeea24bb3..e9e6a2e597 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -39,6 +39,8 @@ module ActionDispatch config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil? ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie + ActionDispatch::Reloader.default_reloader = app.reloader + ActionDispatch.test_app = app end end 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 d00b2c3eb5..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 # @@ -159,7 +159,7 @@ module ActionDispatch # # controller 'geocode' do # get 'geocode/:postalcode' => :show, constraints: { - # postalcode: /# Postcode format + # postalcode: /# Postalcode format # \d{5} #Prefix # (-\d{4})? #Suffix # /x @@ -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 @@ -239,7 +239,7 @@ module ActionDispatch # # rails routes # - # Target specific controllers by prefixing the command with <tt>CONTROLLER=x</tt>. + # Target specific controllers by prefixing the command with <tt>-c</tt> option. # module Routing extend ActiveSupport::Autoload @@ -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 69e6dd5215..2459a45827 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -33,15 +33,15 @@ 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? - controller.to_s =~ %r{\Arails/(info|mailers|welcome)} + internal end def engine? @@ -51,7 +51,7 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes` or looks at the RoutingError page. + # executes `rails routes` or looks at the RoutingError page. # People should not use this class. class RoutesInspector # :nodoc: def initialize(routes) @@ -60,12 +60,10 @@ module ActionDispatch end def format(formatter, filter = nil) - routes_to_display = filter_routes(filter) - + routes_to_display = filter_routes(normalize_filter(filter)) routes = collect_routes(routes_to_display) - if routes.none? - formatter.no_routes(collect_routes(@routes), filter) + formatter.no_routes(collect_routes(@routes)) return formatter.result end @@ -82,10 +80,20 @@ module ActionDispatch private + def normalize_filter(filter) + if filter.is_a?(Hash) && filter[:controller] + { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } + elsif filter + { controller: /#{filter}/, action: /#{filter}/, verb: /#{filter}/, name: /#{filter}/, path: /#{filter}/ } + end + end + def filter_routes(filter) if filter - filter_name = filter.underscore.sub(/_controller$/, '') - @routes.select { |route| route.defaults[:controller] == filter_name } + @routes.select do |route| + route_wrapper = RouteWrapper.new(route) + filter.any? { |default, value| route_wrapper.send(default) =~ value } + end else @routes end @@ -137,7 +145,7 @@ module ActionDispatch @buffer << draw_header(routes) end - def no_routes(routes, filter) + def no_routes(routes) @buffer << if routes.none? <<-MESSAGE.strip_heredoc @@ -145,8 +153,6 @@ module ActionDispatch Please add some routes in config/routes.rb. MESSAGE - elsif missing_controller?(filter) - "The controller #{filter} does not exist!" else "No routes were found for this controller" end @@ -154,10 +160,6 @@ module ActionDispatch end private - def missing_controller?(controller_name) - [ controller_name.camelize, "#{controller_name.camelize}Controller" ].none?(&:safe_constantize) - end - def draw_section(routes) header_lengths = ['Prefix', 'Verb', 'URI Pattern'].map(&:length) name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index afbaa45d20..40b6500553 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,6 +106,7 @@ module ActionDispatch @ast = ast @anchor = anchor @via = via + @internal = options.delete(:internal) path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -120,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 @@ -137,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 @@ -148,7 +152,8 @@ module ActionDispatch required_defaults, defaults, request_method, - precedence) + precedence, + @internal) route end @@ -819,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 @@ -1596,7 +1601,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} @@ -2083,8 +2088,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/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 846b5fa1fc..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 an URL from non sanitized request parameters is insecure!" + raise ArgumentError, ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE end end helper.call self, args, options @@ -513,6 +513,21 @@ module ActionDispatch route = @set.add_route(name, mapping) named_routes[name] = route if name + + 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. + 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. + MSG + end + route end @@ -533,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 @@ -557,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 @@ -615,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 f91679593e..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 an 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/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 78ef860548..44ad2c10d8 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -86,6 +86,7 @@ module ActionDispatch end # Load routes.rb if it hasn't been loaded. + options = options.clone generated_path, query_string_keys = @routes.generate_extras(options, defaults) found_extras = options.reject { |k, _| ! query_string_keys.include? k } @@ -116,7 +117,7 @@ module ActionDispatch # # Tests a route, providing a defaults hash # assert_routing 'controller/action/9', {id: "9", item: "square"}, {controller: "controller", action: "action"}, {}, {item: "square"} # - # # Tests a route with a HTTP method + # # Tests a route with an HTTP method # assert_routing({ method: 'put', path: '/product/321' }, { controller: "product", action: "update", id: "321" }) def assert_routing(path, options, defaults={}, extras={}, message=nil) assert_recognizes(options, path, extras, message) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 711ca10419..8777666f9f 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -71,7 +71,7 @@ module ActionDispatch end # Performs an XMLHttpRequest request with the given parameters, mirroring - # a request from the Prototype library. + # an AJAX request made from JavaScript. # # The request_method is +:get+, +:post+, +:patch+, +:put+, +:delete+ or # +:head+; the parameters are +nil+, a hash, or a url-encoded or multipart @@ -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,12 +317,15 @@ module ActionDispatch params: { id: 1 }, headers: { 'X-Extra-Header' => '123' }, env: { 'action_dispatch.custom' => 'custom' }, - xhr: true + xhr: true, + as: :json MSG end # Performs the actual request. - def process(method, path, params: nil, headers: nil, env: nil, xhr: false) + def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil) + request_encoder = RequestEncoder.encoder(as) + if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -330,14 +334,17 @@ module ActionDispatch url_host += ":#{location.port}" if default != location.port host! url_host end - path = location.query ? "#{location.path}?#{location.query}" : location.path + path = request_encoder.append_format_to location.path + path = location.query ? "#{path}?#{location.query}" : path + else + path = request_encoder.append_format_to path end hostname, port = host.split(':') request_env = { :method => method, - :params => params, + :params => request_encoder.encode_params(params), "SERVER_NAME" => hostname, "SERVER_PORT" => port || (https? ? "443" : "80"), @@ -347,7 +354,7 @@ module ActionDispatch "REQUEST_URI" => path, "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, - "CONTENT_TYPE" => "application/x-www-form-urlencoded", + "CONTENT_TYPE" => request_encoder.content_type, "HTTP_ACCEPT" => accept } @@ -376,6 +383,7 @@ module ActionDispatch response = _mock_session.last_response @response = ActionDispatch::TestResponse.from_response(response) @response.request = @request + @response.response_parser = RequestEncoder.parser(@response.content_type) @html_document = nil @url_options = nil @@ -387,6 +395,56 @@ module ActionDispatch def build_full_uri(path, env) "#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}" end + + class RequestEncoder # :nodoc: + @encoders = {} + + attr_reader :response_parser + + def initialize(mime_name, param_encoder, response_parser, url_encoded_form = false) + @mime = Mime[mime_name] + + unless @mime + raise ArgumentError, "Can't register a request encoder for " \ + "unregistered MIME Type: #{mime_name}. See `Mime::Type.register`." + end + + @url_encoded_form = url_encoded_form + @path_format = ".#{@mime.symbol}" unless @url_encoded_form + @response_parser = response_parser || -> body { body } + @param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc + end + + def append_format_to(path) + path << @path_format unless @url_encoded_form + path + end + + def content_type + @mime.to_s + end + + def encode_params(params) + @param_encoder.call(params) + end + + def self.parser(content_type) + mime = Mime::Type.lookup(content_type) + encoder(mime ? mime.ref : nil).response_parser + end + + def self.encoder(name) + @encoders[name] || WWWFormEncoder + end + + def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil) + @encoders[mime_name] = new(mime_name, param_encoder, response_parser) + end + + register_encoder :json, response_parser: -> body { JSON.parse(body) } + + WWWFormEncoder = new(:url_encoded_form, -> params { params }, nil, true) + end end module Runner @@ -643,33 +701,85 @@ module ActionDispatch # end # end # + # You can also test your JSON API easily by setting what the request should + # be encoded as: + # + # 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_response :success + # assert_equal({ id: Arcticle.last.id, title: 'Ahoy!' }, response.parsed_body) + # end + # end + # + # The `as` option sets the format to JSON, sets the content type to + # 'application/json' and encodes the parameters as JSON. + # + # Calling `parsed_body` on the response parses the response body as what + # the last request was encoded as. If the request wasn't encoded `as` something, + # it's the same as calling `body`. + # + # For any custom MIME Types you've registered, you can even add your own encoders with: + # + # ActionDispatch::IntegrationTest.register_encoder :wibble, + # param_encoder: -> params { params.to_wibble }, + # response_parser: -> body { body } + # + # Where `param_encoder` defines how the params should be encoded and + # `response_parser` defines how the response body should be parsed through + # `parsed_body`. + # # Consult the Rails Testing Guide for more. class IntegrationTest < ActiveSupport::TestCase - include Integration::Runner - include ActionController::TemplateAssertions - include ActionDispatch::Routing::UrlFor + module UrlOptions + extend ActiveSupport::Concern + def url_options + integration_session.url_options + end + end - @@app = nil + module Behavior + extend ActiveSupport::Concern - def self.app - @@app || ActionDispatch.test_app - end + include Integration::Runner + include ActionController::TemplateAssertions - def self.app=(app) - @@app = app - end + included do + include ActionDispatch::Routing::UrlFor + include UrlOptions # don't let UrlFor override the url_options method + ActiveSupport.run_load_hooks(:action_dispatch_integration_test, self) + @@app = nil + end - def app - super || self.class.app - end + module ClassMethods + def app + defined?(@@app) ? @@app : ActionDispatch.test_app + end - def url_options - integration_session.url_options - end + def app=(app) + @@app = app + end - def document_root_element - html_document.root + def register_encoder(*args) + Integration::Session::RequestEncoder.register_encoder(*args) + end + end + + def app + super || self.class.app + end + + def document_root_element + html_document.root + end end + + include Behavior end end diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index eca0439909..1ecd7d14a7 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -1,6 +1,5 @@ require 'action_dispatch/middleware/cookies' require 'action_dispatch/middleware/flash' -require 'active_support/core_ext/hash/indifferent_access' module ActionDispatch module TestProcess 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_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 4b79a90242..9d4b73a43d 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -18,5 +18,11 @@ module ActionDispatch # Was there a server-side error? alias_method :error?, :server_error? + + attr_writer :response_parser # :nodoc: + + def parsed_body + @parsed_body ||= @response_parser.call(body) + end end end diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 3a1410d492..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 = "beta1.1" + 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 1ef10ff956..1e1d6f5429 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -1,5 +1,3 @@ -require File.expand_path('../../../load_paths', __FILE__) - $:.unshift(File.dirname(__FILE__) + '/lib') $:.unshift(File.dirname(__FILE__) + '/fixtures/helpers') $:.unshift(File.dirname(__FILE__) + '/fixtures/alternate_helpers') @@ -36,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 @@ -60,16 +57,14 @@ 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 SharedTestRoutes.draw do - get ':controller(/:action)' + ActiveSupport::Deprecation.silence do + get ':controller(/:action)' + end end module ActionDispatch @@ -118,7 +113,9 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase self.app = build_app app.routes.draw do - get ':controller(/:action)' + ActiveSupport::Deprecation.silence do + get ':controller(/:action)' + end end class DeadEndRoutes < ActionDispatch::Routing::RouteSet @@ -254,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 @@ -375,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/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 899d92f815..db71aa2160 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -177,7 +177,10 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase set.draw do get 'route_one', :to => 'action_pack_assertions#nothing', :as => :route_one get 'route_two', :to => 'action_pack_assertions#nothing', :id => 'two', :as => :route_two - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end process :redirect_to_named_route assert_raise(ActiveSupport::TestCase::Assertion) do @@ -201,7 +204,10 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase with_routing do |set| set.draw do get 'admin/inner_module', :to => 'admin/inner_module#index', :as => :admin_inner_module - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end process :redirect_to_index # redirection is <{"action"=>"index", "controller"=>"admin/admin/inner_module"}> @@ -215,7 +221,10 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase with_routing do |set| set.draw do get '/action_pack_assertions/:id', :to => 'action_pack_assertions#index', :as => :top_level - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end process :redirect_to_top_level_named_route # assert_redirected_to "http://test.host/action_pack_assertions/foo" would pass because of exact match early return @@ -231,7 +240,10 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase set.draw do # this controller exists in the admin namespace as well which is the only difference from previous test get '/user/:id', :to => 'user#index', :as => :top_level - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end process :redirect_to_top_level_named_route # assert_redirected_to top_level_url('foo') would pass because of exact match early return 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/base_test.rb b/actionpack/test/controller/base_test.rb index e3f669dbb5..577a3d5800 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -175,7 +175,10 @@ class UrlOptionsTest < ActionController::TestCase with_routing do |set| set.draw do get 'from_view', :to => 'url_options#from_view', :as => :from_view - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :from_view, params: { route: "from_view_url" } @@ -209,7 +212,10 @@ class DefaultUrlOptionsTest < ActionController::TestCase with_routing do |set| set.draw do get 'from_view', :to => 'default_url_options#from_view', :as => :from_view - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :from_view, params: { route: "from_view_url" } @@ -226,7 +232,10 @@ class DefaultUrlOptionsTest < ActionController::TestCase scope("/:locale") do resources :descriptions end - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :from_view, params: { route: "description_path(1)" } diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index d19b3810c2..7faf3cd8c6 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -172,6 +172,9 @@ class FunctionalCachingController < CachingController def fragment_cached_without_digest end + + def fragment_cached_with_options + end end class FunctionalFragmentCachingTest < ActionController::TestCase @@ -215,6 +218,18 @@ CACHED assert_equal "<p>ERB</p>", @store.read("views/nodigest") 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 + Time.stub(:now, time + 11) do + assert_nil @store.read("views/with_options") + end + end + def test_render_inline_before_fragment_caching get :inline_fragment_cached assert_response :success @@ -369,14 +384,14 @@ class CollectionCacheController < ActionController::Base render 'index' end - def index_explicit_render + def index_explicit_render_in_controller @customers = [Customer.new('david', 1)] - render partial: 'customers/customer', collection: @customers + render partial: 'customers/customer', collection: @customers, cached: true end def index_with_comment @customers = [Customer.new('david', 1)] - render partial: 'customers/commented_customer', collection: @customers, as: :customer + render partial: 'customers/commented_customer', collection: @customers, as: :customer, cached: true end end @@ -387,12 +402,13 @@ class AutomaticCollectionCacheTest < ActionController::TestCase @controller.perform_caching = true @controller.partial_rendered_times = 0 @controller.cache_store = ActiveSupport::Cache::MemoryStore.new - ActionView::PartialRenderer.collection_cache = @controller.cache_store + ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new end def test_collection_fetches_cached_views get :index assert_equal 1, @controller.partial_rendered_times + assert_customer_cached 'david/1', 'david, 1' get :index assert_equal 1, @controller.partial_rendered_times @@ -400,13 +416,16 @@ class AutomaticCollectionCacheTest < ActionController::TestCase def test_preserves_order_when_reading_from_cache_plus_rendering get :index, params: { id: 2 } - get :index_ordered + assert_equal 1, @controller.partial_rendered_times + assert_select ':root', 'david, 2' + get :index_ordered + assert_equal 3, @controller.partial_rendered_times assert_select ':root', "david, 1\n david, 2\n david, 3" end def test_explicit_render_call_with_options - get :index_explicit_render + get :index_explicit_render_in_controller assert_select ':root', "david, 1" end @@ -418,6 +437,12 @@ class AutomaticCollectionCacheTest < ActionController::TestCase get :index_with_comment assert_equal 1, @controller.partial_rendered_times end + + private + def assert_customer_cached(key, content) + assert_match content, + ActionView::PartialRenderer.collection_cache.read("views/#{key}/7c228ab609f0baf0b1f2367469210937") + end end class FragmentCacheKeyTestController < CachingController 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/flash_test.rb b/actionpack/test/controller/flash_test.rb index b063d769a4..eef48e8480 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -323,7 +323,9 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest def with_test_route_set with_routing do |set| set.draw do - get ':action', :to => FlashIntegrationTest::TestController + ActiveSupport::Deprecation.silence do + get ':action', :to => FlashIntegrationTest::TestController + end end @app = self.class.build_app(set) do |middleware| diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 22f1cc7c22..03a9c9ae78 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -322,3 +322,12 @@ class RedirectToSSLTest < ActionController::TestCase assert_equal 'ihaz', response.body end end + +class ForceSSLControllerLevelTest < ActionController::TestCase + def test_no_redirect_websocket_ssl_request + request.env['rack.url_scheme'] = 'wss' + request.env['Upgrade'] = 'websocket' + get :cheeseburger + assert_response 200 + end +end 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 d0a1d1285f..34fb3b1003 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 @@ -390,7 +392,7 @@ class IntegrationTestUsesCorrectClass < ActionDispatch::IntegrationTest reset! %w( get post head patch put delete ).each do |verb| - assert_nothing_raised("'#{verb}' should use integration test methods") { __send__(verb, '/') } + assert_nothing_raised { __send__(verb, '/') } end end end @@ -730,8 +732,10 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest set.draw do get 'moved' => redirect('/method') - match ':action', :to => controller, :via => [:get, :post], :as => :action - get 'get/:action', :to => controller, :as => :get_action + ActiveSupport::Deprecation.silence do + match ':action', :to => controller, :via => [:get, :post], :as => :action + get 'get/:action', :to => controller, :as => :get_action + end end self.singleton_class.include(set.url_helpers) @@ -1105,7 +1109,12 @@ class IntegrationRequestsWithoutSetup < ActionDispatch::IntegrationTest def test_request with_routing do |routes| - routes.draw { get ':action' => FooController } + routes.draw do + ActiveSupport::Deprecation.silence do + get ':action' => FooController + end + end + get '/ok' assert_response 200 @@ -1126,3 +1135,93 @@ class IntegrationRequestsWithSessionSetup < ActionDispatch::IntegrationTest assert_equal({"user_name"=>"david"}, cookies.to_hash) end end + +class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest + class FooController < ActionController::Base + def foos_json + render json: params.permit(:foo) + end + + def foos_wibble + render plain: 'ok' + 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 + assert_match 'foos_json.json', request.path + assert_equal 'application/json', request.content_type + assert_equal({ 'foo' => 'fighters' }, request.request_parameters) + assert_equal({ 'foo' => 'fighters' }, response.parsed_body) + end + end + + def test_encoding_as_without_mime_registration + assert_raise ArgumentError do + ActionDispatch::IntegrationTest.register_encoder :wibble + end + end + + def test_registering_custom_encoder + Mime::Type.register 'text/wibble', :wibble + + ActionDispatch::IntegrationTest.register_encoder(:wibble, + param_encoder: -> params { params }) + + post_to_foos as: :wibble do + assert_response :success + assert_match 'foos_wibble.wibble', request.path + assert_equal 'text/wibble', request.content_type + assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed. + assert_equal 'ok', response.parsed_body + end + ensure + Mime::Type.unregister :wibble + end + + def test_parsed_body_without_as_option + with_routing do |routes| + routes.draw do + ActiveSupport::Deprecation.silence do + get ':action' => FooController + end + end + + get '/foos_json.json', params: { foo: 'heyo' } + + assert_equal({ 'foo' => 'heyo' }, response.parsed_body) + end + end + + private + def post_to_foos(as:) + with_routing do |routes| + routes.draw do + ActiveSupport::Deprecation.silence do + post ':action' => FooController + end + end + + post "/foos_#{as}", params: { foo: 'fighters' }, as: as + + yield + end + end +end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 2ef9734269..5977124594 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -152,7 +152,6 @@ module ActionController def thread_locals tc.assert_equal 'aaron', Thread.current[:setting] - tc.assert_not_equal Thread.current.object_id, Thread.current[:originating_thread] response.headers['Content-Type'] = 'text/event-stream' %w{ hello world }.each do |word| @@ -206,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 @@ -247,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 @@ -261,6 +261,14 @@ module ActionController end end + def setup + super + + def @controller.new_controller_thread + Thread.new { yield } + end + end + def test_set_cookie get :set_cookie assert_equal({'hello' => 'world'}, @response.cookies) diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 6ae33be3c8..57cf2dafdf 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -183,6 +183,12 @@ class ACLogSubscriberTest < ActionController::TestCase assert_equal "test_value", @controller.last_payload[:test_key] end + def test_process_action_headers + get :show + wait + assert_equal "Rails Testing", @controller.last_payload[:headers]['User-Agent'] + end + def test_process_action_with_filter_parameters @request.env["action_dispatch.parameter_filter"] = [:lifo, :amount] 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 76e2d3ff43..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 @@ -160,7 +168,14 @@ class RespondToController < ActionController::Base end end - def variant_with_implicit_rendering + def variant_with_implicit_template_rendering + # This has exactly one variant template defined in the file system (+mobile.html.erb), + # which raises the regular MissingTemplate error for other variants. + end + + def variant_without_implicit_template_rendering + # This differs from the above in that it does not have any templates defined in the file + # system, which triggers the ImplicitRender (204 No Content) behavior. end def variant_with_format_and_custom_render @@ -272,6 +287,8 @@ class RespondToController < ActionController::Base end class RespondToControllerTest < ActionController::TestCase + NO_CONTENT_WARNING = "No template found for RespondToController#variant_without_implicit_template_rendering, rendering head :no_content" + def setup super @request.host = "www.example.com" @@ -615,31 +632,77 @@ 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 } + end + end + + def test_variant_not_set_regular_unknown_format + assert_raises(ActionController::UnknownFormat) do + get :variant_with_implicit_template_rendering + end + end + + def test_variant_with_implicit_template_rendering + get :variant_with_implicit_template_rendering, params: { v: :mobile } + assert_equal "text/html", @response.content_type + assert_equal "mobile", @response.body + end + + def test_variant_without_implicit_rendering_from_browser + assert_raises(ActionController::UnknownFormat) do + get :variant_without_implicit_template_rendering, params: { v: :does_not_matter } + end + end + + def test_variant_variant_not_set_and_without_implicit_rendering_from_browser + assert_raises(ActionController::UnknownFormat) do + get :variant_without_implicit_template_rendering + end + end + + def test_variant_without_implicit_rendering_from_xhr logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new old_logger, ActionController::Base.logger = ActionController::Base.logger, logger - get :variant_with_implicit_rendering, params: { v: :invalid } + get :variant_without_implicit_template_rendering, xhr: true, params: { v: :does_not_matter } assert_response :no_content - assert_equal 1, logger.logged(:info).select{ |s| s =~ /No template found/ }.size, "Implicit head :no_content not logged" + + assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" ensure ActionController::Base.logger = old_logger end - def test_variant_not_set_regular_template_missing - get :variant_with_implicit_rendering + def test_variant_without_implicit_rendering_from_api + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + + get :variant_without_implicit_template_rendering, format: 'json', params: { v: :does_not_matter } assert_response :no_content + + assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end - def test_variant_with_implicit_rendering - get :variant_with_implicit_rendering, params: { v: :implicit } + def test_variant_variant_not_set_and_without_implicit_rendering_from_xhr + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + + get :variant_without_implicit_template_rendering, xhr: true assert_response :no_content - end - def test_variant_with_implicit_template_rendering - get :variant_with_implicit_rendering, params: { v: :mobile } - assert_equal "text/html", @response.content_type - assert_equal "mobile", @response.body + assert_equal 1, logger.logged(:info).select { |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end def test_variant_with_format_and_custom_render @@ -778,24 +841,3 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "phone", @response.body end end - -class RespondToWithBlockOnDefaultRenderController < ActionController::Base - def show - default_render do - render body: 'default_render yielded' - end - end -end - -class RespondToWithBlockOnDefaultRenderControllerTest < ActionController::TestCase - def setup - super - @request.host = "www.example.com" - end - - def test_default_render_uses_block_when_no_template_exists - get :show - assert_equal "default_render yielded", @response.body - assert_equal "text/plain", @response.content_type - end -end diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index c226fa57ee..ee3c498b1c 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -40,6 +40,22 @@ module BareMetalTest end end + class BareEmptyController < ActionController::Metal + def index + self.response_body = nil + end + end + + class BareEmptyTest < ActiveSupport::TestCase + test "response body is nil" do + controller = BareEmptyController.new + controller.set_request!(ActionDispatch::Request.empty) + controller.set_response!(BareController.make_response!(controller.request)) + controller.index + assert_equal nil, controller.response_body + end + end + class HeadController < ActionController::Metal include ActionController::Head diff --git a/actionpack/test/controller/new_base/content_type_test.rb b/actionpack/test/controller/new_base/content_type_test.rb index a9dcdde4b8..0b3a26807d 100644 --- a/actionpack/test/controller/new_base/content_type_test.rb +++ b/actionpack/test/controller/new_base/content_type_test.rb @@ -43,7 +43,9 @@ module ContentType test "default response is text/plain and UTF8" do with_routing do |set| set.draw do - get ':controller', :action => 'index' + ActiveSupport::Deprecation.silence do + get ':controller', :action => 'index' + end end get "/content_type/base" diff --git a/actionpack/test/controller/new_base/render_body_test.rb b/actionpack/test/controller/new_base/render_body_test.rb index f4a3db8b41..c65c245773 100644 --- a/actionpack/test/controller/new_base/render_body_test.rb +++ b/actionpack/test/controller/new_base/render_body_test.rb @@ -85,7 +85,7 @@ module RenderBody test "rendering body from an action with default options renders the body with the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } get "/render_body/simple" assert_body "hello david" @@ -95,7 +95,7 @@ module RenderBody test "rendering body from an action with default options renders the body without the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } get "/render_body/with_layout" diff --git a/actionpack/test/controller/new_base/render_html_test.rb b/actionpack/test/controller/new_base/render_html_test.rb index e9ea57e329..bfed136496 100644 --- a/actionpack/test/controller/new_base/render_html_test.rb +++ b/actionpack/test/controller/new_base/render_html_test.rb @@ -88,7 +88,7 @@ module RenderHtml test "rendering text from an action with default options renders the text with the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } get "/render_html/simple" assert_body "hello david" @@ -98,7 +98,7 @@ module RenderHtml test "rendering text from an action with default options renders the text without the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } get "/render_html/with_layout" diff --git a/actionpack/test/controller/new_base/render_plain_test.rb b/actionpack/test/controller/new_base/render_plain_test.rb index 0881442bd0..94afe7bcfe 100644 --- a/actionpack/test/controller/new_base/render_plain_test.rb +++ b/actionpack/test/controller/new_base/render_plain_test.rb @@ -80,7 +80,7 @@ module RenderPlain test "rendering text from an action with default options renders the text with the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } get "/render_plain/simple" assert_body "hello david" @@ -90,7 +90,7 @@ module RenderPlain test "rendering text from an action with default options renders the text without the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } get "/render_plain/with_layout" diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb index b06ce5db40..0d4c7cdb0a 100644 --- a/actionpack/test/controller/new_base/render_template_test.rb +++ b/actionpack/test/controller/new_base/render_template_test.rb @@ -177,7 +177,7 @@ module RenderTemplate class TestWithLayout < Rack::TestCase test "rendering with implicit layout" do with_routing do |set| - set.draw { get ':controller', :action => :index } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', :action => :index } } get "/render_template/with_layout" diff --git a/actionpack/test/controller/new_base/render_test.rb b/actionpack/test/controller/new_base/render_test.rb index 963f2c2f5c..1fb852a2c4 100644 --- a/actionpack/test/controller/new_base/render_test.rb +++ b/actionpack/test/controller/new_base/render_test.rb @@ -57,7 +57,9 @@ module Render test "render with blank" do with_routing do |set| set.draw do - get ":controller", :action => 'index' + ActiveSupport::Deprecation.silence do + get ":controller", :action => 'index' + end end get "/render/blank_render" @@ -70,7 +72,9 @@ module Render test "rendering more than once raises an exception" do with_routing do |set| set.draw do - get ":controller", :action => 'index' + ActiveSupport::Deprecation.silence do + get ":controller", :action => 'index' + end end assert_raises(AbstractController::DoubleRenderError) do diff --git a/actionpack/test/controller/new_base/render_text_test.rb b/actionpack/test/controller/new_base/render_text_test.rb index 048458178c..d4111d432c 100644 --- a/actionpack/test/controller/new_base/render_text_test.rb +++ b/actionpack/test/controller/new_base/render_text_test.rb @@ -83,7 +83,7 @@ module RenderText test "rendering text from an action with default options renders the text with the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } ActiveSupport::Deprecation.silence do get "/render_text/simple" @@ -96,7 +96,7 @@ module RenderText test "rendering text from an action with default options renders the text without the layout" do with_routing do |set| - set.draw { get ':controller', action: 'index' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller', action: 'index' } } ActiveSupport::Deprecation.silence do get "/render_text/with_layout" diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index bd43ff7697..17c62dc3fe 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -4,6 +4,8 @@ require 'active_support/core_ext/hash/transform_values' class ParametersAccessorsTest < ActiveSupport::TestCase setup do + ActionController::Parameters.permit_all_parameters = false + @params = ActionController::Parameters.new( person: { age: '32', @@ -129,9 +131,87 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params[:person].values_at(:name).first.permitted? end - test "equality with another hash works" do + test "equality with a hash is deprecated" do hash1 = { foo: :bar } params1 = ActionController::Parameters.new(hash1) - assert(params1 == hash1) + assert_deprecated("will be removed in Rails 5.1") do + assert(params1 == hash1) + end + end + + test "is equal to Parameters instance with same params" do + params1 = ActionController::Parameters.new(a: 1, b: 2) + params2 = ActionController::Parameters.new(a: 1, b: 2) + assert(params1 == params2) + end + + test "is equal to Parameters instance with same permitted params" do + params1 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + params2 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + assert(params1 == params2) + end + + test "is equal to Parameters instance with same different source params, but same permitted params" do + params1 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + params2 = ActionController::Parameters.new(a: 1, c: 3).permit(:a) + assert(params1 == params2) + assert(params2 == params1) + end + + test 'is not equal to an unpermitted Parameters instance with same params' do + params1 = ActionController::Parameters.new(a: 1).permit(:a) + params2 = ActionController::Parameters.new(a: 1) + assert(params1 != params2) + assert(params2 != params1) + end + + test "is not equal to Parameters instance with different permitted params" do + params1 = ActionController::Parameters.new(a: 1, b: 2).permit(:a, :b) + params2 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + assert(params1 != params2) + assert(params2 != params1) + end + + test "equality with simple types works" do + assert(@params != 'Hello') + assert(@params != 42) + assert(@params != false) + end + + test "inspect shows both class name, parameters and permitted flag" do + assert_equal( + '<ActionController::Parameters {"person"=>{"age"=>"32", '\ + '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ + '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}} permitted: false>', + @params.inspect + ) + end + + test "inspect prints updated permitted flag in the output" do + assert_match(/permitted: false/, @params.inspect) + + @params.permit! + + assert_match(/permitted: true/, @params.inspect) + end + + if Hash.method_defined?(:dig) + test "#dig delegates the dig method to its values" do + assert_equal "David", @params.dig(:person, :name, :first) + assert_equal "Chicago", @params.dig(:person, :addresses, 0, :city) + end + + test "#dig converts hashes to parameters" do + assert_kind_of ActionController::Parameters, @params.dig(:person) + assert_kind_of ActionController::Parameters, @params.dig(:person, :addresses, 0) + assert @params.dig(:person, :addresses).all? do |value| + value.is_a?(ActionController::Parameters) + end + end + else + test "ActionController::Parameters does not respond to #dig on Ruby 2.2" do + assert_not ActionController::Parameters.method_defined?(:dig) + assert_not @params.respond_to?(:dig) + end end end diff --git a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb index efaf8a96c3..c5bfb10b53 100644 --- a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb +++ b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb @@ -12,12 +12,6 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase ActionController::Parameters.always_permitted_parameters = %w( controller action ) end - test "shows deprecations warning on NEVER_UNPERMITTED_PARAMS" do - assert_deprecated do - ActionController::Parameters::NEVER_UNPERMITTED_PARAMS - end - end - test "returns super on missing constant other than NEVER_UNPERMITTED_PARAMS" do ActionController::Parameters.superclass.stub :const_missing, "super" do assert_equal "super", ActionController::Parameters::NON_EXISTING_CONSTANT diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 3299f2d9d0..b75eb0e3bf 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -27,6 +27,27 @@ class ParametersPermitTest < ActiveSupport::TestCase end end + def walk_permitted params + params.each do |k,v| + case v + when ActionController::Parameters + walk_permitted v + when Array + v.each { |x| walk_permitted v } + end + end + end + + test 'iteration should not impact permit' do + hash = {"foo"=>{"bar"=>{"0"=>{"baz"=>"hello", "zot"=>"1"}}}} + params = ActionController::Parameters.new(hash) + + walk_permitted params + + sanitized = params[:foo].permit(bar: [:baz]) + assert_equal({"0"=>{"baz"=>"hello"}}, sanitized[:bar].to_unsafe_h) + end + test 'if nothing is permitted, the hash becomes empty' do params = ActionController::Parameters.new(id: '1234') permitted = params.permit @@ -339,4 +360,13 @@ 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 end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 0b184eace9..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 @@ -286,7 +303,10 @@ class RedirectTest < ActionController::TestCase with_routing do |set| set.draw do resources :workshops - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :redirect_to_existing_record @@ -310,7 +330,7 @@ class RedirectTest < ActionController::TestCase error = assert_raise(ArgumentError) do get :redirect_to_params end - assert_equal "Generating an 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 @@ -328,7 +348,9 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_with_block_and_accepted_options with_routing do |set| set.draw do - get ':controller/:action' + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :redirect_to_with_block_and_options diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index d1b9586533..8d3134630d 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -16,6 +16,10 @@ 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') + end + def with_template if stale? template: 'test/hello_world' render plain: 'stale' @@ -26,6 +30,9 @@ end class ImplicitRenderTestController < ActionController::Base def empty_action end + + def empty_action_with_template + end end class TestController < ActionController::Base @@ -270,6 +277,18 @@ class ExpiresInRenderTest < ActionController::TestCase response.body end + def test_dynamic_render_with_absolute_path + file = Tempfile.new('name') + file.write "secrets!" + file.flush + assert_raises ActionView::MissingTemplate do + get :dynamic_render, params: { id: file.path } + end + ensure + file.close + file.unlink + end + def test_dynamic_render assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) assert_raises ActionView::MissingTemplate do @@ -370,7 +389,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 @@ -399,7 +418,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 @@ -427,7 +446,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 @@ -462,8 +481,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 @@ -473,7 +510,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 @@ -497,7 +534,7 @@ class EtagRenderTest < ActionController::TestCase begin File.write path, 'foo' - ActionView::Digestor.cache.clear + ActionView::LookupContext::DetailsKey.clear request.if_none_match = etag get :with_template @@ -508,9 +545,14 @@ class EtagRenderTest < ActionController::TestCase end end - def etag(record) - %(W/"#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") - 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 end class MetalRenderTest < ActionController::TestCase @@ -522,13 +564,38 @@ 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 - def test_implicit_no_content_response - get :empty_action + def test_implicit_no_content_response_as_browser + assert_raises(ActionController::UnknownFormat) do + get :empty_action + end + end + + def test_implicit_no_content_response_as_xhr + get :empty_action, xhr: true assert_response :no_content end + + def test_implicit_success_response_with_right_format + get :empty_action_with_template + assert_equal "<h1>Empty action rendered this implicitly.</h1>\n", @response.body + assert_response :success + end + + def test_implicit_unknown_format_response + assert_raises(ActionController::UnknownFormat) do + get :empty_action_with_template, format: 'json' + end + end end class HeadRenderTest < ActionController::TestCase @@ -582,7 +649,10 @@ class HeadRenderTest < ActionController::TestCase with_routing do |set| set.draw do resources :customers - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :head_with_location_object @@ -667,7 +737,7 @@ end class HttpCacheForeverTest < ActionController::TestCase class HttpCacheForeverController < ActionController::Base def cache_me_forever - http_cache_forever(public: params[:public], version: params[:version] || 'v1') do + http_cache_forever(public: params[:public]) do render plain: 'hello' end end @@ -677,20 +747,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 @@ -698,21 +772,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 - - get :cache_me_forever - assert_response :not_modified - @request.if_modified_since = @response.headers['Last-Modified'] - @request.if_none_match = @response.etag + assert_response :ok - get :cache_me_forever, params: {version: 'v2'} - assert_response :success - @request.if_modified_since = @response.headers['Last-Modified'] @request.if_none_match = @response.etag - - get :cache_me_forever, params: {version: 'v2'} + get :cache_me_forever assert_response :not_modified end end diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index f0fd7ddc5e..137236c496 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -72,7 +72,10 @@ class RenderXmlTest < ActionController::TestCase with_routing do |set| set.draw do resources :customers - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :render_with_object_location 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 1984ad8825..d3f2ec6aa1 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -133,7 +133,11 @@ class PerFormTokensController < ActionController::Base self.per_form_csrf_tokens = true def index - render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" + render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" + end + + def button_to + render inline: "<%= button_to 'Button', (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" end def post_one @@ -403,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 @@ -652,15 +687,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_accepts_token_for_correct_path_and_method get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -673,15 +702,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_path get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_two' @@ -693,15 +716,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_method get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -710,6 +727,50 @@ class PerFormTokensControllerTest < ActionController::TestCase end end + def test_rejects_token_for_incorrect_method_button_to + get :button_to, params: { form_method: 'delete' } + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, 'delete' + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_raises(ActionController::InvalidAuthenticityToken) do + patch :post_one, params: { custom_authenticity_token: form_token } + end + end + + test "Accepts proper token for implicit post method on button_to tag" do + get :button_to + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, 'post' + + # 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 + end + + %w{delete post patch}.each do |verb| + test "Accepts proper token for #{verb} method on button_to tag" do + get :button_to, params: { form_method: verb } + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, verb + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + send verb, :post_one, params: { custom_authenticity_token: form_token } + end + end + end + def test_accepts_global_csrf_token get :index @@ -726,15 +787,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_params get :index, params: {form_path: '/per_form_tokens/post_one?foo=bar'} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one?foo=baz' @@ -747,11 +802,20 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_trailing_slash_during_generation get :index, params: {form_path: '/per_form_tokens/post_one/'} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token + 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_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' @@ -764,11 +828,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_trailing_slash_during_validation get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + 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/' @@ -781,12 +841,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_method_is_case_insensitive get :index, params: {form_method: "POST"} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end - + 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 @@ -794,4 +849,19 @@ class PerFormTokensControllerTest < ActionController::TestCase end assert_response :success end + + private + def assert_presence_and_fetch_form_csrf_token + assert_select 'input[name="custom_authenticity_token"]' do |input| + form_csrf_token = input.first['value'] + assert_not_nil form_csrf_token + return form_csrf_token + end + end + + def assert_matches_session_token_on_server(form_token, method = 'post') + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', method) + assert_equal expected, actual + end end 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 a39fede5b9..03bf8f8295 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -15,7 +15,9 @@ class UriReservedCharactersRoutingTest < ActiveSupport::TestCase def setup @set = ActionDispatch::Routing::RouteSet.new @set.draw do - get ':controller/:action/:variable/*additional' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:variable/*additional' + end end safe, unsafe = %w(: @ & = + $ , ;), %w(^ ? # [ ]) @@ -300,7 +302,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase end def test_default_setup - rs.draw { get '/:controller(/:action(/:id))' } + rs.draw { ActiveSupport::Deprecation.silence { get '/:controller(/:action(/:id))' } } assert_equal({:controller => "content", :action => 'index'}, rs.recognize_path("/content")) assert_equal({:controller => "content", :action => 'list'}, rs.recognize_path("/content/list")) assert_equal({:controller => "content", :action => 'show', :id => '10'}, rs.recognize_path("/content/show/10")) @@ -323,7 +325,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_route_with_colon_first rs.draw do - get '/:controller/:action/:id', action: 'index', id: nil + ActiveSupport::Deprecation.silence do + get '/:controller/:action/:id', action: 'index', id: nil + end + get ':url', controller: 'content', action: 'translate' end @@ -331,7 +336,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase end def test_route_with_regexp_for_action - rs.draw { get '/:controller/:action', action: /auth[-|_].+/ } + rs.draw { ActiveSupport::Deprecation.silence { get '/:controller/:action', action: /auth[-|_].+/ } } assert_equal({ action: 'auth_google', controller: 'content' }, rs.recognize_path('/content/auth_google')) assert_equal({ action: 'auth-facebook', controller: 'content' }, rs.recognize_path('/content/auth-facebook')) @@ -342,8 +347,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_route_with_regexp_for_controller rs.draw do - get ':controller/:admintoken(/:action(/:id))', :controller => /admin\/.+/ - get '/:controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller/:admintoken(/:action(/:id))', :controller => /admin\/.+/ + get '/:controller(/:action(/:id))' + end end assert_equal({:controller => "admin/user", :admintoken => "foo", :action => "index"}, @@ -357,7 +364,9 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_route_with_regexp_and_captures_for_controller rs.draw do - get '/:controller(/:action(/:id))', :controller => /admin\/(accounts|users)/ + ActiveSupport::Deprecation.silence do + get '/:controller(/:action(/:id))', :controller => /admin\/(accounts|users)/ + end end assert_equal({:controller => "admin/accounts", :action => "index"}, rs.recognize_path("/admin/accounts")) assert_equal({:controller => "admin/users", :action => "index"}, rs.recognize_path("/admin/users")) @@ -366,11 +375,13 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_route_with_regexp_and_dot rs.draw do - get ':controller/:action/:file', - :controller => /admin|user/, - :action => /upload|download/, - :defaults => {:file => nil}, - :constraints => {:file => %r{[^/]+(\.[^/]+)?}} + ActiveSupport::Deprecation.silence do + get ':controller/:action/:file', + :controller => /admin|user/, + :action => /upload|download/, + :defaults => {:file => nil}, + :constraints => {:file => %r{[^/]+(\.[^/]+)?}} + end end # Without a file extension assert_equal '/user/download/file', @@ -457,7 +468,9 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_named_route_without_hash rs.draw do - get ':controller/:action/:id', :as => 'normal' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id', :as => 'normal' + end end end @@ -509,7 +522,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase rs.draw do get 'page/:year/:month/:day/:title' => 'page#show', :as => 'article', :year => /\d+/, :month => /\d+/, :day => /\d+/ - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end routes = setup_for_named_route @@ -519,7 +535,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase end def test_changing_controller - rs.draw { get ':controller/:action/:id' } + rs.draw { ActiveSupport::Deprecation.silence { get ':controller/:action/:id' } } get URI('http://test.host/admin/user/index/10') @@ -530,7 +546,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_paths_escaped rs.draw do get 'file/*path' => 'content#show_file', :as => 'path' - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end # No + to space in URI escaping, only for query params. @@ -555,7 +574,9 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_non_controllers_cannot_be_matched rs.draw do - get ':controller/:action/:id' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_raise(ActionController::RoutingError) { rs.recognize_path("/not_a/show/10") } end @@ -593,8 +614,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_backwards rs.draw do - get 'page/:id(/:action)' => 'pages#show' - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get 'page/:id(/:action)' => 'pages#show' + get ':controller(/:action(/:id))' + end end get URI('http://test.host/pages/show') @@ -603,10 +626,13 @@ 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 - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal '/page', url_for(rs, { :controller => 'content', :action => 'show_page' }) @@ -623,7 +649,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_route_with_text_default rs.draw do get 'page/:id' => 'content#show_page', :id => 1 - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal '/page/foo', url_for(rs, { :controller => 'content', :action => 'show_page', :id => 'foo' }) @@ -638,7 +667,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase end def test_action_expiry - rs.draw { get ':controller(/:action(/:id))' } + rs.draw { ActiveSupport::Deprecation.silence { get ':controller(/:action(/:id))' } } get URI('http://test.host/content/show') assert_equal '/content', controller.url_for(:controller => 'content', :only_path => true) end @@ -661,7 +690,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase :defaults => { :year => nil }, :constraints => { :year => /\d{4}/ } ) - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal '/test', url_for(rs, { :controller => 'post', :action => 'show' }) @@ -673,7 +705,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_set_to_nil_forgets rs.draw do get 'pages(/:year(/:month(/:day)))' => 'content#list_pages', :month => nil, :day => nil - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal '/pages/2005', @@ -720,7 +755,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_named_route_method rs.draw do get 'categories' => 'content#categories', :as => 'categories' - get ':controller(/:action(/:id))' + + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end assert_equal '/categories', url_for(rs, { :controller => 'content', :action => 'categories' }) @@ -736,7 +774,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase rs.draw do get 'journal' => 'content#list_journal', :date => nil, :user_id => nil - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal '/journal', url_for(rs, { @@ -776,10 +817,12 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_subpath_recognized rs.draw do - get '/books/:id/edit' => 'subpath_books#edit' - get '/items/:id/:action' => 'subpath_books' - get '/posts/new/:action' => 'subpath_books' - get '/posts/:id' => 'subpath_books#show' + ActiveSupport::Deprecation.silence do + get '/books/:id/edit' => 'subpath_books#edit' + get '/items/:id/:action' => 'subpath_books' + get '/posts/new/:action' => 'subpath_books' + get '/posts/:id' => 'subpath_books#show' + end end hash = rs.recognize_path "/books/17/edit" @@ -801,9 +844,11 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_subpath_generated rs.draw do - get '/books/:id/edit' => 'subpath_books#edit' - get '/items/:id/:action' => 'subpath_books' - get '/posts/new/:action' => 'subpath_books' + ActiveSupport::Deprecation.silence do + get '/books/:id/edit' => 'subpath_books#edit' + get '/items/:id/:action' => 'subpath_books' + get '/posts/new/:action' => 'subpath_books' + end end assert_equal "/books/7/edit", url_for(rs, { :controller => "subpath_books", :id => 7, :action => "edit" }) @@ -827,8 +872,11 @@ class LegacyRouteSetTests < ActiveSupport::TestCase get 'ca' => 'ca#aa' get 'cb' => 'cb#ab' get 'cc' => 'cc#ac' - get ':controller/:action/:id' - get ':controller/:action/:id.:format' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + get ':controller/:action/:id.:format' + end end hash = rs.recognize_path "/cc" @@ -839,8 +887,11 @@ class LegacyRouteSetTests < ActiveSupport::TestCase rs.draw do get 'cb' => 'cb#ab' get 'cc' => 'cc#ac' - get ':controller/:action/:id' - get ':controller/:action/:id.:format' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + get ':controller/:action/:id.:format' + end end hash = rs.recognize_path "/cc" @@ -871,29 +922,34 @@ class RouteSetTest < ActiveSupport::TestCase @default_route_set ||= begin set = ActionDispatch::Routing::RouteSet.new set.draw do - get '/:controller(/:action(/:id))' + + ActiveSupport::Deprecation.silence do + get '/:controller(/:action(/:id))' + end end set end end def test_generate_extras - set.draw { get ':controller/(:action(/:id))' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller/(:action(/:id))' } } path, extras = set.generate_extras(:controller => "foo", :action => "bar", :id => 15, :this => "hello", :that => "world") assert_equal "/foo/bar/15", path assert_equal %w(that this), extras.map(&:to_s).sort end def test_extra_keys - set.draw { get ':controller/:action/:id' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller/:action/:id' } } extras = set.extra_keys(:controller => "foo", :action => "bar", :id => 15, :this => "hello", :that => "world") assert_equal %w(that this), extras.map(&:to_s).sort end def test_generate_extras_not_first set.draw do - get ':controller/:action/:id.:format' - get ':controller/:action/:id' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id.:format' + get ':controller/:action/:id' + end end path, extras = set.generate_extras(:controller => "foo", :action => "bar", :id => 15, :this => "hello", :that => "world") assert_equal "/foo/bar/15", path @@ -902,8 +958,10 @@ class RouteSetTest < ActiveSupport::TestCase def test_generate_not_first set.draw do - get ':controller/:action/:id.:format' - get ':controller/:action/:id' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id.:format' + get ':controller/:action/:id' + end end assert_equal "/foo/bar/15?this=hello", url_for(set, { :controller => "foo", :action => "bar", :id => 15, :this => "hello" }) @@ -911,8 +969,10 @@ class RouteSetTest < ActiveSupport::TestCase def test_extra_keys_not_first set.draw do - get ':controller/:action/:id.:format' - get ':controller/:action/:id' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id.:format' + get ':controller/:action/:id' + end end extras = set.extra_keys(:controller => "foo", :action => "bar", :id => 15, :this => "hello", :that => "world") assert_equal %w(that this), extras.map(&:to_s).sort @@ -1044,7 +1104,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_draw_default_route set.draw do - get '/:controller/:action/:id' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal 1, set.routes.size @@ -1059,7 +1121,10 @@ class RouteSetTest < ActiveSupport::TestCase def test_route_with_parameter_shell set.draw do get 'page/:id' => 'pages#show', :id => /\d+/ - get '/:controller(/:action(/:id))' + + ActiveSupport::Deprecation.silence do + get '/:controller(/:action(/:id))' + end end assert_equal({:controller => 'pages', :action => 'index'}, request_path_params('/pages')) @@ -1314,7 +1379,9 @@ class RouteSetTest < ActiveSupport::TestCase @set = make_set false set.draw do - get ':controller/:id/:action' + ActiveSupport::Deprecation.silence do + get ':controller/:id/:action' + end end get URI('http://test.host/people/7/show') @@ -1327,7 +1394,10 @@ class RouteSetTest < ActiveSupport::TestCase set.draw do get 'about' => "welcome#about" - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:id/:action' + end end get URI('http://test.host/welcom/get/7') @@ -1338,7 +1408,7 @@ class RouteSetTest < ActiveSupport::TestCase end def test_generate - set.draw { get ':controller/:action/:id' } + set.draw { ActiveSupport::Deprecation.silence { get ':controller/:action/:id' } } args = { :controller => "foo", :action => "bar", :id => "7", :x => "y" } assert_equal "/foo/bar/7?x=y", url_for(set, args) @@ -1349,7 +1419,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_generate_with_path_prefix set.draw do scope "my" do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end end @@ -1360,7 +1432,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_generate_with_blank_path_prefix set.draw do scope "" do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end end @@ -1372,9 +1446,11 @@ class RouteSetTest < ActiveSupport::TestCase @set = make_set false set.draw do - get "/connection/manage(/:action)" => 'connection/manage#index' - get "/connection/connection" => "connection/connection#index" - get '/connection' => 'connection#index', :as => 'family_connection' + ActiveSupport::Deprecation.silence do + get "/connection/manage(/:action)" => 'connection/manage#index' + get "/connection/connection" => "connection/connection#index" + get '/connection' => 'connection#index', :as => 'family_connection' + end end assert_equal({ :controller => 'connection/manage', @@ -1392,7 +1468,9 @@ class RouteSetTest < ActiveSupport::TestCase @set = make_set false set.draw do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end get URI('http://test.host/books/show/10') @@ -1407,7 +1485,10 @@ class RouteSetTest < ActiveSupport::TestCase set.draw do get 'show_weblog/:parameter' => 'weblog#show' - get ':controller(/:action(/:id))' + + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end get URI('http://test.host/weblog/show/1') @@ -1435,7 +1516,7 @@ class RouteSetTest < ActiveSupport::TestCase def test_expiry_determination_should_consider_values_with_to_param @set = make_set false - set.draw { get 'projects/:project_id/:controller/:action' } + set.draw { ActiveSupport::Deprecation.silence { get 'projects/:project_id/:controller/:action' } } get URI('http://test.host/projects/1/weblog/show') @@ -1612,7 +1693,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_assign_route_options_with_anchor_chars set.draw do - get '/cars/:action/:person/:car/', :controller => 'cars' + ActiveSupport::Deprecation.silence do + get '/cars/:action/:person/:car/', :controller => 'cars' + end end assert_equal '/cars/buy/1/2', url_for(set, { :controller => 'cars', :action => 'buy', :person => '1', :car => '2' }) @@ -1622,7 +1705,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_segmentation_of_dot_path set.draw do - get '/books/:action.rss', :controller => 'books' + ActiveSupport::Deprecation.silence do + get '/books/:action.rss', :controller => 'books' + end end assert_equal '/books/list.rss', url_for(set, { :controller => 'books', :action => 'list' }) @@ -1632,7 +1717,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_segmentation_of_dynamic_dot_path set.draw do - get '/books(/:action(.:format))', :controller => 'books' + ActiveSupport::Deprecation.silence do + get '/books(/:action(.:format))', :controller => 'books' + end end assert_equal '/books/list.rss', url_for(set, { :controller => 'books', :action => 'list', :format => 'rss' }) @@ -1647,7 +1734,7 @@ class RouteSetTest < ActiveSupport::TestCase end def test_slashes_are_implied - set.draw { get("/:controller(/:action(/:id))") } + set.draw { ActiveSupport::Deprecation.silence { get("/:controller(/:action(/:id))") } } assert_equal '/content', url_for(set, { :controller => 'content', :action => 'index' }) assert_equal '/content/list', url_for(set, { :controller => 'content', :action => 'list' }) @@ -1738,7 +1825,9 @@ class RouteSetTest < ActiveSupport::TestCase :constraints => { :page => /\d+/ }, :defaults => { :page => 1 } - get ':controller/:action/:id' + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end assert_equal '/ibocorp', url_for(set, { :controller => 'ibocorp', :action => "show", :page => 1 }) @@ -1761,7 +1850,11 @@ class RouteSetTest < ActiveSupport::TestCase :day => nil, :month => nil get "blog/show/:id", :controller => "blog", :action => "show", :id => /\d+/ - get "blog/:controller/:action(/:id)" + + ActiveSupport::Deprecation.silence do + get "blog/:controller/:action(/:id)" + end + get "*anything", :controller => "blog", :action => "unknown_request" end @@ -1850,13 +1943,20 @@ class RackMountIntegrationTests < ActiveSupport::TestCase get 'news(.:format)' => "news#index" - get 'comment/:id(/:action)' => "comments#show" - get 'ws/:controller(/:action(/:id))', :ws => true - get 'account(/:action)' => "account#subscription" - get 'pages/:page_id/:controller(/:action(/:id))' - get ':controller/ping', :action => 'ping' + ActiveSupport::Deprecation.silence do + get 'comment/:id(/:action)' => "comments#show" + get 'ws/:controller(/:action(/:id))', :ws => true + get 'account(/:action)' => "account#subscription" + get 'pages/:page_id/:controller(/:action(/:id))' + get ':controller/ping', :action => 'ping' + end + get 'こんにちは/世界', :controller => 'news', :action => 'index' - match ':controller(/:action(/:id))(.:format)', :via => :all + + ActiveSupport::Deprecation.silence do + match ':controller(/:action(/:id))(.:format)', :via => :all + end + root :to => "news#index" } @@ -1964,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 b9caddcdb7..ea59156f65 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -137,6 +137,10 @@ XML head :created, location: 'created resource' end + def render_cookie + render plain: cookies["foo"] + end + def delete_cookie cookies.delete("foo") render plain: 'ok' @@ -150,6 +154,10 @@ XML render html: '<body class="foo"></body>'.html_safe end + def boom + raise 'boom!' + end + private def generate_url(opts) @@ -163,7 +171,9 @@ XML @request.delete_header 'PATH_INFO' @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end end end @@ -547,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 } } @@ -559,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( @@ -668,7 +678,10 @@ XML with_routing do |set| set.draw do get 'file/*path', to: 'test_case_test/test#test_params' - get ':controller/:action' + + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end get :test_params, params: { path: ['hello', 'world'] } @@ -829,6 +842,12 @@ XML assert_equal 'bar', cookies['foo'] end + def test_cookies_should_be_escaped_properly + cookies['foo'] = '+' + get :render_cookie + assert_equal '+', @response.body + end + def test_should_detect_if_cookie_is_deleted cookies['foo'] = 'bar' get :delete_cookie @@ -966,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 @@ -998,7 +1037,9 @@ class ResponseDefaultHeadersTest < ActionController::TestCase @request.env['PATH_INFO'] = nil @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end end end @@ -1125,7 +1166,9 @@ class AnonymousControllerTest < ActionController::TestCase @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end end end diff --git a/actionpack/test/controller/url_for_integration_test.rb b/actionpack/test/controller/url_for_integration_test.rb index dfc2712e3e..a6ca5fc868 100644 --- a/actionpack/test/controller/url_for_integration_test.rb +++ b/actionpack/test/controller/url_for_integration_test.rb @@ -52,12 +52,15 @@ module ActionPack get 'news(.:format)' => "news#index" - get 'comment/:id(/:action)' => "comments#show" - get 'ws/:controller(/:action(/:id))', :ws => true - get 'account(/:action)' => "account#subscription" - get 'pages/:page_id/:controller(/:action(/:id))' - get ':controller/ping', :action => 'ping' - get ':controller(/:action(/:id))(.:format)' + ActiveSupport::Deprecation.silence { + get 'comment/:id(/:action)' => "comments#show" + get 'ws/:controller(/:action(/:id))', :ws => true + get 'account(/:action)' => "account#subscription" + get 'pages/:page_id/:controller(/:action(/:id))' + get ':controller/ping', :action => 'ping' + get ':controller(/:action(/:id))(.:format)' + } + root :to => "news#index" } diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 67212fea38..b4d2088c0a 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -4,7 +4,13 @@ module AbstractController module Testing class UrlForTest < ActionController::TestCase class W - include ActionDispatch::Routing::RouteSet.new.tap { |r| r.draw { get ':controller(/:action(/:id(.:format)))' } }.url_helpers + include ActionDispatch::Routing::RouteSet.new.tap { |r| + r.draw { + ActiveSupport::Deprecation.silence { + get ':controller(/:action(/:id(.:format)))' + } + } + }.url_helpers end def teardown @@ -260,7 +266,7 @@ module AbstractController w = Class.new { config = ActionDispatch::Routing::RouteSet::Config.new '/subdir' r = ActionDispatch::Routing::RouteSet.new(config) - r.draw { get ':controller(/:action(/:id(.:format)))' } + r.draw { ActiveSupport::Deprecation.silence { get ':controller(/:action(/:id(.:format)))' } } include r.url_helpers } add_host!(w) @@ -315,7 +321,10 @@ module AbstractController with_routing do |set| set.draw do get 'home/sweet/home/:user', :to => 'home#index', :as => :home - get ':controller/:action/:id' + + ActiveSupport::Deprecation.silence do + get ':controller/:action/:id' + end end # We need to create a new class in order to install the new named route. diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index 5f2abc9606..bc0d215530 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -20,7 +20,9 @@ class UrlRewriterTests < ActionController::TestCase @rewriter = Rewriter.new(@request) #.new(@request, @params) @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do - get ':controller(/:action(/:id))' + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))' + end end end end diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 6d377c4691..daf17558aa 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -99,7 +99,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_parsing_json_doesnot_rescue_exception req = Class.new(ActionDispatch::Request) do def params_parsers - { Mime[:json] => Proc.new { |data| raise Interrupt } } + { json: Proc.new { |data| raise Interrupt } } end def content_length; get_header('rack.input').length; end diff --git a/actionpack/test/dispatch/callbacks_test.rb b/actionpack/test/dispatch/callbacks_test.rb index 5ba76d9ab9..7b707df7f6 100644 --- a/actionpack/test/dispatch/callbacks_test.rb +++ b/actionpack/test/dispatch/callbacks_test.rb @@ -37,13 +37,19 @@ class DispatcherTest < ActiveSupport::TestCase def test_to_prepare_and_cleanup_delegation prepared = cleaned = false - ActionDispatch::Callbacks.to_prepare { prepared = true } - ActionDispatch::Callbacks.to_prepare { cleaned = true } + assert_deprecated do + ActionDispatch::Callbacks.to_prepare { prepared = true } + ActionDispatch::Callbacks.to_prepare { cleaned = true } + end - ActionDispatch::Reloader.prepare! + assert_deprecated do + ActionDispatch::Reloader.prepare! + end assert prepared - ActionDispatch::Reloader.cleanup! + assert_deprecated do + ActionDispatch::Reloader.cleanup! + end assert cleaned end 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..5a39db145e 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 diff --git a/actionpack/test/dispatch/executor_test.rb b/actionpack/test/dispatch/executor_test.rb new file mode 100644 index 0000000000..28bb232ecd --- /dev/null +++ b/actionpack/test/dispatch/executor_test.rb @@ -0,0 +1,134 @@ +require 'abstract_unit' + +class ExecutorTest < ActiveSupport::TestCase + class MyBody < Array + def initialize(&block) + @on_close = block + end + + def foo + "foo" + end + + def bar + "bar" + end + + def close + @on_close.call if @on_close + end + end + + def test_returned_body_object_always_responds_to_close + body = call_and_return_body + assert_respond_to body, :close + end + + def test_returned_body_object_always_responds_to_close_even_if_called_twice + body = call_and_return_body + assert_respond_to body, :close + body.close + + body = call_and_return_body + assert_respond_to body, :close + body.close + end + + def test_returned_body_object_behaves_like_underlying_object + body = call_and_return_body do + b = MyBody.new + b << "hello" + b << "world" + [200, { "Content-Type" => "text/html" }, b] + end + assert_equal 2, body.size + assert_equal "hello", body[0] + assert_equal "world", body[1] + assert_equal "foo", body.foo + assert_equal "bar", body.bar + end + + def test_it_calls_close_on_underlying_object_when_close_is_called_on_body + close_called = false + body = call_and_return_body do + b = MyBody.new do + close_called = true + end + [200, { "Content-Type" => "text/html" }, b] + end + body.close + assert close_called + end + + def test_returned_body_object_responds_to_all_methods_supported_by_underlying_object + body = call_and_return_body do + [200, { "Content-Type" => "text/html" }, MyBody.new] + end + assert_respond_to body, :size + assert_respond_to body, :each + assert_respond_to body, :foo + assert_respond_to body, :bar + end + + def test_run_callbacks_are_called_before_close + running = false + executor.to_run { running = true } + + body = call_and_return_body + assert running + + running = false + body.close + assert !running + end + + def test_complete_callbacks_are_called_on_close + completed = false + executor.to_complete { completed = true } + + body = call_and_return_body + assert !completed + + body.close + assert completed + end + + def test_complete_callbacks_are_called_on_exceptions + completed = false + executor.to_complete { completed = true } + + begin + call_and_return_body do + raise "error" + end + rescue + end + + assert completed + end + + def test_callbacks_execute_in_shared_context + result = false + executor.to_run { @in_shared_context = true } + executor.to_complete { result = @in_shared_context } + + call_and_return_body.close + assert result + assert !defined?(@in_shared_context) # it's not in the test itself + end + + private + def call_and_return_body(&block) + app = middleware(block || proc { [200, {}, 'response'] }) + _, _, body = app.call({'rack.input' => StringIO.new('')}) + body + end + + def middleware(inner_app) + ActionDispatch::Executor.new(inner_app, executor) + end + + def executor + @executor ||= Class.new(ActiveSupport::Executor) + end +end 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..69098326b9 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -178,6 +178,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/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 149e37bf3d..672b272590 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -49,7 +49,7 @@ class MimeTypeTest < ActiveSupport::TestCase test "parse application with trailing star" do accept = "application/*" - expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip]] + expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip], Mime[:gzip]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed 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/reloader_test.rb b/actionpack/test/dispatch/reloader_test.rb index 62e8197e20..fe8a4a3a17 100644 --- a/actionpack/test/dispatch/reloader_test.rb +++ b/actionpack/test/dispatch/reloader_test.rb @@ -4,15 +4,17 @@ class ReloaderTest < ActiveSupport::TestCase Reloader = ActionDispatch::Reloader teardown do - Reloader.reset_callbacks :prepare - Reloader.reset_callbacks :cleanup + ActiveSupport::Reloader.reset_callbacks :prepare + ActiveSupport::Reloader.reset_callbacks :complete end def test_prepare_callbacks a = b = c = nil - Reloader.to_prepare { |*args| a = b = c = 1 } - Reloader.to_prepare { |*args| b = c = 2 } - Reloader.to_prepare { |*args| c = 3 } + assert_deprecated do + Reloader.to_prepare { |*args| a = b = c = 1 } + Reloader.to_prepare { |*args| b = c = 2 } + Reloader.to_prepare { |*args| c = 3 } + end # Ensure to_prepare callbacks are not run when defined assert_nil a || b || c @@ -60,9 +62,15 @@ class ReloaderTest < ActiveSupport::TestCase def test_condition_specifies_when_to_reload i, j = 0, 0, 0, 0 - Reloader.to_prepare { |*args| i += 1 } - Reloader.to_cleanup { |*args| j += 1 } - app = Reloader.new(lambda { |env| [200, {}, []] }, lambda { i < 3 }) + assert_deprecated do + Reloader.to_prepare { |*args| i += 1 } + Reloader.to_cleanup { |*args| j += 1 } + end + + x = Class.new(ActiveSupport::Reloader) + x.check = lambda { i < 3 } + + app = Reloader.new(lambda { |env| [200, {}, []] }, x) 5.times do resp = app.call({}) resp[2].close @@ -109,7 +117,9 @@ class ReloaderTest < ActiveSupport::TestCase def test_cleanup_callbacks_are_called_when_body_is_closed cleaned = false - Reloader.to_cleanup { cleaned = true } + assert_deprecated do + Reloader.to_cleanup { cleaned = true } + end body = call_and_return_body assert !cleaned @@ -120,7 +130,9 @@ class ReloaderTest < ActiveSupport::TestCase def test_prepare_callbacks_arent_called_when_body_is_closed prepared = false - Reloader.to_prepare { prepared = true } + assert_deprecated do + Reloader.to_prepare { prepared = true } + end body = call_and_return_body prepared = false @@ -131,31 +143,43 @@ class ReloaderTest < ActiveSupport::TestCase def test_manual_reloading prepared = cleaned = false - Reloader.to_prepare { prepared = true } - Reloader.to_cleanup { cleaned = true } + assert_deprecated do + Reloader.to_prepare { prepared = true } + Reloader.to_cleanup { cleaned = true } + end - Reloader.prepare! + assert_deprecated do + Reloader.prepare! + end assert prepared assert !cleaned prepared = cleaned = false - Reloader.cleanup! - assert !prepared + assert_deprecated do + Reloader.cleanup! + end + assert prepared assert cleaned end def test_prepend_prepare_callback i = 10 - Reloader.to_prepare { i += 1 } - Reloader.to_prepare(:prepend => true) { i = 0 } + assert_deprecated do + Reloader.to_prepare { i += 1 } + Reloader.to_prepare(:prepend => true) { i = 0 } + end - Reloader.prepare! + assert_deprecated do + Reloader.prepare! + end assert_equal 1, i end def test_cleanup_callbacks_are_called_on_exceptions cleaned = false - Reloader.to_cleanup { cleaned = true } + assert_deprecated do + Reloader.to_cleanup { cleaned = true } + end begin call_and_return_body do @@ -169,8 +193,11 @@ class ReloaderTest < ActiveSupport::TestCase private def call_and_return_body(&block) + x = Class.new(ActiveSupport::Reloader) + x.check = lambda { true } + @response ||= 'response' - @reloader ||= Reloader.new(block || proc {[200, {}, @response]}) + @reloader ||= Reloader.new(block || proc {[200, {}, @response]}, x) @reloader.call({'rack.input' => StringIO.new('')})[2] end end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index a3992ad008..a07138b55e 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -37,9 +37,9 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest ) end - test "parses json params for application/vnd.api+json" do + test "does not parse unregistered media types such as application/vnd.api+json" do assert_parses( - {"person" => {"name" => "David"}}, + {}, "{\"person\": {\"name\": \"David\"}}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } ) end @@ -103,7 +103,9 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest def with_test_routing with_routing do |set| set.draw do - post ':action', :to => ::JsonParamsParsingTest::TestController + ActiveSupport::Deprecation.silence do + post ':action', :to => ::JsonParamsParsingTest::TestController + end end yield end @@ -143,13 +145,6 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end - test "parses json params for application/vnd.api+json" do - assert_parses( - {"user" => {"username" => "sikachu"}, "username" => "sikachu"}, - "{\"username\": \"sikachu\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } - ) - end - test "parses json with non-object JSON content" do assert_parses( {"user" => {"_json" => "string content" }, "_json" => "string content" }, @@ -157,6 +152,34 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end + 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) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end + end + + 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) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end + end + private def assert_parses(expected, actual, headers = {}) with_test_routing(UsersController) do @@ -170,7 +193,9 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest def with_test_routing(controller) with_routing do |set| set.draw do - post ':action', :to => controller + ActiveSupport::Deprecation.silence do + post ':action', :to => controller + end end yield end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index b36fbd3c76..bab4413b2a 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -159,7 +159,9 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest test "does not raise EOFError on GET request with multipart content-type" do with_routing do |set| set.draw do - get ':action', controller: 'multipart_params_parsing_test/test' + ActiveSupport::Deprecation.silence do + get ':action', controller: 'multipart_params_parsing_test/test' + end end headers = { "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x" } get "/parse", headers: headers @@ -188,7 +190,9 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest def with_test_routing with_routing do |set| set.draw do - post ':action', :controller => 'multipart_params_parsing_test/test' + ActiveSupport::Deprecation.silence do + post ':action', :controller => 'multipart_params_parsing_test/test' + end end yield end diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index bc6716525e..f04022a544 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -144,7 +144,9 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest test "ambiguous query string returns a bad request" do with_routing do |set| set.draw do - get ':action', :to => ::QueryStringParsingTest::TestController + ActiveSupport::Deprecation.silence do + get ':action', :to => ::QueryStringParsingTest::TestController + end end get "/parse", headers: { "QUERY_STRING" => "foo[]=bar&foo[4]=bar" } @@ -156,7 +158,9 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest def assert_parses(expected, actual) with_routing do |set| set.draw do - get ':action', :to => ::QueryStringParsingTest::TestController + ActiveSupport::Deprecation.silence do + get ':action', :to => ::QueryStringParsingTest::TestController + end end @app = self.class.build_app(set) do |middleware| middleware.use(EarlyParse) 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/url_encoded_params_parsing_test.rb b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb index 365edf849a..b9f8c52378 100644 --- a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb @@ -140,7 +140,9 @@ class UrlEncodedParamsParsingTest < ActionDispatch::IntegrationTest def with_test_routing with_routing do |set| set.draw do - post ':action', to: ::UrlEncodedParamsParsingTest::TestController + ActiveSupport::Deprecation.silence do + post ':action', to: ::UrlEncodedParamsParsingTest::TestController + end end yield 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 8b3849cb7a..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' @@ -445,4 +495,20 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end + + 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.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 7382c267c7..5aafcb23c2 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -17,10 +17,10 @@ module ActionDispatch @set = ActionDispatch::Routing::RouteSet.new end - def draw(options = {}, &block) + def draw(options = nil, &block) @set.draw(&block) inspector = ActionDispatch::Routing::RoutesInspector.new(@set.routes) - inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options[:filter]).split("\n") + inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options).split("\n") end def test_displaying_routes_for_engines @@ -133,7 +133,9 @@ module ActionDispatch def test_inspect_routes_shows_dynamic_action_route output = draw do - get 'api/:action' => 'api' + ActiveSupport::Deprecation.silence do + get 'api/:action' => 'api' + end end assert_equal [ @@ -144,7 +146,9 @@ module ActionDispatch def test_inspect_routes_shows_controller_and_action_only_route output = draw do - get ':controller/:action' + ActiveSupport::Deprecation.silence do + get ':controller/:action' + end end assert_equal [ @@ -155,7 +159,9 @@ module ActionDispatch def test_inspect_routes_shows_controller_and_action_route_with_constraints output = draw do - get ':controller(/:action(/:id))', :id => /\d+/ + ActiveSupport::Deprecation.silence do + get ':controller(/:action(/:id))', :id => /\d+/ + end end assert_equal [ @@ -164,7 +170,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_route_with_defaults + def test_rails_routes_shows_route_with_defaults output = draw do get 'photos/:id' => 'photos#show', :defaults => {:format => 'jpg'} end @@ -175,7 +181,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_route_with_constraints + def test_rails_routes_shows_route_with_constraints output = draw do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end @@ -186,7 +192,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_routes_with_dashes + def test_rails_routes_shows_routes_with_dashes output = draw do get 'about-us' => 'pages#about_us' get 'our-work/latest' @@ -209,7 +215,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_route_with_rack_app + def test_rails_routes_shows_route_with_rack_app output = draw do get 'foo/:id' => MountedRackApp, :id => /[A-Z]\d{5}/ end @@ -220,7 +226,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_named_route_with_mounted_rack_app + def test_rails_routes_shows_named_route_with_mounted_rack_app output = draw do mount MountedRackApp => '/foo' end @@ -231,7 +237,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_overridden_named_route_with_mounted_rack_app_with_name + def test_rails_routes_shows_overridden_named_route_with_mounted_rack_app_with_name output = draw do mount MountedRackApp => '/foo', as: 'blog' end @@ -242,7 +248,7 @@ module ActionDispatch ], output end - def test_rake_routes_shows_route_with_rack_app_nested_with_dynamic_constraints + def test_rails_routes_shows_route_with_rack_app_nested_with_dynamic_constraints constraint = Class.new do def inspect "( my custom constraint )" @@ -261,7 +267,7 @@ module ActionDispatch ], output end - def test_rake_routes_dont_show_app_mounted_in_assets_prefix + def test_rails_routes_dont_show_app_mounted_in_assets_prefix output = draw do get '/sprockets' => MountedRackApp end @@ -269,7 +275,7 @@ module ActionDispatch assert_no_match(/\/sprockets/, output.first) end - def test_rake_routes_shows_route_defined_in_under_assets_prefix + def test_rails_routes_shows_route_defined_in_under_assets_prefix output = draw do scope '/sprockets' do get '/foo' => 'foo#bar' @@ -297,7 +303,7 @@ module ActionDispatch end def test_routes_can_be_filtered - output = draw(filter: 'posts') do + output = draw('posts') do resources :articles resources :posts end @@ -313,13 +319,35 @@ module ActionDispatch " DELETE /posts/:id(.:format) posts#destroy"], output end + def test_routes_can_be_filtered_with_namespaced_controllers + output = draw('admin/posts') do + resources :articles + namespace :admin do + resources :posts + end + end + + assert_equal [" Prefix Verb URI Pattern Controller#Action", + " admin_posts GET /admin/posts(.:format) admin/posts#index", + " POST /admin/posts(.:format) admin/posts#create", + " new_admin_post GET /admin/posts/new(.:format) admin/posts#new", + "edit_admin_post GET /admin/posts/:id/edit(.:format) admin/posts#edit", + " admin_post GET /admin/posts/:id(.:format) admin/posts#show", + " PATCH /admin/posts/:id(.:format) admin/posts#update", + " PUT /admin/posts/:id(.:format) admin/posts#update", + " DELETE /admin/posts/:id(.:format) admin/posts#destroy"], output + end + + def test_regression_route_with_controller_regexp output = draw do - get ':controller(/:action)', controller: /api\/[^\/]+/, format: false + ActiveSupport::Deprecation.silence do + get ':controller(/:action)', controller: /api\/[^\/]+/, format: false + end 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 @@ -336,18 +364,18 @@ module ActionDispatch end def test_routes_with_undefined_filter - output = draw(:filter => 'Rails::MissingController') do + output = draw(controller: 'Rails::MissingController') do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end assert_equal [ - "The controller Rails::MissingController does not exist!", + "No routes were found for this controller", "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." ], output end def test_no_routes_matched_filter - output = draw(:filter => 'rails/dummy') do + output = draw('rails/dummy') do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end @@ -358,7 +386,7 @@ module ActionDispatch end def test_no_routes_were_defined - output = draw(:filter => 'Rails::DummyController') { } + output = draw('Rails::DummyController') {} assert_equal [ "You don't have any routes defined!", @@ -369,6 +397,29 @@ module ActionDispatch ], output end + def test_displaying_routes_for_internal_engines + engine = Class.new(Rails::Engine) do + def self.inspect + "Blog::Engine" + end + end + engine.routes.draw do + get '/cart', to: 'cart#show' + post '/cart', to: 'cart#create' + patch '/cart', to: 'cart#update' + end + + output = draw do + get '/custom/assets', to: 'custom_assets#show' + mount engine => "/blog", as: "blog", internal: true + end + + assert_equal [ + " Prefix Verb URI Pattern Controller#Action", + "custom_assets GET /custom/assets(.:format) custom_assets#show", + ], output + end + end end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5ead9357ae..75fdc9469f 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -116,7 +116,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_raise(ArgumentError) do draw do namespace :admin do - get '/:controller(/:action(/:id(.:format)))' + ActiveSupport::Deprecation.silence do + get '/:controller(/:action(/:id(.:format)))' + end end end end @@ -125,7 +127,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_namespace_without_controller_segment draw do namespace :admin do - get 'hello/:controllers/:action' + ActiveSupport::Deprecation.silence do + get 'hello/:controllers/:action' + end end end get '/admin/hello/foo/new' @@ -427,7 +431,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get 'global/hide_notice' get 'global/export', :action => :export, :as => :export_request get '/export/:id/:file', :action => :export, :as => :export_download, :constraints => { :file => /.*/ } - get 'global/:action' + + ActiveSupport::Deprecation.silence do + get 'global/:action' + end end end @@ -450,7 +457,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_local draw do - get "/local/:action", :controller => "local" + ActiveSupport::Deprecation.silence do + get "/local/:action", :controller => "local" + end end get '/local/dashboard' @@ -1506,7 +1515,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_not_matching_shorthand_with_dynamic_parameters draw do - get ':controller/:action/admin' + ActiveSupport::Deprecation.silence do + get ':controller/:action/admin' + end end get '/finances/overview/admin' @@ -1542,7 +1553,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_scoped_controller_with_namespace_and_action draw do namespace :account do - get ':action/callback', :action => /twitter|github/, :controller => "callbacks", :as => :callback + ActiveSupport::Deprecation.silence do + get ':action/callback', :action => /twitter|github/, :controller => "callbacks", :as => :callback + end end end @@ -1837,7 +1850,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_url_generator_for_generic_route draw do - get "whatever/:controller(/:action(/:id))" + ActiveSupport::Deprecation.silence do + get "whatever/:controller(/:action(/:id))" + end end get '/whatever/foo/bar' @@ -1849,7 +1864,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_url_generator_for_namespaced_generic_route draw do - get "whatever/:controller(/:action(/:id))", :id => /\d+/ + ActiveSupport::Deprecation.silence do + get "whatever/:controller(/:action(/:id))", :id => /\d+/ + end end get '/whatever/foo/bar/show' @@ -3125,12 +3142,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end assert_raise(ArgumentError) do - assert_deprecated do - draw { controller("/feeds") { get '/feeds/:service', :to => :show } } - end - end - - assert_raise(ArgumentError) do draw { resources :feeds, :controller => '/feeds' } end end @@ -3599,6 +3610,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/?id=1', root_path(params) end + def test_dynamic_controller_segments_are_deprecated + assert_deprecated do + draw do + get '/:controller', action: 'index' + end + end + end + + def test_dynamic_action_segments_are_deprecated + assert_deprecated do + draw do + get '/pages/:action', controller: 'pages' + end + end + end + private def draw(&block) @@ -3964,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 @@ -3985,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 @@ -4122,7 +4150,11 @@ class TestOptimizedNamedRoutes < ActionDispatch::IntegrationTest app.draw do ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] } get '/foo' => ok, as: :foo - get '/post(/:action(/:id))' => ok, as: :posts + + ActiveSupport::Deprecation.silence do + get '/post(/:action(/:id))' => ok, as: :posts + end + get '/:foo/:foo_type/bars/:id' => ok, as: :bar get '/projects/:id.:format' => ok, as: :project get '/pages/:id' => ok, as: :page @@ -4292,11 +4324,16 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest test "invalid UTF-8 encoding returns a 400 Bad Request" do with_routing do |set| - set.draw do - get "/bar/:id", :to => redirect("/foo/show/%{id}") - get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show" - get "/foo(/:action(/:id))", :controller => "test_invalid_urls/foo" - get "/:controller(/:action(/:id))" + ActiveSupport::Deprecation.silence do + set.draw do + get "/bar/:id", :to => redirect("/foo/show/%{id}") + get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show" + + ActiveSupport::Deprecation.silence do + get "/foo(/:action(/:id))", :controller => "test_invalid_urls/foo" + get "/:controller(/:action(/:id))" + end + end end get "/%E2%EF%BF%BD%A6" @@ -4627,7 +4664,9 @@ class TestErrorsInController < ActionDispatch::IntegrationTest Routes = ActionDispatch::Routing::RouteSet.new Routes.draw do - get '/:controller(/:action)' + ActiveSupport::Deprecation.silence do + get '/:controller(/:action)' + end end APP = build_app Routes @@ -4717,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/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb index dbb996973d..769de1a1e0 100644 --- a/actionpack/test/dispatch/session/cache_store_test.rb +++ b/actionpack/test/dispatch/session/cache_store_test.rb @@ -164,7 +164,9 @@ class CacheStoreTest < ActionDispatch::IntegrationTest def with_test_route_set with_routing do |set| set.draw do - get ':action', :to => ::CacheStoreTest::TestController + ActiveSupport::Deprecation.silence do + get ':action', :to => ::CacheStoreTest::TestController + end end @app = self.class.build_app(set) do |middleware| diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index f07e215e3a..09cb1d925f 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -345,7 +345,9 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def with_test_route_set(options = {}) with_routing do |set| set.draw do - get ':action', :to => ::CookieStoreTest::TestController + ActiveSupport::Deprecation.silence do + get ':action', :to => ::CookieStoreTest::TestController + end end options = { :key => SessionKey }.merge!(options) diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index 3fed9bad4f..18cb227dad 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -187,7 +187,9 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest def with_test_route_set with_routing do |set| set.draw do - get ':action', :to => ::MemCacheStoreTest::TestController + ActiveSupport::Deprecation.silence do + get ':action', :to => ::MemCacheStoreTest::TestController + end end @app = self.class.build_app(set) do |middleware| diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index c66a0e6a7a..668b2b6cfe 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -7,7 +7,7 @@ class SSLTest < ActionDispatch::IntegrationTest def build_app(headers: {}, ssl_options: {}) headers = HEADERS.merge(headers) - ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options + ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options.reverse_merge(hsts: { subdomains: true }) end end @@ -39,6 +39,13 @@ class RedirectSSLTest < SSLTest assert_equal redirect[:body].join, @response.body end + test 'exclude can avoid redirect' do + excluding = { exclude: -> request { request.path =~ /healthcheck/ } } + + assert_not_redirected 'http://example.org/healthcheck', redirect: excluding + assert_redirected from: 'http://example.org/', redirect: excluding + end + test 'https is not redirected' do assert_not_redirected 'https://example.org' end @@ -98,15 +105,16 @@ end class StrictTransportSecurityTest < SSLTest EXPECTED = 'max-age=15552000' + EXPECTED_WITH_SUBDOMAINS = 'max-age=15552000; includeSubDomains' - def assert_hsts(expected, url: 'https://example.org', hsts: {}, headers: {}) + def assert_hsts(expected, url: 'https://example.org', hsts: { subdomains: true }, headers: {}) self.app = build_app ssl_options: { hsts: hsts }, headers: headers get url assert_equal expected, response.headers['Strict-Transport-Security'] end test 'enabled by default' do - assert_hsts EXPECTED + assert_hsts EXPECTED_WITH_SUBDOMAINS end test 'not sent with http:// responses' do @@ -126,11 +134,15 @@ class StrictTransportSecurityTest < SSLTest end test ':expires sets max-age' do - assert_hsts 'max-age=500', hsts: { expires: 500 } + assert_deprecated do + assert_hsts 'max-age=500', hsts: { expires: 500 } + end end test ':expires supports AS::Duration arguments' do - assert_hsts 'max-age=31557600', hsts: { expires: 1.year } + assert_deprecated do + assert_hsts 'max-age=31557600', hsts: { expires: 1.year } + end end test 'include subdomains' do @@ -142,11 +154,15 @@ class StrictTransportSecurityTest < SSLTest end test 'opt in to browser preload lists' do - assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + assert_deprecated do + assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + end end test 'opt out of browser preload lists' do - assert_hsts EXPECTED, hsts: { preload: false } + assert_deprecated do + assert_hsts EXPECTED, hsts: { preload: false } + end 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/collection_cache/index.html.erb b/actionpack/test/fixtures/collection_cache/index.html.erb index 521b1450df..853e501ab4 100644 --- a/actionpack/test/fixtures/collection_cache/index.html.erb +++ b/actionpack/test/fixtures/collection_cache/index.html.erb @@ -1 +1 @@ -<%= render @customers %>
\ No newline at end of file +<%= render partial: 'customers/customer', collection: @customers, cached: true %> 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 new file mode 100644 index 0000000000..951c761995 --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb @@ -0,0 +1,3 @@ +<body> +<%= cache 'with_options', skip_digest: true, expires_in: 10 do %><p>ERB</p><% end %> +</body> diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb new file mode 100644 index 0000000000..ded99ba52d --- /dev/null +++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb @@ -0,0 +1 @@ +mobile diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb new file mode 100644 index 0000000000..dd294f8cf6 --- /dev/null +++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb @@ -0,0 +1 @@ +<h1>Empty action rendered this implicitly.</h1> diff --git a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb index 317801ad30..317801ad30 100644 --- a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb +++ b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 15d51e5d6c..75caf56d32 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -3,7 +3,7 @@ require 'abstract_unit' module ActionDispatch module Journey class TestRouter < ActiveSupport::TestCase - attr_reader :routes, :mapper + attr_reader :mapper, :routes, :route_set, :router def setup @app = Routing::RouteSet::Dispatcher.new({}) @@ -15,36 +15,36 @@ module ActionDispatch end def test_dashes - mapper.get '/foo-bar-baz', to: 'foo#bar' + get '/foo-bar-baz', to: 'foo#bar' env = rails_env 'PATH_INFO' => '/foo-bar-baz' called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end assert called end def test_unicode - mapper.get '/ほげ', to: 'foo#bar' + get '/ほげ', to: 'foo#bar' #match the escaped version of /ほげ env = rails_env 'PATH_INFO' => '/%E3%81%BB%E3%81%92' called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end assert called end def test_regexp_first_precedence - mapper.get "/whois/:domain", :domain => /\w+\.[\w\.]+/, to: "foo#bar" - mapper.get "/whois/:id(.:format)", to: "foo#baz" + get "/whois/:domain", :domain => /\w+\.[\w\.]+/, to: "foo#bar" + get "/whois/:id(.:format)", to: "foo#baz" env = rails_env 'PATH_INFO' => '/whois/example.com' list = [] - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| list << r end assert_equal 2, list.length @@ -55,7 +55,7 @@ module ActionDispatch end def test_required_parts_verified_are_anchored - mapper.get "/foo/:id", :id => /\d/, anchor: false, to: "foo#bar" + get "/foo/:id", :id => /\d/, anchor: false, to: "foo#bar" assert_raises(ActionController::UrlGenerationError) do @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => '10' }, { }) @@ -63,7 +63,7 @@ module ActionDispatch end def test_required_parts_are_verified_when_building - mapper.get "/foo/:id", :id => /\d+/, anchor: false, to: "foo#bar" + get "/foo/:id", :id => /\d+/, anchor: false, to: "foo#bar" path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => '10' }, { }) assert_equal '/foo/10', path @@ -74,7 +74,7 @@ module ActionDispatch end def test_only_required_parts_are_verified - mapper.get "/foo(/:id)", :id => /\d/, :to => "foo#bar" + get "/foo(/:id)", :id => /\d/, :to => "foo#bar" path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :id => '10' }, { }) assert_equal '/foo/10', path @@ -88,8 +88,7 @@ module ActionDispatch def test_knows_what_parts_are_missing_from_named_route route_name = "gorby_thunderhorse" - mapper = ActionDispatch::Routing::Mapper.new @route_set - mapper.get "/foo/:id", :as => route_name, :id => /\d+/, :to => "foo#bar" + get "/foo/:id", :as => route_name, :id => /\d+/, :to => "foo#bar" error = assert_raises(ActionController::UrlGenerationError) do @formatter.generate(route_name, { }, { }) @@ -109,19 +108,16 @@ module ActionDispatch end def test_X_Cascade - mapper.get "/messages(.:format)", to: "foo#bar" - resp = @router.serve(rails_env({ 'REQUEST_METHOD' => 'GET', 'PATH_INFO' => '/lol' })) + get "/messages(.:format)", to: "foo#bar" + resp = router.serve(rails_env({ 'REQUEST_METHOD' => 'GET', 'PATH_INFO' => '/lol' })) assert_equal ['Not Found'], resp.last assert_equal 'pass', resp[1]['X-Cascade'] assert_equal 404, resp.first end def test_clear_trailing_slash_from_script_name_on_root_unanchored_routes - route_set = Routing::RouteSet.new - mapper = Routing::Mapper.new route_set - app = lambda { |env| [200, {}, ['success!']] } - mapper.get '/weblog', :to => app + get '/weblog', :to => app env = rack_env('SCRIPT_NAME' => '', 'PATH_INFO' => '/weblog') resp = route_set.call env @@ -130,38 +126,38 @@ module ActionDispatch end def test_defaults_merge_correctly - mapper.get '/foo(/:id)', to: "foo#bar", id: nil + get '/foo(/:id)', to: "foo#bar", id: nil env = rails_env 'PATH_INFO' => '/foo/10' - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| assert_equal({:id => '10', :controller => "foo", :action => "bar"}, params) end env = rails_env 'PATH_INFO' => '/foo' - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| assert_equal({:id => nil, :controller => "foo", :action => "bar"}, params) end end def test_recognize_with_unbound_regexp - mapper.get "/foo", anchor: false, to: "foo#bar" + get "/foo", anchor: false, to: "foo#bar" env = rails_env 'PATH_INFO' => '/foo/bar' - @router.recognize(env) { |*_| } + router.recognize(env) { |*_| } assert_equal '/foo', env.env['SCRIPT_NAME'] assert_equal '/bar', env.env['PATH_INFO'] end def test_bound_regexp_keeps_path_info - mapper.get "/foo", to: "foo#bar" + get "/foo", to: "foo#bar" env = rails_env 'PATH_INFO' => '/foo' before = env.env['SCRIPT_NAME'] - @router.recognize(env) { |*_| } + router.recognize(env) { |*_| } assert_equal before, env.env['SCRIPT_NAME'] assert_equal '/foo', env.env['PATH_INFO'] @@ -174,41 +170,41 @@ module ActionDispatch "/messages/:id/edit(.:format)", "/messages/:id(.:format)" ].each do |path| - mapper.get path, to: "foo#bar" + get path, to: "foo#bar" end env = rails_env 'PATH_INFO' => '/messages/unknown/path' yielded = false - @router.recognize(env) do |*whatever| + router.recognize(env) do |*whatever| yielded = true end assert_not yielded end def test_required_part_in_recall - mapper.get "/messages/:a/:b", to: "foo#bar" + get "/messages/:a/:b", to: "foo#bar" path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar", :a => 'a' }, { :b => 'b' }) assert_equal "/messages/a/b", path end def test_splat_in_recall - mapper.get "/*path", to: "foo#bar" + get "/*path", to: "foo#bar" path, _ = @formatter.generate(nil, { :controller => "foo", :action => "bar" }, { :path => 'b' }) assert_equal "/b", path end def test_recall_should_be_used_when_scoring - mapper.get "/messages/:action(/:id(.:format))", to: 'foo#bar' - mapper.get "/messages/:id(.:format)", to: 'bar#baz' + get "/messages/:action(/:id(.:format))", to: 'foo#bar' + get "/messages/:id(.:format)", to: 'bar#baz' path, _ = @formatter.generate(nil, { :controller => "foo", :id => 10 }, { :action => 'index' }) assert_equal "/messages/index/10", path end def test_nil_path_parts_are_ignored - mapper.get "/:controller(/:action(.:format))", to: "tasks#lol" + get "/:controller(/:action(.:format))", to: "tasks#lol" params = { :controller => "tasks", :format => nil } extras = { :action => 'lol' } @@ -220,14 +216,14 @@ module ActionDispatch def test_generate_slash params = [ [:controller, "tasks"], [:action, "show"] ] - mapper.get "/", Hash[params] + get "/", Hash[params] path, _ = @formatter.generate(nil, Hash[params], {}) assert_equal '/', path end def test_generate_calls_param_proc - mapper.get '/:controller(/:action)', to: "foo#bar" + get '/:controller(/:action)', to: "foo#bar" parameterized = [] params = [ [:controller, "tasks"], @@ -243,7 +239,7 @@ module ActionDispatch end def test_generate_id - mapper.get '/:controller(/:action)', to: 'foo#bar' + get '/:controller(/:action)', to: 'foo#bar' path, params = @formatter.generate( nil, {:id=>1, :controller=>"tasks", :action=>"show"}, {}) @@ -252,7 +248,7 @@ module ActionDispatch end def test_generate_escapes - mapper.get '/:controller(/:action)', to: "foo#bar" + get '/:controller(/:action)', to: "foo#bar" path, _ = @formatter.generate(nil, { :controller => "tasks", @@ -262,7 +258,7 @@ module ActionDispatch end def test_generate_escapes_with_namespaced_controller - mapper.get '/:controller(/:action)', to: "foo#bar" + get '/:controller(/:action)', to: "foo#bar" path, _ = @formatter.generate( nil, { :controller => "admin/tasks", @@ -272,7 +268,7 @@ module ActionDispatch end def test_generate_extra_params - mapper.get '/:controller(/:action)', to: "foo#bar" + get '/:controller(/:action)', to: "foo#bar" path, params = @formatter.generate( nil, { :id => 1, @@ -285,7 +281,7 @@ module ActionDispatch end def test_generate_missing_keys_no_matches_different_format_keys - mapper.get '/:controller/:action/:name', to: "foo#bar" + get '/:controller/:action/:name', to: "foo#bar" primarty_parameters = { :id => 1, :controller => "tasks", @@ -311,7 +307,7 @@ module ActionDispatch end def test_generate_uses_recall_if_needed - mapper.get '/:controller(/:action(/:id))', to: "foo#bar" + get '/:controller(/:action(/:id))', to: "foo#bar" path, params = @formatter.generate( nil, @@ -322,7 +318,7 @@ module ActionDispatch end def test_generate_with_name - mapper.get '/:controller(/:action)', to: 'foo#bar', as: 'tasks' + get '/:controller(/:action)', to: 'foo#bar', as: 'tasks' path, params = @formatter.generate( "tasks", @@ -338,13 +334,13 @@ module ActionDispatch '/content/show/10' => { :controller => 'content', :action => 'show', :id => "10" }, }.each do |request_path, expected| define_method("test_recognize_#{expected.keys.map(&:to_s).join('_')}") do - mapper.get "/:controller(/:action(/:id))", to: 'foo#bar' + get "/:controller(/:action(/:id))", to: 'foo#bar' route = @routes.first env = rails_env 'PATH_INFO' => request_path called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| assert_equal route, r assert_equal({ :action => "bar" }.merge(expected), params) called = true @@ -359,13 +355,13 @@ module ActionDispatch :splat => ['/segment/a/b%20c+d', { :segment => 'segment', :splat => 'a/b c+d' }] }.each do |name, (request_path, expected)| define_method("test_recognize_#{name}") do - mapper.get '/:segment/*splat', to: 'foo#bar' + get '/:segment/*splat', to: 'foo#bar' env = rails_env 'PATH_INFO' => request_path called = false route = @routes.first - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| assert_equal route, r assert_equal(expected.merge(:controller=>"foo", :action=>"bar"), params) called = true @@ -376,7 +372,7 @@ module ActionDispatch end def test_namespaced_controller - mapper.get "/:controller(/:action(/:id))", { :controller => /.+?/ } + get "/:controller(/:action(/:id))", { :controller => /.+?/ } route = @routes.first env = rails_env 'PATH_INFO' => '/admin/users/show/10' @@ -387,7 +383,7 @@ module ActionDispatch :id => '10' } - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| assert_equal route, r assert_equal(expected, params) called = true @@ -396,13 +392,13 @@ module ActionDispatch end def test_recognize_literal - mapper.get "/books(/:action(.:format))", controller: "books" + get "/books(/:action(.:format))", controller: "books" route = @routes.first env = rails_env 'PATH_INFO' => '/books/list.rss' expected = { :controller => 'books', :action => 'list', :format => 'rss' } called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| assert_equal route, r assert_equal(expected, params) called = true @@ -412,7 +408,7 @@ module ActionDispatch end def test_recognize_head_route - mapper.match "/books(/:action(.:format))", via: 'head', to: 'foo#bar' + match "/books(/:action(.:format))", via: 'head', to: 'foo#bar' env = rails_env( 'PATH_INFO' => '/books/list.rss', @@ -420,7 +416,7 @@ module ActionDispatch ) called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end @@ -428,13 +424,13 @@ module ActionDispatch end def test_recognize_head_request_as_get_route - mapper.get "/books(/:action(.:format))", to: 'foo#bar' + get "/books(/:action(.:format))", to: 'foo#bar' env = rails_env 'PATH_INFO' => '/books/list.rss', "REQUEST_METHOD" => "HEAD" called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end @@ -442,13 +438,13 @@ module ActionDispatch end def test_recognize_cares_about_get_verbs - mapper.match "/books(/:action(.:format))", to: "foo#bar", via: :get + match "/books(/:action(.:format))", to: "foo#bar", via: :get env = rails_env 'PATH_INFO' => '/books/list.rss', "REQUEST_METHOD" => "POST" called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end @@ -456,13 +452,13 @@ module ActionDispatch end def test_recognize_cares_about_post_verbs - mapper.match "/books(/:action(.:format))", to: "foo#bar", via: :post + match "/books(/:action(.:format))", to: "foo#bar", via: :post env = rails_env 'PATH_INFO' => '/books/list.rss', "REQUEST_METHOD" => "POST" called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end @@ -470,14 +466,14 @@ module ActionDispatch end def test_multi_verb_recognition - mapper.match "/books(/:action(.:format))", to: "foo#bar", via: [:post, :get] + match "/books(/:action(.:format))", to: "foo#bar", via: [:post, :get] %w( POST GET ).each do |verb| env = rails_env 'PATH_INFO' => '/books/list.rss', "REQUEST_METHOD" => verb called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end @@ -488,7 +484,7 @@ module ActionDispatch "REQUEST_METHOD" => 'PUT' called = false - @router.recognize(env) do |r, params| + router.recognize(env) do |r, params| called = true end @@ -497,6 +493,18 @@ module ActionDispatch private + def get *args + ActiveSupport::Deprecation.silence do + mapper.get(*args) + end + end + + def match *args + ActiveSupport::Deprecation.silence do + mapper.match(*args) + end + end + def rails_env env, klass = ActionDispatch::Request klass.new(rack_env(env)) end |