From 4d11acd58ad3cdafb96cde75354d3a6cad676c86 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Sat, 23 Jul 2011 22:34:04 -0700 Subject: [PATCH 1/2] More removal of inadvertent symbol creation This is the third and hopefully final patch for removing inadvertent symbol creation. It removes it from the following methods: alias_method autoload? private_class_method private_constant private protected public public_class_method public_constant remove_method undef_method untrace_var It also removes it from const_get if an invalid constant name is used or const_missing has not been overridden. If a valid constant name is used and const_missing has been overridden, then a symbol must be created so it can be passed to const_missing. --- eval_error.c | 9 +++++++++ eval_intern.h | 3 +++ load.c | 6 +++++- object.c | 21 ++++++++++++++++++++- proc.c | 2 +- test/ruby/test_module.rb | 8 ++++++++ test/ruby/test_symbol.rb | 33 +++++++++++++++++++++++++++++++++ variable.c | 12 ++++++++++-- vm_method.c | 33 ++++++++++++++++++++++++++++++--- 9 files changed, 119 insertions(+), 8 deletions(-) diff --git a/eval_error.c b/eval_error.c index fd06adf..95a9728 100644 --- a/eval_error.c +++ b/eval_error.c @@ -206,6 +206,15 @@ rb_print_undef(VALUE klass, ID id, int scope) rb_class2name(klass)); } +void +rb_print_undef_str(VALUE klass, VALUE name) +{ + rb_name_error_str(name, "undefined method `%s' for %s `%s'", + RSTRING_PTR(name), + (TYPE(klass) == T_MODULE) ? "module" : "class", + rb_class2name(klass)); +} + static int sysexit_status(VALUE err) { diff --git a/eval_intern.h b/eval_intern.h index c4f0c46..dd10920 100644 --- a/eval_intern.h +++ b/eval_intern.h @@ -198,9 +198,12 @@ int rb_threadptr_reset_raised(rb_thread_t *th); VALUE rb_f_eval(int argc, VALUE *argv, VALUE self); VALUE rb_make_exception(int argc, VALUE *argv); +NORETURN(void rb_method_name_error(VALUE, VALUE)); + NORETURN(void rb_fiber_start(void)); NORETURN(void rb_print_undef(VALUE, ID, int)); +NORETURN(void rb_print_undef_str(VALUE, VALUE)); NORETURN(void rb_vm_localjump_error(const char *,VALUE, int)); NORETURN(void rb_vm_jump_tag_but_local_jump(int, VALUE)); NORETURN(void rb_raise_method_missing(rb_thread_t *th, int argc, VALUE *argv, diff --git a/load.c b/load.c index 62e2cac..b45ff30 100644 --- a/load.c +++ b/load.c @@ -705,7 +705,11 @@ rb_mod_autoload(VALUE mod, VALUE sym, VALUE file) static VALUE rb_mod_autoload_p(VALUE mod, VALUE sym) { - return rb_autoload_p(mod, rb_to_id(sym)); + ID id = rb_check_id(sym); + if(!id) { + return Qnil; + } + return rb_autoload_p(mod, id); } /* diff --git a/object.c b/object.c index cad41e6..9248ee7 100644 --- a/object.c +++ b/object.c @@ -35,6 +35,7 @@ VALUE rb_cFalseClass; static ID id_eq, id_eql, id_match, id_inspect; static ID id_init_copy, id_init_clone, id_init_dup; +static ID id_const_missing; /* * call-seq: @@ -1774,7 +1775,24 @@ rb_mod_const_get(int argc, VALUE *argv, VALUE mod) else { rb_scan_args(argc, argv, "11", &name, &recur); } - id = rb_to_id(name); + id = rb_check_id(name); + if (!id) { + if(rb_is_const_name(name)) { + if (rb_method_basic_definition_p(CLASS_OF(mod), id_const_missing)) { + if (mod && rb_class_real(mod) != rb_cObject) { + rb_name_error_str(name, "uninitialized constant %s::%s", + rb_class2name(mod), + RSTRING_PTR(name)); + } else { + rb_name_error_str(name, "uninitialized constant %s", RSTRING_PTR(name)); + } + } else { + id = rb_to_id(name); + } + } else { + rb_name_error_str(name, "wrong constant name %s", RSTRING_PTR(name)); + } + } if (!rb_is_const_id(id)) { rb_name_error(id, "wrong constant name %s", rb_id2name(id)); } @@ -2813,6 +2831,7 @@ Init_Object(void) id_init_copy = rb_intern("initialize_copy"); id_init_clone = rb_intern("initialize_clone"); id_init_dup = rb_intern("initialize_dup"); + id_const_missing = rb_intern("const_missing"); for (i=0; conv_method_names[i].method; i++) { conv_method_names[i].id = rb_intern(conv_method_names[i].method); diff --git a/proc.c b/proc.c index 46296cc..838a465 100644 --- a/proc.c +++ b/proc.c @@ -1140,7 +1140,7 @@ method_owner(VALUE obj) return data->me.klass; } -static void +void rb_method_name_error(VALUE klass, VALUE str) { const char *s0 = " class"; diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb index b6e827a..e2680dd 100644 --- a/test/ruby/test_module.rb +++ b/test/ruby/test_module.rb @@ -472,6 +472,9 @@ class TestModule < Test::Unit::TestCase assert_raise(NameError) { c2::Bar } assert_raise(NameError) { c2.const_get(:Bar) } assert_raise(NameError) { c2.const_get(:Bar, false) } + assert_raise(NameError) { c2.const_get("Bar", false) } + assert_raise(NameError) { c2.const_get("BaR11", false) } + assert_raise(NameError) { Object.const_get("BaR11", false) } c1.instance_eval do def const_missing(x) @@ -483,6 +486,11 @@ class TestModule < Test::Unit::TestCase assert_equal(:Bar, c2::Bar) assert_equal(:Bar, c2.const_get(:Bar)) assert_equal(:Bar, c2.const_get(:Bar, false)) + assert_equal(:Bar, c2.const_get("Bar")) + assert_equal(:Bar, c2.const_get("Bar", false)) + + v = c2.const_get("Bar11", false) + assert_equal("Bar11".to_sym, v) assert_raise(NameError) { c1.const_get(:foo) } end diff --git a/test/ruby/test_symbol.rb b/test/ruby/test_symbol.rb index 0113504..25a48cb 100644 --- a/test/ruby/test_symbol.rb +++ b/test/ruby/test_symbol.rb @@ -197,4 +197,37 @@ class TestSymbol < Test::Unit::TestCase assert !Symbol.all_symbols.any? {|sym| sym.to_s == str}, msg end end + + def test_no_inadvertent_symbol_creation3 + feature50XX = '[ruby-core:384XX]' + c = Class.new do + def self.alias_method(str) + super(:puts, str) + end + end + s = "gadzoooks" + {:alias_method => ["#{s}1", NameError], + :autoload? => ["#{s}2", nil], + :const_get => ["A#{s}3", NameError], + :private_class_method => ["#{s}4", NameError], + :private_constant => ["#{s}5", NameError], + :private => ["#{s}6", NameError], + :protected => ["#{s}7", NameError], + :public => ["#{s}8", NameError], + :public_class_method => ["#{s}9", NameError], + :public_constant => ["#{s}10", NameError], + :remove_method => ["#{s}11", NameError], + :undef_method => ["#{s}12", NameError], + :untrace_var => ["#{s}13", NameError], + }.each do |meth, arr| + str, ret = arr + msg = "#{meth}(#{str}) #{feature50XX}" + if ret.is_a?(Class) && (ret < Exception) + assert_raises(ret){c.send(meth, str)} + else + assert(c.send(meth, str) == ret, msg) + end + assert !Symbol.all_symbols.any? {|sym| sym.to_s == str}, msg + end + end end diff --git a/variable.c b/variable.c index 8a34d2e..bbcd2fd 100644 --- a/variable.c +++ b/variable.c @@ -634,7 +634,10 @@ rb_f_untrace_var(int argc, VALUE *argv) rb_secure(4); rb_scan_args(argc, argv, "11", &var, &cmd); - id = rb_to_id(var); + id = rb_check_id(var); + if (!id) { + rb_name_error_str(var, "undefined global variable %s", RSTRING_PTR(var)); + } if (!st_lookup(rb_global_tbl, (st_data_t)id, &data)) { rb_name_error(id, "undefined global variable %s", rb_id2name(id)); } @@ -1965,6 +1968,7 @@ set_const_visibility(VALUE mod, int argc, VALUE *argv, rb_const_flag_t flag) int i; st_data_t v; ID id; + VALUE val; if (rb_safe_level() >= 4 && !OBJ_UNTRUSTED(mod)) { rb_raise(rb_eSecurityError, @@ -1972,7 +1976,11 @@ set_const_visibility(VALUE mod, int argc, VALUE *argv, rb_const_flag_t flag) } for (i = 0; i < argc; i++) { - id = rb_to_id(argv[i]); + val = argv[i]; + id = rb_check_id(val); + if (!id) { + rb_name_error_str(val, "constant %s::%s not defined", rb_class2name(mod), RSTRING_PTR(val)); + } if (RCLASS_CONST_TBL(mod) && st_lookup(RCLASS_CONST_TBL(mod), (st_data_t)id, &v)) { ((rb_const_entry_t*)v)->flag = flag; return; diff --git a/vm_method.c b/vm_method.c index cea26cd..c8a876b 100644 --- a/vm_method.c +++ b/vm_method.c @@ -469,9 +469,17 @@ static VALUE rb_mod_remove_method(int argc, VALUE *argv, VALUE mod) { int i; + ID id; + VALUE v; for (i = 0; i < argc; i++) { - remove_method(mod, rb_to_id(argv[i])); + v = argv[i]; + id = rb_check_id(v); + if (!id) { + rb_name_error_str(v, "method `%s' not defined in %s", + RSTRING_PTR(v), rb_class2name(mod)); + } + remove_method(mod, id); } return mod; } @@ -678,8 +686,16 @@ static VALUE rb_mod_undef_method(int argc, VALUE *argv, VALUE mod) { int i; + ID id; + VALUE v; + for (i = 0; i < argc; i++) { - rb_undef(mod, rb_to_id(argv[i])); + v = argv[i]; + id = rb_check_id(v); + if (!id) { + rb_method_name_error(mod, v); + } + rb_undef(mod, id); } return mod; } @@ -938,6 +954,10 @@ rb_alias(VALUE klass, ID name, ID def) static VALUE rb_mod_alias_method(VALUE mod, VALUE newname, VALUE oldname) { + ID oldid = rb_check_id(oldname); + if(!oldid) { + rb_print_undef_str(mod, oldname); + } rb_alias(mod, rb_to_id(newname), rb_to_id(oldname)); return mod; } @@ -955,9 +975,16 @@ static void set_method_visibility(VALUE self, int argc, VALUE *argv, rb_method_flag_t ex) { int i; + ID id; + VALUE v; secure_visibility(self); for (i = 0; i < argc; i++) { - rb_export_method(self, rb_to_id(argv[i]), ex); + v = argv[i]; + id = rb_check_id(v); + if (!id) { + rb_print_undef_str(self, v); + } + rb_export_method(self, id, ex); } rb_clear_cache_by_class(self); } -- 1.7.5