diff options
author | Eileen M. Uchitelle <eileencodes@users.noreply.github.com> | 2019-04-03 13:21:51 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-04-03 13:21:51 -0400 |
commit | d39b2b684e94e856a4a1257984ff6a1d5f978a2c (patch) | |
tree | 924b846a52d8501a7a78cd84cde0dd411dbcd498 | |
parent | f960920a8e8501cb5af2504166fe45b867705b88 (diff) | |
parent | eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0 (diff) | |
download | rails-d39b2b684e94e856a4a1257984ff6a1d5f978a2c.tar.gz rails-d39b2b684e94e856a4a1257984ff6a1d5f978a2c.tar.bz2 rails-d39b2b684e94e856a4a1257984ff6a1d5f978a2c.zip |
Merge pull request #35825 from jhawthorn/always_filter_view_paths
Make Resolver#find_all_anywhere equivalent to #find_all
-rw-r--r-- | actionpack/test/controller/new_base/render_file_test.rb | 4 | ||||
-rw-r--r-- | actionpack/test/controller/render_test.rb | 17 | ||||
-rw-r--r-- | actionview/lib/action_view/lookup_context.rb | 5 | ||||
-rw-r--r-- | actionview/lib/action_view/path_set.rb | 15 | ||||
-rw-r--r-- | actionview/lib/action_view/renderer/template_renderer.rb | 2 | ||||
-rw-r--r-- | actionview/lib/action_view/template/resolver.rb | 29 | ||||
-rw-r--r-- | actionview/lib/action_view/testing/resolvers.rb | 4 | ||||
-rw-r--r-- | actionview/test/template/fallback_file_system_resolver_test.rb | 2 | ||||
-rw-r--r-- | actionview/test/template/lookup_context_test.rb | 8 |
9 files changed, 42 insertions, 44 deletions
diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb index 82325c5bb2..01d0223519 100644 --- a/actionpack/test/controller/new_base/render_file_test.rb +++ b/actionpack/test/controller/new_base/render_file_test.rb @@ -17,12 +17,12 @@ module RenderFile def relative_path @secret = "in the sauce" - render file: "../../fixtures/test/render_file_with_ivar" + render file: "../actionpack/test/fixtures/test/render_file_with_ivar" end def relative_path_with_dot @secret = "in the sauce" - render file: "../../fixtures/test/dot.directory/render_file_with_ivar" + render file: "../actionpack/test/fixtures/test/dot.directory/render_file_with_ivar" end def pathname diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 6d198ca42f..8bb6617eaa 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -323,11 +323,12 @@ class ExpiresInRenderTest < ActionController::TestCase end def test_dynamic_render_with_file - # This is extremely bad, but should be possible to do. assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__)) - response = assert_deprecated { get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' } } - assert_equal File.read(File.expand_path("../../test/abstract_unit.rb", __dir__)), - response.body + assert_deprecated do + assert_raises ActionView::MissingTemplate do + get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' } + end + end end def test_dynamic_render_with_absolute_path @@ -351,9 +352,11 @@ class ExpiresInRenderTest < ActionController::TestCase def test_permitted_dynamic_render_file_hash assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__)) - response = assert_deprecated { get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } } - assert_equal File.read(File.expand_path("../../test/abstract_unit.rb", __dir__)), - response.body + assert_deprecated do + assert_raises ActionView::MissingTemplate do + get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } + end + end end def test_dynamic_render_file_hash diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index fd3d025cbf..b9a8cc129c 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -130,9 +130,8 @@ module ActionView end alias :find_template :find - def find_file(name, prefixes = [], partial = false, keys = [], options = {}) - @view_paths.find_file(*args_for_lookup(name, prefixes, partial, keys, options)) - end + alias :find_file :find + deprecate :find_file def find_all(name, prefixes = [], partial = false, keys = [], options = {}) @view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys, options)) diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb index 691b53e2da..d54749d8e3 100644 --- a/actionview/lib/action_view/path_set.rb +++ b/actionview/lib/action_view/path_set.rb @@ -48,12 +48,11 @@ module ActionView #:nodoc: find_all(*args).first || raise(MissingTemplate.new(self, *args)) end - def find_file(path, prefixes = [], *args) - _find_all(path, prefixes, args, true).first || raise(MissingTemplate.new(self, path, prefixes, *args)) - end + alias :find_file :find + deprecate :find_file def find_all(path, prefixes = [], *args) - _find_all path, prefixes, args, false + _find_all path, prefixes, args end def exists?(path, prefixes, *args) @@ -71,15 +70,11 @@ module ActionView #:nodoc: private - def _find_all(path, prefixes, args, outside_app) + def _find_all(path, prefixes, args) prefixes = [prefixes] if String === prefixes prefixes.each do |prefix| paths.each do |resolver| - if outside_app - templates = resolver.find_all_anywhere(path, prefix, *args) - else - templates = resolver.find_all(path, prefix, *args) - end + templates = resolver.find_all(path, prefix, *args) return templates unless templates.empty? end end diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index 1faef9ca81..4aab3f913f 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -30,7 +30,7 @@ module ActionView Template::RawFile.new(options[:file]) else ActiveSupport::Deprecation.warn "render file: should be given the absolute path to a file" - @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details) + @lookup_context.with_fallbacks.find_template(options[:file], nil, false, keys, @details) end elsif options.key?(:inline) handler = Template.handler_for_extension(options[:type] || "erb") diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 095e6cc3a1..d5cb3c823a 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -118,17 +118,12 @@ module ActionView locals = locals.map(&:to_s).sort!.freeze cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, false, locals) + find_templates(name, prefix, partial, details, locals) end end - def find_all_anywhere(name, prefix, partial = false, details = {}, key = nil, locals = []) - locals = locals.map(&:to_s).sort!.freeze - - cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, true, locals) - end - end + alias :find_all_anywhere :find_all + deprecate :find_all_anywhere def find_all_with_query(query) # :nodoc: @cache.cache_query(query) { find_template_paths(File.join(@path, query)) } @@ -141,8 +136,8 @@ module ActionView # This is what child classes implement. No defaults are needed # because Resolver guarantees that the arguments are present and # normalized. - def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) - raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) method" + def find_templates(name, prefix, partial, details, locals = []) + raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, locals = []) method" end # Handles templates caching. If a key is given and caching is on @@ -179,14 +174,14 @@ module ActionView private - def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals) + def find_templates(name, prefix, partial, details, locals) path = Path.build(name, prefix, partial) - query(path, details, details[:formats], outside_app_allowed, locals) + query(path, details, details[:formats], locals) end - def query(path, details, formats, outside_app_allowed, locals) + def query(path, details, formats, locals) template_paths = find_template_paths_from_details(path, details) - template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed + template_paths = reject_files_external_to_app(template_paths) template_paths.map do |template| build_template(template, path.virtual, locals) @@ -360,6 +355,8 @@ module ActionView # The same as FileSystemResolver but does not allow templates to store # a virtual path since it is invalid for such resolvers. class FallbackFileSystemResolver < FileSystemResolver #:nodoc: + private_class_method :new + def self.instances [new(""), new("/")] end @@ -367,5 +364,9 @@ module ActionView def build_template(template, virtual_path, locals) super(template, nil, locals) end + + def reject_files_external_to_app(files) + files + end end end diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index a16dc0096e..11ae9f731c 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -23,7 +23,7 @@ module ActionView #:nodoc: private - def query(path, exts, _, _, locals) + def query(path, exts, _, locals) query = +"" EXTENSIONS.each_key do |ext| query << "(" << exts[ext].map { |e| e && Regexp.escape(".#{e}") }.join("|") << "|)" @@ -47,7 +47,7 @@ module ActionView #:nodoc: end class NullResolver < PathResolver - def query(path, exts, _, _, locals) + def query(path, exts, _, locals) handler, format, variant = extract_handler_and_format_and_variant(path) [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals)] end diff --git a/actionview/test/template/fallback_file_system_resolver_test.rb b/actionview/test/template/fallback_file_system_resolver_test.rb index 304cdb8a03..fa770f3a15 100644 --- a/actionview/test/template/fallback_file_system_resolver_test.rb +++ b/actionview/test/template/fallback_file_system_resolver_test.rb @@ -4,7 +4,7 @@ require "abstract_unit" class FallbackFileSystemResolverTest < ActiveSupport::TestCase def setup - @root_resolver = ActionView::FallbackFileSystemResolver.new("/") + @root_resolver = ActionView::FallbackFileSystemResolver.send(:new, "/") end def test_should_have_no_virtual_path diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 3e357fe1a7..72e1f50fdf 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -143,16 +143,16 @@ class LookupContextTest < ActiveSupport::TestCase assert_deprecated do @lookup_context.with_fallbacks do assert_equal 3, @lookup_context.view_paths.size - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[0] + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[1] end end @lookup_context = @lookup_context.with_fallbacks assert_equal 3, @lookup_context.view_paths.size - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[0] + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[1] end test "add fallbacks just once in nested fallbacks calls" do |