diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | actionpack/lib/abstract_controller/railties/routes_helpers.rb | 7 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/filter_parameters.rb | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/parameter_filter.rb | 85 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 21 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/testing/assertions/routing.rb | 37 | ||||
| -rw-r--r-- | actionpack/test/abstract_unit.rb | 5 | ||||
| -rw-r--r-- | actionpack/test/controller/integration_test.rb | 7 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request_test.rb | 44 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing/ipv6_redirect_test.rb | 10 | ||||
| -rw-r--r-- | actionpack/test/dispatch/url_generation_test.rb | 9 |
12 files changed, 48 insertions, 192 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c972c64766..8205d79dd2 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Deprecate `ActionDispatch::Http::ParameterFilter` in favor of `ActiveSupport::ParameterFilter`. + + *Yoshiyuki Kinjo* + * Remove undocumented `params` option from `url_for` helper. *Ilkka Oksanen* diff --git a/actionpack/lib/abstract_controller/railties/routes_helpers.rb b/actionpack/lib/abstract_controller/railties/routes_helpers.rb index c97be074c8..fbd93705ed 100644 --- a/actionpack/lib/abstract_controller/railties/routes_helpers.rb +++ b/actionpack/lib/abstract_controller/railties/routes_helpers.rb @@ -7,8 +7,11 @@ module AbstractController Module.new do define_method(:inherited) do |klass| super(klass) - - routes.include_helpers klass, include_path_helpers + if namespace = klass.module_parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) } + klass.include(namespace.railtie_routes_url_helpers(include_path_helpers)) + else + klass.include(routes.url_helpers(include_path_helpers)) + end end end end diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index ec012ad02d..9f8f178402 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "action_dispatch/http/parameter_filter" +require "active_support/parameter_filter" module ActionDispatch module Http @@ -28,8 +28,8 @@ module ActionDispatch # => reverses the value to all keys matching /secret/i module FilterParameters ENV_MATCH = [/RAW_POST_DATA/, "rack.request.form_vars"] # :nodoc: - NULL_PARAM_FILTER = ParameterFilter.new # :nodoc: - NULL_ENV_FILTER = ParameterFilter.new ENV_MATCH # :nodoc: + NULL_PARAM_FILTER = ActiveSupport::ParameterFilter.new # :nodoc: + NULL_ENV_FILTER = ActiveSupport::ParameterFilter.new ENV_MATCH # :nodoc: def initialize super @@ -69,7 +69,7 @@ module ActionDispatch end def parameter_filter_for(filters) # :doc: - ParameterFilter.new(filters) + ActiveSupport::ParameterFilter.new(filters) end KV_RE = "[^&;=]+" diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index 6689092859..ddeb3d81e2 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -1,87 +1,12 @@ # frozen_string_literal: true -require "active_support/core_ext/object/duplicable" -require "active_support/core_ext/array/extract" +require "active_support/deprecation/constant_accessor" +require "active_support/parameter_filter" module ActionDispatch module Http - class ParameterFilter - FILTERED = "[FILTERED]" # :nodoc: - - def initialize(filters = []) - @filters = filters - end - - def filter(params) - compiled_filter.call(params) - end - - private - - def compiled_filter - @compiled_filter ||= CompiledFilter.compile(@filters) - end - - class CompiledFilter # :nodoc: - def self.compile(filters) - return lambda { |params| params.dup } if filters.empty? - - strings, regexps, blocks = [], [], [] - - filters.each do |item| - case item - when Proc - blocks << item - when Regexp - regexps << item - else - strings << Regexp.escape(item.to_s) - end - end - - deep_regexps = regexps.extract! { |r| r.to_s.include?("\\.") } - deep_strings = strings.extract! { |s| s.include?("\\.") } - - regexps << Regexp.new(strings.join("|"), true) unless strings.empty? - deep_regexps << Regexp.new(deep_strings.join("|"), true) unless deep_strings.empty? - - new regexps, deep_regexps, blocks - end - - attr_reader :regexps, :deep_regexps, :blocks - - def initialize(regexps, deep_regexps, blocks) - @regexps = regexps - @deep_regexps = deep_regexps.any? ? deep_regexps : nil - @blocks = blocks - end - - def call(params, parents = [], original_params = params) - filtered_params = params.class.new - - params.each do |key, value| - parents.push(key) if deep_regexps - if regexps.any? { |r| key =~ r } - value = FILTERED - elsif deep_regexps && (joined = parents.join(".")) && deep_regexps.any? { |r| joined =~ r } - value = FILTERED - elsif value.is_a?(Hash) - value = call(value, parents, original_params) - elsif value.is_a?(Array) - value = value.map { |v| v.is_a?(Hash) ? call(v, parents, original_params) : v } - elsif blocks.any? - key = key.dup if key.duplicable? - value = value.dup if value.duplicable? - blocks.each { |b| b.arity == 2 ? b.call(key, value) : b.call(key, value, original_params) } - end - parents.pop if deep_regexps - - filtered_params[key] = value - end - - filtered_params - end - end - end + include ActiveSupport::Deprecation::DeprecatedConstantAccessor + deprecate_constant "ParameterFilter", "ActiveSupport::ParameterFilter", + message: "ActionDispatch::Http::ParameterFilter is deprecated and will be removed from Rails 6.1. Use ActiveSupport::ParameterFilter instead." end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 2f68cefa94..99f3b4c2cd 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -676,6 +676,7 @@ module ActionDispatch def define_generate_prefix(app, name) _route = @set.named_routes.get name _routes = @set + _url_helpers = @set.url_helpers script_namer = ->(options) do prefix_options = options.slice(*_route.segment_keys) @@ -687,7 +688,7 @@ module ActionDispatch # We must actually delete prefix segment keys to avoid passing them to next url_for. _route.segment_keys.each { |k| options.delete(k) } - @set.url_helpers.send("#{name}_path", prefix_options) + _url_helpers.send("#{name}_path", prefix_options) end app.routes.define_mounted_helper(name, script_namer) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 2c811b5e55..fedf622e90 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -458,11 +458,6 @@ module ActionDispatch return if @finalized @append.each { |blk| eval_block(blk) } @finalized = true - @url_helpers = build_url_helper_module true - @deferred_classes.each { |klass, include_path_helpers| - include_helpers klass, include_path_helpers - } - @deferred_classes.clear end def clear! @@ -491,10 +486,11 @@ module ActionDispatch return if MountedHelpers.method_defined?(name) routes = self + helpers = routes.url_helpers MountedHelpers.class_eval do define_method "_#{name}" do - RoutesProxy.new(routes, _routes_context, routes.url_helpers, script_namer) + RoutesProxy.new(routes, _routes_context, helpers, script_namer) end end @@ -505,20 +501,7 @@ module ActionDispatch RUBY end - class UnfinalizedRouteSet < StandardError - end - def url_helpers(supports_path = true) - raise UnfinalizedRouteSet, "routes have not been finalized. Please call `finalize!` or use `draw(&block)`" unless @finalized - - if supports_path - @url_helpers - else - build_url_helper_module false - end - end - - def build_url_helper_module(supports_path) routes = self Module.new do diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 0e8712f8d9..af41521c5c 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -138,20 +138,6 @@ module ActionDispatch assert_generates(path.is_a?(Hash) ? path[:path] : path, generate_options, defaults, extras, message) end - # Provides a hook on `finalize!` so we can mutate a controller after the - # route set has been drawn. - class WithRouting < ActionDispatch::Routing::RouteSet # :nodoc: - def initialize(&block) - super() - @block = block - end - - def finalize! - super - @block.call self - end - end - # A helper to make it easier to test different route configurations. # This method temporarily replaces @routes with a new RouteSet instance. # @@ -166,19 +152,16 @@ module ActionDispatch # end # def with_routing - old_routes = @routes - old_controller = nil - @routes = WithRouting.new do |_routes| - if defined?(@controller) && @controller - old_controller, @controller = @controller, @controller.clone - _routes = @routes - - @controller.singleton_class.include(_routes.url_helpers) - - if @controller.respond_to? :view_context_class - @controller.view_context_class = Class.new(@controller.view_context_class) do - include _routes.url_helpers - end + old_routes, @routes = @routes, ActionDispatch::Routing::RouteSet.new + if defined?(@controller) && @controller + old_controller, @controller = @controller, @controller.clone + _routes = @routes + + @controller.singleton_class.include(_routes.url_helpers) + + if @controller.respond_to? :view_context_class + @controller.view_context_class = Class.new(@controller.view_context_class) do + include _routes.url_helpers end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 7c9f15108d..65dd28b3d7 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -100,10 +100,7 @@ end class ActionDispatch::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) - routes ||= ActionDispatch::Routing::RouteSet.new.tap { |rs| - rs.draw { } - } - RoutedRackApp.new(routes) do |middleware| + RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| middleware.use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") middleware.use ActionDispatch::DebugExceptions middleware.use ActionDispatch::Callbacks diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index b078e8ad9f..39ede1442a 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -542,6 +542,9 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest def with_test_route_set with_routing do |set| controller = ::IntegrationProcessTest::IntegrationController.clone + controller.class_eval do + include set.url_helpers + end set.draw do get "moved" => redirect("/method") @@ -552,10 +555,6 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end - controller.class_eval do - include set.url_helpers - end - singleton_class.include(set.url_helpers) yield diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index c7b68e5266..9d1246b3a4 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -1059,47 +1059,9 @@ class RequestParameters < BaseRequestTest end class RequestParameterFilter < BaseRequestTest - test "process parameter filter" do - test_hashes = [ - [{ "foo" => "bar" }, { "foo" => "bar" }, %w'food'], - [{ "foo" => "bar" }, { "foo" => "[FILTERED]" }, %w'foo'], - [{ "foo" => "bar", "bar" => "foo" }, { "foo" => "[FILTERED]", "bar" => "foo" }, %w'foo baz'], - [{ "foo" => "bar", "baz" => "foo" }, { "foo" => "[FILTERED]", "baz" => "[FILTERED]" }, %w'foo baz'], - [{ "bar" => { "foo" => "bar", "bar" => "foo" } }, { "bar" => { "foo" => "[FILTERED]", "bar" => "foo" } }, %w'fo'], - [{ "foo" => { "foo" => "bar", "bar" => "foo" } }, { "foo" => "[FILTERED]" }, %w'f banana'], - [{ "deep" => { "cc" => { "code" => "bar", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, { "deep" => { "cc" => { "code" => "[FILTERED]", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, %w'deep.cc.code'], - [{ "baz" => [{ "foo" => "baz" }, "1"] }, { "baz" => [{ "foo" => "[FILTERED]" }, "1"] }, [/foo/]]] - - test_hashes.each do |before_filter, after_filter, filter_words| - parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words) - assert_equal after_filter, parameter_filter.filter(before_filter) - - filter_words << "blah" - filter_words << lambda { |key, value| - value.reverse! if key =~ /bargain/ - } - filter_words << lambda { |key, value, original_params| - value.replace("world!") if original_params["barg"]["blah"] == "bar" && key == "hello" - } - - parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words) - before_filter["barg"] = { :bargain => "gain", "blah" => "bar", "bar" => { "bargain" => { "blah" => "foo", "hello" => "world" } } } - after_filter["barg"] = { :bargain => "niag", "blah" => "[FILTERED]", "bar" => { "bargain" => { "blah" => "[FILTERED]", "hello" => "world!" } } } - - assert_equal after_filter, parameter_filter.filter(before_filter) - end - end - - test "parameter filter should maintain hash with indifferent access" do - test_hashes = [ - [{ "foo" => "bar" }.with_indifferent_access, ["blah"]], - [{ "foo" => "bar" }.with_indifferent_access, []] - ] - - test_hashes.each do |before_filter, filter_words| - parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words) - assert_instance_of ActiveSupport::HashWithIndifferentAccess, - parameter_filter.filter(before_filter) + test "parameter filter is deprecated" do + assert_deprecated do + ActionDispatch::Http::ParameterFilter.new(["blah"]) end end diff --git a/actionpack/test/dispatch/routing/ipv6_redirect_test.rb b/actionpack/test/dispatch/routing/ipv6_redirect_test.rb index fb423d2951..31559bffc7 100644 --- a/actionpack/test/dispatch/routing/ipv6_redirect_test.rb +++ b/actionpack/test/dispatch/routing/ipv6_redirect_test.rb @@ -4,11 +4,6 @@ require "abstract_unit" class IPv6IntegrationTest < ActionDispatch::IntegrationTest Routes = ActionDispatch::Routing::RouteSet.new - Routes.draw do - get "/", to: "bad_route_request#index", as: :index - get "/foo", to: "bad_route_request#foo", as: :foo - end - include Routes.url_helpers class ::BadRouteRequestController < ActionController::Base @@ -22,6 +17,11 @@ class IPv6IntegrationTest < ActionDispatch::IntegrationTest end end + Routes.draw do + get "/", to: "bad_route_request#index", as: :index + get "/foo", to: "bad_route_request#foo", as: :foo + end + def _routes Routes end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index b0096d26be..aef9351de1 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -4,13 +4,16 @@ require "abstract_unit" module TestUrlGeneration class WithMountPoint < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new + include Routes.url_helpers + class ::MyRouteGeneratingController < ActionController::Base + include Routes.url_helpers def index render plain: foo_path end end - Routes = ActionDispatch::Routing::RouteSet.new Routes.draw do get "/foo", to: "my_route_generating#index", as: :foo @@ -19,10 +22,6 @@ module TestUrlGeneration mount MyRouteGeneratingController.action(:index), at: "/bar" end - class ::MyRouteGeneratingController - include Routes.url_helpers - end - APP = build_app Routes def _routes |