From 34918aa83260246e545911efe6e1672507c3e699 Mon Sep 17 00:00:00 2001 From: nobu Date: Fri, 22 Jul 2011 12:06:42 +0000 Subject: * object.c (rb_mod_{const,cvar}_defined, rb_obj_ivar_defined): avoid inadvertent symbol creation in reflection methods. based on a patch by Jeremy Evans at [ruby-core:38367]. [Feature #5072] * vm_method.c (rb_mod_method_defined) (rb_mod_{public,private,protected}_method_defined) (obj_respond_to): ditto. * parse.y (rb_check_id): new function returns already interned ID or 0. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@32621 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 13 +++++ include/ruby/ruby.h | 1 + internal.h | 8 +++ object.c | 9 ++-- parse.y | 137 ++++++++++++++++++++++++++++++++++++++++------- test/ruby/test_module.rb | 3 ++ test/ruby/test_symbol.rb | 15 ++++++ vm_method.c | 18 +++++-- 8 files changed, 178 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 80243a32d0..12bbad5bbb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +Fri Jul 22 21:06:39 2011 Nobuyoshi Nakada + + * object.c (rb_mod_{const,cvar}_defined, rb_obj_ivar_defined): + avoid inadvertent symbol creation in reflection methods. based + on a patch by Jeremy Evans at [ruby-core:38367]. [Feature #5072] + + * vm_method.c (rb_mod_method_defined) + (rb_mod_{public,private,protected}_method_defined) + (obj_respond_to): ditto. + + * parse.y (rb_check_id): new function returns already interned ID + or 0. + Fri Jul 22 20:44:49 2011 Nobuyoshi Nakada * parse.y (rb_is_global_id, rb_is_attrset_id): add missing diff --git a/include/ruby/ruby.h b/include/ruby/ruby.h index 340bea102a..6b106596f3 100644 --- a/include/ruby/ruby.h +++ b/include/ruby/ruby.h @@ -1098,6 +1098,7 @@ ID rb_intern(const char*); ID rb_intern2(const char*, long); ID rb_intern_str(VALUE str); const char *rb_id2name(ID); +ID rb_check_id(VALUE); ID rb_to_id(VALUE); VALUE rb_id2str(ID); diff --git a/internal.h b/internal.h index 172e7f45ad..59340d2884 100644 --- a/internal.h +++ b/internal.h @@ -133,6 +133,14 @@ VALUE rb_obj_equal(VALUE obj1, VALUE obj2); /* parse.y */ VALUE rb_parser_get_yydebug(VALUE); VALUE rb_parser_set_yydebug(VALUE, VALUE); +int rb_is_const_name(VALUE name); +int rb_is_class_name(VALUE name); +int rb_is_global_name(VALUE name); +int rb_is_instance_name(VALUE name); +int rb_is_attrset_name(VALUE name); +int rb_is_local_name(VALUE name); +int rb_is_method_name(VALUE name); +int rb_is_junk_name(VALUE name); /* proc.c */ VALUE rb_proc_location(VALUE self); diff --git a/object.c b/object.c index 57b349a9c3..42d9d2509b 100644 --- a/object.c +++ b/object.c @@ -1833,7 +1833,8 @@ rb_mod_const_defined(int argc, VALUE *argv, VALUE mod) else { rb_scan_args(argc, argv, "11", &name, &recur); } - id = rb_to_id(name); + if (!(id = rb_check_id(name)) && rb_is_const_name(name)) + return Qfalse; if (!rb_is_const_id(id)) { rb_name_error(id, "wrong constant name %s", rb_id2name(id)); } @@ -1923,8 +1924,9 @@ rb_obj_ivar_set(VALUE obj, VALUE iv, VALUE val) static VALUE rb_obj_ivar_defined(VALUE obj, VALUE iv) { - ID id = rb_to_id(iv); + ID id = rb_check_id(iv); + if (!id && rb_is_instance_name(iv)) return Qfalse; if (!rb_is_instance_id(id)) { rb_name_error(id, "`%s' is not allowed as an instance variable name", rb_id2name(id)); } @@ -2002,8 +2004,9 @@ rb_mod_cvar_set(VALUE obj, VALUE iv, VALUE val) static VALUE rb_mod_cvar_defined(VALUE obj, VALUE iv) { - ID id = rb_to_id(iv); + ID id = rb_check_id(iv); + if (!id && rb_is_class_name(iv)) return Qfalse; if (!rb_is_class_id(id)) { rb_name_error(id, "`%s' is not allowed as a class variable name", rb_id2name(id)); } diff --git a/parse.y b/parse.y index 2dfd33d92a..d38e0d04c6 100644 --- a/parse.y +++ b/parse.y @@ -9678,24 +9678,29 @@ rb_enc_symname_p(const char *name, rb_encoding *enc) return rb_enc_symname2_p(name, strlen(name), enc); } -int -rb_enc_symname2_p(const char *name, long len, rb_encoding *enc) +static int +rb_enc_symname_type(const char *name, long len, rb_encoding *enc) { const char *m = name; const char *e = m + len; - int localid = FALSE; + int type = ID_JUNK; - if (!m || len <= 0) return FALSE; + if (!m || len <= 0) return -1; switch (*m) { case '\0': - return FALSE; + return -1; case '$': - if (is_special_global_name(++m, e, enc)) return TRUE; + type = ID_GLOBAL; + if (is_special_global_name(++m, e, enc)) return type; goto id; case '@': - if (*++m == '@') ++m; + type = ID_INSTANCE; + if (*++m == '@') { + ++m; + type = ID_CLASS; + } goto id; case '<': @@ -9716,7 +9721,7 @@ rb_enc_symname2_p(const char *name, long len, rb_encoding *enc) switch (*++m) { case '~': ++m; break; case '=': if (*++m == '=') ++m; break; - default: return FALSE; + default: return -1; } break; @@ -9733,32 +9738,53 @@ rb_enc_symname2_p(const char *name, long len, rb_encoding *enc) break; case '[': - if (*++m != ']') return FALSE; + if (*++m != ']') return -1; if (*++m == '=') ++m; break; case '!': - if (len == 1) return FALSE; + if (len == 1) return ID_JUNK; switch (*++m) { case '=': case '~': ++m; break; - default: return FALSE; + default: return -1; } break; default: - localid = !rb_enc_isupper(*m, enc); + type = rb_enc_isupper(*m, enc) ? ID_CONST : ID_LOCAL; id: if (m >= e || (*m != '_' && !rb_enc_isalpha(*m, enc) && ISASCII(*m))) - return FALSE; + return -1; while (m < e && is_identchar(m, e, enc)) m += rb_enc_mbclen(m, e, enc); - if (localid) { - switch (*m) { - case '!': case '?': case '=': ++m; - } + switch (*m) { + case '!': case '?': + type = ID_JUNK; + ++m; + break; + case '=': + type = ID_ATTRSET; + ++m; + break; } break; } - return m == e; + return m == e ? type : -1; +} + +int +rb_enc_symname2_p(const char *name, long len, rb_encoding *enc) +{ + return rb_enc_symname_type(name, len, enc) != -1; +} + +static int +rb_str_symname_type(VALUE name) +{ + const char *ptr = StringValuePtr(name); + long len = RSTRING_LEN(name); + int type = rb_enc_symname_type(ptr, len, rb_enc_get(name)); + RB_GC_GUARD(name); + return type; } static ID @@ -10076,6 +10102,81 @@ rb_is_junk_id(ID id) return is_junk_id(id); } +ID +rb_check_id(VALUE name) +{ + st_data_t id; + VALUE tmp; + + if (SYMBOL_P(name)) { + return SYM2ID(name); + } + else if (RB_TYPE_P(name, T_STRING)) { + tmp = rb_check_string_type(name); + if (NIL_P(tmp)) { + tmp = rb_inspect(name); + rb_raise(rb_eTypeError, "%s is not a symbol", + RSTRING_PTR(tmp)); + } + name = tmp; + } + if (!st_lookup(global_symbols.sym_id, (st_data_t)name, &id)) + return (ID)0; + return (ID)id; +} + +int +rb_is_const_name(VALUE name) +{ + return rb_str_symname_type(name) == ID_CONST; +} + +int +rb_is_class_name(VALUE name) +{ + return rb_str_symname_type(name) == ID_CLASS; +} + +int +rb_is_global_name(VALUE name) +{ + return rb_str_symname_type(name) == ID_GLOBAL; +} + +int +rb_is_instance_name(VALUE name) +{ + return rb_str_symname_type(name) == ID_INSTANCE; +} + +int +rb_is_attrset_name(VALUE name) +{ + return rb_str_symname_type(name) == ID_ATTRSET; +} + +int +rb_is_local_name(VALUE name) +{ + return rb_str_symname_type(name) == ID_LOCAL; +} + +int +rb_is_method_name(VALUE name) +{ + switch (rb_str_symname_type(name)) { + case ID_LOCAL: case ID_ATTRSET: case ID_JUNK: + return TRUE; + } + return FALSE; +} + +int +rb_is_junk_name(VALUE name) +{ + return rb_str_symname_type(name) == -1; +} + #endif /* !RIPPER */ static void diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb index e6c107961d..1aa7f8c691 100644 --- a/test/ruby/test_module.rb +++ b/test/ruby/test_module.rb @@ -495,6 +495,9 @@ class TestModule < Test::Unit::TestCase def test_const_get_invalid_name c1 = Class.new assert_raise(NameError) { c1.const_defined?(:foo) } + name = "gadzooks" + assert !Symbol.all_symbols.any? {|sym| sym.to_s == name} + assert_raise(NameError) { c1.const_defined?(name) } end def test_const_get_no_inherited diff --git a/test/ruby/test_symbol.rb b/test/ruby/test_symbol.rb index 9061f191bf..282964cdf5 100644 --- a/test/ruby/test_symbol.rb +++ b/test/ruby/test_symbol.rb @@ -144,4 +144,19 @@ class TestSymbol < Test::Unit::TestCase assert_equal(':"\\u3042\\u3044\\u3046"', "\u3042\u3044\u3046".encode(e).to_sym.inspect) end end + + def test_no_inadvertent_symbol_creation + feature5072 = '[ruby-core:38367]' + c = Class.new + s = "gadzooks" + {:respond_to? => "#{s}1", :method_defined? => "#{s}2", + :public_method_defined? => "#{s}3", :private_method_defined? => "#{s}4", + :protected_method_defined? => "#{s}5", :const_defined? => "A#{s}", + :instance_variable_defined? => "@#{s}", :class_variable_defined? => "@@#{s}" + }.each do |meth, str| + msg = "#{meth}(#{str}) #{feature5072}" + assert !c.send(meth, str), msg + assert !Symbol.all_symbols.any? {|sym| sym.to_s == str}, msg + end + end end diff --git a/vm_method.c b/vm_method.c index 218d51f431..cea26cdc31 100644 --- a/vm_method.c +++ b/vm_method.c @@ -713,7 +713,8 @@ rb_mod_undef_method(int argc, VALUE *argv, VALUE mod) static VALUE rb_mod_method_defined(VALUE mod, VALUE mid) { - if (!rb_method_boundp(mod, rb_to_id(mid), 1)) { + ID id = rb_check_id(mid); + if (!id || !rb_method_boundp(mod, id, 1)) { return Qfalse; } return Qtrue; @@ -763,7 +764,9 @@ check_definition(VALUE mod, ID mid, rb_method_flag_t noex) static VALUE rb_mod_public_method_defined(VALUE mod, VALUE mid) { - return check_definition(mod, rb_to_id(mid), NOEX_PUBLIC); + ID id = rb_check_id(mid); + if (!id) return Qfalse; + return check_definition(mod, id, NOEX_PUBLIC); } /* @@ -795,7 +798,9 @@ rb_mod_public_method_defined(VALUE mod, VALUE mid) static VALUE rb_mod_private_method_defined(VALUE mod, VALUE mid) { - return check_definition(mod, rb_to_id(mid), NOEX_PRIVATE); + ID id = rb_check_id(mid); + if (!id) return Qfalse; + return check_definition(mod, id, NOEX_PRIVATE); } /* @@ -827,7 +832,9 @@ rb_mod_private_method_defined(VALUE mod, VALUE mid) static VALUE rb_mod_protected_method_defined(VALUE mod, VALUE mid) { - return check_definition(mod, rb_to_id(mid), NOEX_PROTECTED); + ID id = rb_check_id(mid); + if (!id) return Qfalse; + return check_definition(mod, id, NOEX_PROTECTED); } int @@ -1238,7 +1245,8 @@ obj_respond_to(int argc, VALUE *argv, VALUE obj) ID id; rb_scan_args(argc, argv, "11", &mid, &priv); - id = rb_to_id(mid); + if (!(id = rb_check_id(mid))) + return Qfalse; if (basic_obj_respond_to(obj, id, !RTEST(priv))) return Qtrue; return Qfalse; -- cgit v1.2.3