* 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

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  <nobu@ruby-lang.org>
+
+	* 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  <nobu@ruby-lang.org>
 
 	* 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;