From bb29fbc132fcd33ab94d1eb1e3240dc03a4233f2 Mon Sep 17 00:00:00 2001 From: Genadi Samokovarov Date: Sat, 29 Nov 2014 13:35:29 +0200 Subject: Don't leak Object constants in core_ext/module/qualified_const --- activesupport/CHANGELOG.md | 9 +- .../core_ext/module/qualified_const.rb | 42 +++++-- .../test/core_ext/module/qualified_const_test.rb | 130 +++++++++++---------- 3 files changed, 107 insertions(+), 74 deletions(-) (limited to 'activesupport') diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8e0e5bb2ef..07f7fae5d5 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,8 @@ +* Deprecated `Module#qualified_const_` in favour of the builtin Module#const_ + methods. + + *Genadi Samokovarov* + * Deprecate passing string to define callback. *Yuichiro Kaneko* @@ -6,9 +11,9 @@ `ActiveSupport::Cache::MemCachedStore#escape_key`, and `ActiveSupport::Cache::FileStore#key_file_path` are deprecated and replaced with `normalize_key` that now calls `super`. - + `ActiveSupport::Cache::LocaleCache#set_cache_value` is deprecated and replaced with `write_cache_value`. - + *Michael Grosser* * Implements an evented file watcher to asynchronously detect changes in the diff --git a/activesupport/lib/active_support/core_ext/module/qualified_const.rb b/activesupport/lib/active_support/core_ext/module/qualified_const.rb index 65525013db..3ea39d4267 100644 --- a/activesupport/lib/active_support/core_ext/module/qualified_const.rb +++ b/activesupport/lib/active_support/core_ext/module/qualified_const.rb @@ -3,13 +3,16 @@ require 'active_support/core_ext/string/inflections' #-- # Allows code reuse in the methods below without polluting Module. #++ -module QualifiedConstUtils - def self.raise_if_absolute(path) - raise NameError.new("wrong constant name #$&") if path =~ /\A::[^:]+/ - end - def self.names(path) - path.split('::') +module ActiveSupport + module QualifiedConstUtils + def self.raise_if_absolute(path) + raise NameError.new("wrong constant name #$&") if path =~ /\A::[^:]+/ + end + + def self.names(path) + path.split('::') + end end end @@ -24,9 +27,14 @@ end #++ class Module def qualified_const_defined?(path, search_parents=true) - QualifiedConstUtils.raise_if_absolute(path) + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + Module#qualified_const_defined? is deprecated in favour of the builtin + Module#const_defined? and will be removed in Rails 5.1. + MESSAGE - QualifiedConstUtils.names(path).inject(self) do |mod, name| + ActiveSupport::QualifiedConstUtils.raise_if_absolute(path) + + ActiveSupport::QualifiedConstUtils.names(path).inject(self) do |mod, name| return unless mod.const_defined?(name, search_parents) mod.const_get(name) end @@ -34,19 +42,29 @@ class Module end def qualified_const_get(path) - QualifiedConstUtils.raise_if_absolute(path) + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + Module#qualified_const_get is deprecated in favour of the builtin + Module#const_get and will be removed in Rails 5.1. + MESSAGE + + ActiveSupport::QualifiedConstUtils.raise_if_absolute(path) - QualifiedConstUtils.names(path).inject(self) do |mod, name| + ActiveSupport::QualifiedConstUtils.names(path).inject(self) do |mod, name| mod.const_get(name) end end def qualified_const_set(path, value) - QualifiedConstUtils.raise_if_absolute(path) + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + Module#qualified_const_set is deprecated in favour of the builtin + Module#const_set and will be removed in Rails 5.1. + MESSAGE + + ActiveSupport::QualifiedConstUtils.raise_if_absolute(path) const_name = path.demodulize mod_name = path.deconstantize - mod = mod_name.empty? ? self : qualified_const_get(mod_name) + mod = mod_name.empty? ? self : const_get(mod_name) mod.const_set(const_name, value) end end diff --git a/activesupport/test/core_ext/module/qualified_const_test.rb b/activesupport/test/core_ext/module/qualified_const_test.rb index 37c9228a64..a3146cabe1 100644 --- a/activesupport/test/core_ext/module/qualified_const_test.rb +++ b/activesupport/test/core_ext/module/qualified_const_test.rb @@ -19,84 +19,94 @@ end class QualifiedConstTest < ActiveSupport::TestCase test "Object.qualified_const_defined?" do - assert Object.qualified_const_defined?("QualifiedConstTestMod") - assert !Object.qualified_const_defined?("NonExistingQualifiedConstTestMod") - - assert Object.qualified_const_defined?("QualifiedConstTestMod::X") - assert !Object.qualified_const_defined?("QualifiedConstTestMod::Y") - - assert Object.qualified_const_defined?("QualifiedConstTestMod::M::X") - assert !Object.qualified_const_defined?("QualifiedConstTestMod::M::Y") - - if Module.method(:const_defined?).arity == 1 - assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X") - else - assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X") - assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X", false) - assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X", true) + assert_deprecated do + assert Object.qualified_const_defined?("QualifiedConstTestMod") + assert !Object.qualified_const_defined?("NonExistingQualifiedConstTestMod") + + assert Object.qualified_const_defined?("QualifiedConstTestMod::X") + assert !Object.qualified_const_defined?("QualifiedConstTestMod::Y") + + assert Object.qualified_const_defined?("QualifiedConstTestMod::M::X") + assert !Object.qualified_const_defined?("QualifiedConstTestMod::M::Y") + + if Module.method(:const_defined?).arity == 1 + assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X") + else + assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X") + assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X", false) + assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X", true) + end end end test "mod.qualified_const_defined?" do - assert QualifiedConstTestMod.qualified_const_defined?("M") - assert !QualifiedConstTestMod.qualified_const_defined?("NonExistingM") - - assert QualifiedConstTestMod.qualified_const_defined?("M::X") - assert !QualifiedConstTestMod.qualified_const_defined?("M::Y") - - assert QualifiedConstTestMod.qualified_const_defined?("M::C::X") - assert !QualifiedConstTestMod.qualified_const_defined?("M::C::Y") - - if Module.method(:const_defined?).arity == 1 - assert !QualifiedConstTestMod.qualified_const_defined?("QualifiedConstTestMod::N::X") - else - assert QualifiedConstTestMod.qualified_const_defined?("N::X") - assert !QualifiedConstTestMod.qualified_const_defined?("N::X", false) - assert QualifiedConstTestMod.qualified_const_defined?("N::X", true) + assert_deprecated do + assert QualifiedConstTestMod.qualified_const_defined?("M") + assert !QualifiedConstTestMod.qualified_const_defined?("NonExistingM") + + assert QualifiedConstTestMod.qualified_const_defined?("M::X") + assert !QualifiedConstTestMod.qualified_const_defined?("M::Y") + + assert QualifiedConstTestMod.qualified_const_defined?("M::C::X") + assert !QualifiedConstTestMod.qualified_const_defined?("M::C::Y") + + if Module.method(:const_defined?).arity == 1 + assert !QualifiedConstTestMod.qualified_const_defined?("QualifiedConstTestMod::N::X") + else + assert QualifiedConstTestMod.qualified_const_defined?("N::X") + assert !QualifiedConstTestMod.qualified_const_defined?("N::X", false) + assert QualifiedConstTestMod.qualified_const_defined?("N::X", true) + end end end test "qualified_const_get" do - assert_equal false, Object.qualified_const_get("QualifiedConstTestMod::X") - assert_equal false, QualifiedConstTestMod.qualified_const_get("X") - assert_equal 1, QualifiedConstTestMod.qualified_const_get("M::X") - assert_equal 1, QualifiedConstTestMod.qualified_const_get("N::X") - assert_equal 2, QualifiedConstTestMod.qualified_const_get("M::C::X") - - assert_raise(NameError) { QualifiedConstTestMod.qualified_const_get("M::C::Y")} + assert_deprecated do + assert_equal false, Object.qualified_const_get("QualifiedConstTestMod::X") + assert_equal false, QualifiedConstTestMod.qualified_const_get("X") + assert_equal 1, QualifiedConstTestMod.qualified_const_get("M::X") + assert_equal 1, QualifiedConstTestMod.qualified_const_get("N::X") + assert_equal 2, QualifiedConstTestMod.qualified_const_get("M::C::X") + + assert_raise(NameError) { QualifiedConstTestMod.qualified_const_get("M::C::Y")} + end end test "qualified_const_set" do - begin - m = Module.new - assert_equal m, Object.qualified_const_set("QualifiedConstTestMod2", m) - assert_equal m, ::QualifiedConstTestMod2 - - # We are going to assign to existing constants on purpose, so silence warnings. - silence_warnings do - assert_equal true, QualifiedConstTestMod.qualified_const_set("QualifiedConstTestMod::X", true) - assert_equal true, QualifiedConstTestMod::X - - assert_equal 10, QualifiedConstTestMod::M.qualified_const_set("X", 10) - assert_equal 10, QualifiedConstTestMod::M::X - end - ensure - silence_warnings do - QualifiedConstTestMod.qualified_const_set('QualifiedConstTestMod::X', false) - QualifiedConstTestMod::M.qualified_const_set('X', 1) + assert_deprecated do + begin + m = Module.new + assert_equal m, Object.qualified_const_set("QualifiedConstTestMod2", m) + assert_equal m, ::QualifiedConstTestMod2 + + # We are going to assign to existing constants on purpose, so silence warnings. + silence_warnings do + assert_equal true, QualifiedConstTestMod.qualified_const_set("QualifiedConstTestMod::X", true) + assert_equal true, QualifiedConstTestMod::X + + assert_equal 10, QualifiedConstTestMod::M.qualified_const_set("X", 10) + assert_equal 10, QualifiedConstTestMod::M::X + end + ensure + silence_warnings do + QualifiedConstTestMod.qualified_const_set('QualifiedConstTestMod::X', false) + QualifiedConstTestMod::M.qualified_const_set('X', 1) + end end end end test "reject absolute paths" do - assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X")} - assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X::Y")} + assert_deprecated do + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X::Y")} - assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X")} - assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X::Y")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X::Y")} - assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X", nil)} - assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X::Y", nil)} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X", nil)} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X::Y", nil)} + end end private -- cgit v1.2.3