Merge pull request #35825 from jhawthorn/always_filter_view_paths

Make Resolver#find_all_anywhere equivalent to #find_all

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

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

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))

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

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")

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

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

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

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