TEST_CODEOWNERS/rails
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Prevent assigning internal ivars to AV::Base

Browse Source 
Previously, both the `@rendered_format` and
`@marked_for_same_origin_verification` instance variables would be
assigned to instances of `ActionView::Base`, making them accessible in
view templates. However, these instance variables are really internal to
the controller and result in extra string allocations because the `@`
gets stripped and readded when going through the assignment.

This commit prefixes the variables with an underscore to help indicate
that they are internal, and then adds them to the list of
`_protected_ivars` to prevent assigning them when rendering templates.

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
This commit is contained in:
Hartley McGuire 2023-10-30 20:19:07 -04:00
parent 89b0a8cb24
commit 24213d6954
No known key found for this signature in database
GPG Key ID: E823FC1403858A82
4 changed files with 35 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
actionpack/lib/action_controller/base.rb
View File

@ -247,6 +247,7 @@ def self.without_modules(*modules)
PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + %i(
@_params @_response @_request @_config @_url_options @_action_has_layout @_view_context_class
@_view_renderer @_lookup_context @_routes @_view_runtime @_db_runtime @_helper_proxy
@_marked_for_same_origin_verification @_rendered_format
)
def _protected_ivars

6
actionpack/lib/action_controller/metal/request_forgery_protection.rb
View File

@ -340,7 +340,7 @@ def reset(request)
def initialize(...)
super
@marked_for_same_origin_verification = nil
@_marked_for_same_origin_verification = nil
end
def reset_csrf_token(request) # :doc:
@ -414,13 +414,13 @@ def verify_same_origin_request # :doc:
# GET requests are checked for cross-origin JavaScript after rendering.
def mark_for_same_origin_verification! # :doc:
@marked_for_same_origin_verification = request.get?
@_marked_for_same_origin_verification = request.get?
end
# If the +verify_authenticity_token+ before_action ran, verify that
# JavaScript responses are only served to same-origin GET requests.
def marked_for_same_origin_verification? # :doc:
@marked_for_same_origin_verification ||= false
@_marked_for_same_origin_verification ||= false
end
# Check for cross-origin JavaScript responses.

6
actionview/lib/action_view/rendering.rb
View File

@ -27,10 +27,10 @@ module Rendering
extend ActiveSupport::Concern
include ActionView::ViewPaths
attr_reader :rendered_format
attr_internal_reader :rendered_format
def initialize
@rendered_format = nil
@_rendered_format = nil
super
end
@ -136,7 +136,7 @@ def _render_template(options)
end
rendered_format = rendered_template.format || lookup_context.formats.first
@rendered_format = Template::Types[rendered_format]
@_rendered_format = Template::Types[rendered_format]
rendered_template.body
end

28
actionview/test/actionpack/controller/render_test.rb
View File

@ -126,6 +126,10 @@ def hello_world_with_layout_false
render layout: false
end
def render_instance_variables
render inline: "<%= instance_variables.sort %>"
end
# :ported:
def render_template_with_instance_variables
@secret = "in the sauce"
@ -721,6 +725,7 @@ class RenderTest < ActionController::TestCase
get :render_hello_world_with_forward_slash, to: "test#render_hello_world_with_forward_slash"
get :render_implicit_html_template_from_xhr_request, to: "test#render_implicit_html_template_from_xhr_request"
get :render_implicit_js_template_without_layout, to: "test#render_implicit_js_template_without_layout"
get :render_instance_variables, to: "test#render_instance_variables"
get :render_line_offset, to: "test#render_line_offset"
get :render_nothing_with_appendix, to: "test#render_nothing_with_appendix"
get :render_template_in_top_directory, to: "test#render_template_in_top_directory"
@ -785,6 +790,29 @@ def test_simple_show
assert_equal "<html>Hello world!</html>", @response.body
end
def test_controller_does_not_leak_instance_variables
expected = [
:@_assigns, # attr_internal on ActionView::Base
:@_config, # attr_internal on ActionView::Base
:@_controller, # attr_internal on ActionView::Helpers::ControllerHelper
:@_default_form_builder, # attr_internal on ActionView::Helpers::FormHelper
:@_ivars, # ActionController::Testing::Functional (only appears inside an ActionController::TestCase)
:@_request, # attr_internal on ActionView::Helpers::ControllerHelper
:@current_template, # instance variable on ActionView::Base
:@lookup_context, # attr_reader on ActionView::Base
:@output_buffer, # attr_accessor on ActionView::Base::Context
:@variable_for_layout, # part of this test class
:@view_flow, # attr_accessor on ActionView::Base::Context
:@view_renderer, # attr_reader on ActionView::Base
:@virtual_path, # instance variable on ActionView::Base
].inspect
get :render_instance_variables
assert_response 200
assert_equal expected, @response.body
end
# :ported:
def test_renders_default_template_for_missing_action
get :'hyphen-ated'