From 87b6e6aa4328f16edd68978079f473169cceecbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ana=20Mar=C3=ADa=20Mart=C3=ADnez=20G=C3=B3mez?= Date: Tue, 7 Aug 2018 17:23:57 +0200 Subject: Use public_send in value_for_collection Avoid exposing private methods in view's helpers. Fixes https://github.com/rails/rails/issues/33546 --- actionview/CHANGELOG.md | 10 ++++++++++ actionview/lib/action_view/helpers/form_options_helper.rb | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'actionview') diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 6d45cc1d8a..8597fea48d 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,13 @@ +* Stop exposing public methods in view's helpers. + + For example, in methods like `options_from_collection_for_select`, + it was possible to call private methods from the objects used. + + See [#33546](https://github.com/rails/rails/issues/33546) for details. + + *[Ana María Martínez Gómez](https://github.com/Ana06)* + + * Fix issue with `button_to`'s `to_form_params` `button_to` was throwing exception when invoked with `params` hash that diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 7884a8d997..9c0238a01a 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -802,7 +802,7 @@ module ActionView end def value_for_collection(item, value) - value.respond_to?(:call) ? value.call(item) : item.send(value) + value.respond_to?(:call) ? value.call(item) : item.public_send(value) end def prompt_text(prompt) -- cgit v1.2.3 From a9764dcc07cf9d6280b313da734d98e096b7d122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ana=20Mar=C3=ADa=20Mart=C3=ADnez=20G=C3=B3mez?= Date: Wed, 8 Aug 2018 10:08:43 +0200 Subject: Use public_send in extract_values_from_collection Avoid exposing private methods in view's helpers. However, as `extract_values_from_collection` is only called from `options_from_collection_for_select` where `value_for_collection` is previously called, this case was already covered. The change makes anyway sense for consistency and in case the code changes in the future. --- actionview/lib/action_view/helpers/form_options_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionview') diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 9c0238a01a..0fd68b66d4 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -794,7 +794,7 @@ module ActionView def extract_values_from_collection(collection, value_method, selected) if selected.is_a?(Proc) collection.map do |element| - element.send(value_method) if selected.call(element) + element.public_send(value_method) if selected.call(element) end.compact else selected -- cgit v1.2.3 From 0c62e141a3fc9b2d00935bb99d2d5f465e1a4fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ana=20Mar=C3=ADa=20Mart=C3=ADnez=20G=C3=B3mez?= Date: Wed, 8 Aug 2018 10:20:46 +0200 Subject: Add one more method affected in CHANGELOG --- actionview/CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'actionview') diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 8597fea48d..9fc120acbc 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,7 +1,8 @@ * Stop exposing public methods in view's helpers. - For example, in methods like `options_from_collection_for_select`, - it was possible to call private methods from the objects used. + For example, in methods like `options_from_collection_for_select` + and `collection_select` it was possible to call private methods from + the objects used. See [#33546](https://github.com/rails/rails/issues/33546) for details. -- cgit v1.2.3 From 4ca9fa11f9a9b5604371f260515c28a0f29cd921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ana=20Mar=C3=ADa=20Mart=C3=ADnez=20G=C3=B3mez?= Date: Wed, 8 Aug 2018 10:35:03 +0200 Subject: Deprecate use of private methods in view's helpers Instead of dropping it completely in case someone is relying (probably inadvertenly) on it. --- actionview/CHANGELOG.md | 4 ++-- actionview/lib/action_view/helpers/form_options_helper.rb | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) (limited to 'actionview') diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 9fc120acbc..4c552c635a 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,7 +1,7 @@ -* Stop exposing public methods in view's helpers. +* Deprecate exposing public methods in view's helpers. For example, in methods like `options_from_collection_for_select` - and `collection_select` it was possible to call private methods from + and `collection_select` it is possible to call private methods from the objects used. See [#33546](https://github.com/rails/rails/issues/33546) for details. diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 0fd68b66d4..2ecba2e337 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -794,7 +794,7 @@ module ActionView def extract_values_from_collection(collection, value_method, selected) if selected.is_a?(Proc) collection.map do |element| - element.public_send(value_method) if selected.call(element) + public_or_deprecated_send(element, value_method) if selected.call(element) end.compact else selected @@ -802,7 +802,17 @@ module ActionView end def value_for_collection(item, value) - value.respond_to?(:call) ? value.call(item) : item.public_send(value) + value.respond_to?(:call) ? value.call(item) : public_or_deprecated_send(item, value) + end + + def public_or_deprecated_send(item, value) + begin + item.public_send(value) + rescue NoMethodError + raise unless item.respond_to?(value, true) && !item.respond_to?(value) + ActiveSupport::Deprecation.warn "Using private methods in view's helpers is deprecated" + item.send(value) + end end def prompt_text(prompt) -- cgit v1.2.3 From 0853cdffa2f09f02e854baed35c3323b6b35006f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ana=20Mar=C3=ADa=20Mart=C3=ADnez=20G=C3=B3mez?= Date: Wed, 8 Aug 2018 17:46:53 +0200 Subject: Add tests for privates methods in view's helpers Test that using private methods in `options_from_collection_for_select` is deprecated. Make the unused `secret` paramether in the `Post` Struct private to use it in the test. --- actionview/test/template/form_options_helper_test.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'actionview') diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index 8f796bdb83..7bce946d3b 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -21,7 +21,12 @@ class FormOptionsHelperTest < ActionView::TestCase tests ActionView::Helpers::FormOptionsHelper silence_warnings do - Post = Struct.new("Post", :title, :author_name, :body, :secret, :written_on, :category, :origin, :allow_comments) + Post = Struct.new("Post", :title, :author_name, :body, :written_on, :category, :origin, :allow_comments) do + private + def secret + "This is super secret: #{author_name} is not the real author of #{title}" + end + end Continent = Struct.new("Continent", :continent_name, :countries) Country = Struct.new("Country", :country_id, :country_name) Firm = Struct.new("Firm", :time_zone) @@ -68,6 +73,14 @@ class FormOptionsHelperTest < ActionView::TestCase ) end + def test_collection_options_with_private_value_method + assert_deprecated(/Using private methods in view's helpers is deprecated/) { options_from_collection_for_select(dummy_posts, "secret", "title") } + end + + def test_collection_options_with_private_text_method + assert_deprecated(/Using private methods in view's helpers is deprecated/) { options_from_collection_for_select(dummy_posts, "author_name", "secret") } + end + def test_collection_options_with_preselected_value assert_dom_equal( "\n\n", -- cgit v1.2.3