Introduce Template::File as new render file:

The previous behaviour of render file: was essentially the same as
render template:, except that templates can be specified as an absolute
path on the filesystem.

This makes sense for historic reasons, but now render file: is almost
exclusively used to render raw files (not .erb) like public/404.html. In
addition to complicating the code in template/resolver.rb, I think the
current behaviour is surprising to developers.

This commit deprecates the existing "lookup a template from anywhere"
behaviour and replaces it with "render this file exactly as it is on
disk". Handlers will no longer be used (it will render the same as if
the :raw handler was used), but formats (.html, .xml, etc) will still be
detected (and will default to :plain).

The existing render file: behaviour was the path through which Rails
apps were vulnerable in the recent CVE-2019-5418. Although the
vulnerability has been patched in a fully backwards-compatible way, I
think it's a strong hint that we should drop the existing
previously-vulnerable behaviour if it isn't a benefit to developers.

2 files changed, 35 insertions, 11 deletions

diff --git a/actionview/test/actionpack/abstract/render_test.rb b/actionview/test/actionpack/abstract/render_test.rb
index d863548a5c..e4e8ac93b2 100644
--- a/actionview/test/actionpack/abstract/render_test.rb
+++ b/actionview/test/actionpack/abstract/render_test.rb
@@ -26,7 +26,7 @@ module AbstractController
       end
 
       def file
-        render file: "some/file"
+        ActiveSupport::Deprecation.silence { render file: "some/file" }
       end
 
       def inline
diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb
index 52c3c54d96..c8ce7366d1 100644
--- a/actionview/test/actionpack/controller/render_test.rb
+++ b/actionview/test/actionpack/controller/render_test.rb
@@ -872,48 +872,64 @@ class RenderTest < ActionController::TestCase
 
   # :ported:
   def test_render_file_with_instance_variables
-    get :render_file_with_instance_variables
+    assert_deprecated do
+      get :render_file_with_instance_variables
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   def test_render_file
-    get :hello_world_file
+    assert_deprecated do
+      get :hello_world_file
+    end
     assert_equal "Hello world!", @response.body
   end
 
   # :ported:
   def test_render_file_not_using_full_path
-    get :render_file_not_using_full_path
+    assert_deprecated do
+      get :render_file_not_using_full_path
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_not_using_full_path_with_dot_in_path
-    get :render_file_not_using_full_path_with_dot_in_path
+    assert_deprecated do
+      get :render_file_not_using_full_path_with_dot_in_path
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_using_pathname
-    get :render_file_using_pathname
+    assert_deprecated do
+      get :render_file_using_pathname
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_with_locals
-    get :render_file_with_locals
+    assert_deprecated do
+      get :render_file_with_locals
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_as_string_with_locals
-    get :render_file_as_string_with_locals
+    assert_deprecated do
+      get :render_file_as_string_with_locals
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :assessed:
   def test_render_file_from_template
-    get :render_file_from_template
+    assert_deprecated do
+      get :render_file_from_template
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
@@ -1133,11 +1149,19 @@ class RenderTest < ActionController::TestCase
   end
 
   def test_bad_render_to_string_still_throws_exception
-    assert_raise(ActionView::MissingTemplate) { get :render_to_string_with_exception }
+    assert_deprecated do
+      assert_raise(ActionView::MissingTemplate) do
+        get :render_to_string_with_exception
+      end
+    end
   end
 
   def test_render_to_string_that_throws_caught_exception_doesnt_break_assigns
-    assert_nothing_raised { get :render_to_string_with_caught_exception }
+    assert_deprecated do
+      assert_nothing_raised do
+        get :render_to_string_with_caught_exception
+      end
+    end
     assert_equal "i'm before the render", @controller.instance_variable_get(:@before)
     assert_equal "i'm after the render", @controller.instance_variable_get(:@after)
   end