diff options
Diffstat (limited to 'actionpack')
19 files changed, 168 insertions, 35 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 608512d846..3314a0b77d 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,20 @@ +## Rails 5.0.0.beta1 (December 18, 2015) ## + +* No changes. + + +* 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 diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 7f349f2741..8edea0f52b 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -49,7 +49,7 @@ module AbstractController # instance methods on that abstract class. Public instance methods of # a controller would normally be considered action methods, so methods # declared on abstract classes are being removed. - # (ActionController::Metal and ActionController::Base are defined as abstract) + # (<tt>ActionController::Metal</tt> and ActionController::Base are defined as abstract) def internal_methods controller = self @@ -80,7 +80,7 @@ module AbstractController # action_methods are cached and there is sometimes need to refresh # them. ::clear_action_methods! allows you to do that, so next time - # you run action_methods, they will be recalculated + # you run action_methods, they will be recalculated. def clear_action_methods! @action_methods = nil end diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index d5317e4717..d63ce9c1c3 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -48,7 +48,8 @@ module AbstractController def _normalize_callback_option(options, from, to) # :nodoc: if from = options[from] - from = Array(from).map {|o| "action_name == '#{o}'"}.join(" || ") + _from = Array(from).map(&:to_s).to_set + from = proc {|c| _from.include? c.action_name } options[to] = Array(options[to]).unshift(from) end end diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index c42384b741..b9ad51a9cf 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -62,7 +62,7 @@ module ActionController # with the specified +key+ value. The key is expanded using # ActiveSupport::Cache.expand_cache_key. def fragment_cache_key(key) - head = self.class.fragment_cache_keys.map { |key| instance_exec(&key) } + head = self.class.fragment_cache_keys.map { |k| instance_exec(&k) } tail = key.is_a?(Hash) ? url_for(key).split("://").last : key ActiveSupport::Cache.expand_cache_key([*head, *tail], :views) end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 8e040bb465..f6a93a8940 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -166,7 +166,7 @@ module ActionController alias :response_code :status # :nodoc: - # Basic url_for that can be overridden for more robust functionality + # Basic url_for that can be overridden for more robust functionality. def url_for(string) string end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 0a36fecd27..2ac6e37e34 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -397,7 +397,7 @@ module ActionController # RewriteRule ^(.*)$ dispatch.fcgi [E=X-HTTP_AUTHORIZATION:%{HTTP:Authorization},QSA,L] module Token TOKEN_KEY = 'token=' - TOKEN_REGEX = /^(Token|Bearer) / + TOKEN_REGEX = /^(Token|Bearer)\s+/ AUTHN_PAIR_DELIMITERS = /(?:,|;|\t+)/ extend self diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 0febc905f1..b13ba06962 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -20,8 +20,6 @@ module ActionController # * <tt>String</tt> starting with <tt>protocol://</tt> (like <tt>http://</tt>) or a protocol relative reference (like <tt>//</tt>) - Is passed straight through as the target for redirection. # * <tt>String</tt> not containing a protocol - The current protocol and host is prepended to the string. # * <tt>Proc</tt> - A block that will be executed in the controller's context. Should return any option accepted by +redirect_to+. - # * <tt>:back</tt> - Back to the page that issued the request. Useful for forms that are triggered from multiple places. - # Short-hand for <tt>redirect_to(request.env["HTTP_REFERER"])</tt> # # === Examples: # @@ -30,7 +28,6 @@ module ActionController # redirect_to "http://www.rubyonrails.org" # redirect_to "/images/screenshot.jpg" # redirect_to articles_url - # redirect_to :back # redirect_to proc { edit_post_url(@post) } # # The redirection happens as a "302 Found" header unless otherwise specified using the <tt>:status</tt> option: @@ -61,13 +58,8 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # - # When using <tt>redirect_to :back</tt>, if there is no referrer, - # <tt>ActionController::RedirectBackError</tt> will be raised. You - # may specify some fallback behavior for this case by rescuing - # <tt>ActionController::RedirectBackError</tt>. def redirect_to(options = {}, response_status = {}) #:doc: raise ActionControllerError.new("Cannot redirect to nil!") unless options - raise ActionControllerError.new("Cannot redirect to a parameter hash!") if options.is_a?(ActionController::Parameters) raise AbstractController::DoubleRenderError if response_body self.status = _extract_redirect_to_status(options, response_status) @@ -75,6 +67,32 @@ module ActionController self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" end + # Redirects the browser to the page that issued the request (the referrer) + # if possible, otherwise redirects to the provided default fallback + # location. + # + # The referrer information is pulled from the HTTP `Referer` (sic) header on + # the request. This is an optional header and its presence on the request is + # subject to browser security settings and user preferences. If the request + # is missing this header, the <tt>fallback_location</tt> will be used. + # + # redirect_back fallback_location: { action: "show", id: 5 } + # redirect_back fallback_location: post + # redirect_back fallback_location: "http://www.rubyonrails.org" + # redirect_back fallback_location: "/images/screenshot.jpg" + # redirect_back fallback_location: articles_url + # 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. + def redirect_back(fallback_location:, **args) + if referer = request.headers["Referer"] + redirect_to referer, **args + else + redirect_to fallback_location, **args + end + end + def _compute_redirect_to_location(request, options) #:nodoc: case options # The scheme name consist of a letter followed by any combination of @@ -87,6 +105,12 @@ module ActionController when String request.protocol + request.host_with_port + options when :back + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + `redirect_to :back` is deprecated and will be removed from Rails 5.1. + Please use `redirect_back(fallback_location: fallback_location)` where + `fallback_location` represents the location to use if the request has + no HTTP referer information. + MESSAGE request.headers["Referer"] or raise RedirectBackError when Proc _compute_redirect_to_location request, options.call diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 8bc3c271e2..957aa746c0 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/hash/transform_values' require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/string/filters' require 'active_support/rescuable' @@ -175,7 +176,7 @@ module ActionController # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} def to_h if permitted? - @parameters.deep_dup + convert_parameters_to_hashes(@parameters) else slice(*self.class.always_permitted_parameters).permit!.to_h end @@ -185,7 +186,7 @@ module ActionController # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this # parameter. def to_unsafe_h - @parameters.deep_dup + convert_parameters_to_hashes(@parameters) end alias_method :to_unsafe_hash, :to_unsafe_h @@ -594,6 +595,21 @@ module ActionController end end + def convert_parameters_to_hashes(value) + case value + when Array + value.map { |v| convert_parameters_to_hashes(v) } + when Hash + value.transform_values do |v| + convert_parameters_to_hashes(v) + end.with_indifferent_access + when Parameters + value.to_h + else + value + end + end + def convert_hashes_to_parameters(key, value) converted = convert_value_to_parameters(value) @parameters[key] = converted unless converted.equal?(value) diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 0e636b8257..429a98f236 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -36,7 +36,7 @@ module ActionDispatch # development: # secret_key_base: 'secret key' # - # To generate a secret key for an existing application, run `rake secret`. + # To generate a secret key for an existing application, run `rails secret`. # # If you are upgrading an existing Rails 3 app, you should leave your # existing secret_token in place and simply add the new secret_key_base. diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 59c3f9248f..d00b2c3eb5 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -237,7 +237,7 @@ module ActionDispatch # # == View a list of all your routes # - # rake routes + # rails routes # # Target specific controllers by prefixing the command with <tt>CONTROLLER=x</tt>. # diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index b6c031dcf4..f91679593e 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -172,8 +172,11 @@ module ActionDispatch _routes.url_for(options.symbolize_keys.reverse_merge!(url_options), route_name) when ActionController::Parameters + unless options.permitted? + raise ArgumentError.new("Generating an URL from non sanitized request parameters is insecure!") + end route_name = options.delete :use_route - _routes.url_for(options.to_unsafe_h.symbolize_keys. + _routes.url_for(options.to_h.symbolize_keys. reverse_merge!(url_options), route_name) when String options diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 255ac9f4ed..5cfb5f02d8 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -8,7 +8,7 @@ module ActionPack MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "alpha" + PRE = "beta1" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actionpack/test/assertions/response_assertions_test.rb b/actionpack/test/assertions/response_assertions_test.rb index e76c222824..841fa6aaad 100644 --- a/actionpack/test/assertions/response_assertions_test.rb +++ b/actionpack/test/assertions/response_assertions_test.rb @@ -19,6 +19,11 @@ module ActionDispatch end end + def setup + @controller = nil + @request = nil + end + def test_assert_response_predicate_methods [:success, :missing, :redirect, :error].each do |sym| @response = FakeResponse.new RESPONSE_PREDICATES[sym].to_s.sub(/\?/, '').to_sym diff --git a/actionpack/test/controller/helper_test.rb b/actionpack/test/controller/helper_test.rb index 3ecfedefd1..feb882a2b3 100644 --- a/actionpack/test/controller/helper_test.rb +++ b/actionpack/test/controller/helper_test.rb @@ -141,20 +141,10 @@ class HelperTest < ActiveSupport::TestCase def test_helper_for_nested_controller assert_equal 'hello: Iz guuut!', call_controller(Fun::GamesController, "render_hello_world").last.body - # request = ActionController::TestRequest.new - # - # resp = Fun::GamesController.action(:render_hello_world).call(request.env) - # assert_equal 'hello: Iz guuut!', resp.last.body end def test_helper_for_acronym_controller assert_equal "test: baz", call_controller(Fun::PdfController, "test").last.body - # - # request = ActionController::TestRequest.new - # response = ActionDispatch::TestResponse.new - # request.action = 'test' - # - # assert_equal 'test: baz', Fun::PdfController.process(request, response).body end def test_default_helpers_only diff --git a/actionpack/test/controller/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb index 9c5a01c318..98e3c891a7 100644 --- a/actionpack/test/controller/http_token_authentication_test.rb +++ b/actionpack/test/controller/http_token_authentication_test.rb @@ -94,6 +94,14 @@ class HttpTokenAuthenticationTest < ActionController::TestCase assert_response :success end + test "authentication request with tab in header" do + @request.env['HTTP_AUTHORIZATION'] = "Token\ttoken=\"lifo\"" + get :index + + assert_response :success + assert_equal 'Hello Secret', @response.body + end + test "authentication request without credential" do get :display diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 87816515e7..f23aa599c1 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -297,4 +297,32 @@ class ParametersPermitTest < ActiveSupport::TestCase assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess assert_not @params.to_h.is_a? ActionController::Parameters end + + test "to_h only deep dups Ruby collections" do + company = Class.new do + attr_reader :dupped + def dup; @dupped = true; end + end.new + + params = ActionController::Parameters.new(prem: { likes: %i( dancing ) }) + assert_equal({ 'prem' => { 'likes' => %i( dancing ) } }, params.permit!.to_h) + + params = ActionController::Parameters.new(companies: [ company, :acme ]) + assert_equal({ 'companies' => [ company, :acme ] }, params.permit!.to_h) + assert_not company.dupped + end + + test "to_unsafe_h only deep dups Ruby collections" do + company = Class.new do + attr_reader :dupped + def dup; @dupped = true; end + end.new + + params = ActionController::Parameters.new(prem: { likes: %i( dancing ) }) + assert_equal({ 'prem' => { 'likes' => %i( dancing ) } }, params.to_unsafe_h) + + params = ActionController::Parameters.new(companies: [ company, :acme ]) + assert_equal({ 'companies' => [ company, :acme ] }, params.to_unsafe_h) + assert_not company.dupped + end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 631ff7d02a..0b184eace9 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -42,6 +42,10 @@ class RedirectController < ActionController::Base redirect_to :back, :status => 307 end + def redirect_back_with_status + redirect_back(fallback_location: "/things/stuff", status: 307) + end + def host_redirect redirect_to :action => "other_host", :only_path => false, :host => 'other.test.host' end @@ -187,7 +191,11 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back_with_status @request.env["HTTP_REFERER"] = "http://www.example.com/coming/from" - get :redirect_to_back_with_status + + assert_deprecated do + get :redirect_to_back_with_status + end + assert_response 307 assert_equal "http://www.example.com/coming/from", redirect_to_url end @@ -236,7 +244,11 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back @request.env["HTTP_REFERER"] = "http://www.example.com/coming/from" - get :redirect_to_back + + assert_deprecated do + get :redirect_to_back + end + assert_response :redirect assert_equal "http://www.example.com/coming/from", redirect_to_url end @@ -244,10 +256,32 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back_with_no_referer assert_raise(ActionController::RedirectBackError) { @request.env["HTTP_REFERER"] = nil + + assert_deprecated do + get :redirect_to_back + end + get :redirect_to_back } end + def test_redirect_back + referer = "http://www.example.com/coming/from" + @request.env["HTTP_REFERER"] = referer + + get :redirect_back_with_status + + assert_response 307 + assert_equal referer, redirect_to_url + end + + def test_redirect_back_with_no_referer + get :redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do @@ -273,10 +307,10 @@ class RedirectTest < ActionController::TestCase end def test_redirect_to_params - error = assert_raise(ActionController::ActionControllerError) do + error = assert_raise(ArgumentError) do get :redirect_to_params end - assert_equal "Cannot redirect to a parameter hash!", error.message + assert_equal "Generating an URL from non sanitized request parameters is insecure!", error.message end def test_redirect_to_with_block diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index e50373a0cc..b9caddcdb7 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -172,7 +172,7 @@ XML before_action { @dynamic_opt = 'opt' } def test_url_options_reset - render plain: url_for(params) + render plain: url_for end def default_url_options diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 78e883f134..67212fea38 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -375,6 +375,13 @@ module AbstractController assert_equal({'query[person][position][]' => 'prof' }.to_query, params[3]) end + def test_url_action_controller_parameters + add_host! + assert_raise(ArgumentError) do + W.new.url_for(ActionController::Parameters.new(:controller => 'c', :action => 'a', protocol: 'javascript', f: '%0Aeval(name)')) + end + end + def test_path_generation_for_symbol_parameter_keys assert_generates("/image", :controller=> :image) end |