nobu	2013-10-09 23:57:04 +0900 (Wed, 09 Oct 2013)

  New Revision: 43219

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=43219

  Log:
    object.c: avoid inadvertent symbol creation
    
    * object.c (id_for_attr): avoid inadvertent symbol creation.

  Modified files:
    trunk/ChangeLog
    trunk/object.c
    trunk/test/-ext-/symbol/test_inadvertent_creation.rb
    trunk/vm_method.c
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 43218)
+++ ChangeLog	(revision 43219)
@@ -1,3 +1,7 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Wed Oct  9 23:57:02 2013  Nobuyoshi Nakada  <nobu@r...>
+
+	* object.c (id_for_attr): avoid inadvertent symbol creation.
+
 Wed Oct  9 18:03:01 2013  Nobuyoshi Nakada  <nobu@r...>
 
 	* vm_method.c (rb_attr): preserve encoding of the attribute ID in
Index: object.c
===================================================================
--- object.c	(revision 43218)
+++ object.c	(revision 43219)
@@ -1889,6 +1889,35 @@ check_setter_id(VALUE name, int (*valid_ https://github.com/ruby/ruby/blob/trunk/object.c#L1889
     return id;
 }
 
+static int
+rb_is_attr_id(ID id)
+{
+    return rb_is_local_id(id) || rb_is_const_id(id);
+}
+
+static int
+rb_is_attr_name(VALUE name)
+{
+    return rb_is_local_name(name) || rb_is_const_name(name);
+}
+
+static const char invalid_attribute_name[] = "invalid attribute name `%"PRIsVALUE"'";
+
+static ID
+id_for_attr(VALUE name)
+{
+    return id_for_setter(name, attr, invalid_attribute_name);
+}
+
+ID
+rb_check_attr_id(ID id)
+{
+    if (!rb_is_attr_id(id)) {
+	rb_name_error_str(id, invalid_attribute_name, QUOTE_ID(id));
+    }
+    return id;
+}
+
 /*
  *  call-seq:
  *     attr_reader(symbol, ...)  -> nil
@@ -1908,7 +1937,7 @@ rb_mod_attr_reader(int argc, VALUE *argv https://github.com/ruby/ruby/blob/trunk/object.c#L1937
     int i;
 
     for (i=0; i<argc; i++) {
-	rb_attr(klass, rb_to_id(argv[i]), TRUE, FALSE, TRUE);
+	rb_attr(klass, id_for_attr(argv[i]), TRUE, FALSE, TRUE);
     }
     return Qnil;
 }
@@ -1918,7 +1947,7 @@ rb_mod_attr(int argc, VALUE *argv, VALUE https://github.com/ruby/ruby/blob/trunk/object.c#L1947
 {
     if (argc == 2 && (argv[1] == Qtrue || argv[1] == Qfalse)) {
 	rb_warning("optional boolean argument is obsoleted");
-	rb_attr(klass, rb_to_id(argv[0]), 1, RTEST(argv[1]), TRUE);
+	rb_attr(klass, id_for_attr(argv[0]), 1, RTEST(argv[1]), TRUE);
 	return Qnil;
     }
     return rb_mod_attr_reader(argc, argv, klass);
@@ -1940,7 +1969,7 @@ rb_mod_attr_writer(int argc, VALUE *argv https://github.com/ruby/ruby/blob/trunk/object.c#L1969
     int i;
 
     for (i=0; i<argc; i++) {
-	rb_attr(klass, rb_to_id(argv[i]), FALSE, TRUE, TRUE);
+	rb_attr(klass, id_for_attr(argv[i]), FALSE, TRUE, TRUE);
     }
     return Qnil;
 }
@@ -1968,7 +1997,7 @@ rb_mod_attr_accessor(int argc, VALUE *ar https://github.com/ruby/ruby/blob/trunk/object.c#L1997
     int i;
 
     for (i=0; i<argc; i++) {
-	rb_attr(klass, rb_to_id(argv[i]), TRUE, TRUE, TRUE);
+	rb_attr(klass, id_for_attr(argv[i]), TRUE, TRUE, TRUE);
     }
     return Qnil;
 }
Index: vm_method.c
===================================================================
--- vm_method.c	(revision 43218)
+++ vm_method.c	(revision 43219)
@@ -809,6 +809,8 @@ rb_method_boundp(VALUE klass, ID id, int https://github.com/ruby/ruby/blob/trunk/vm_method.c#L809
     return 0;
 }
 
+extern ID rb_check_attr_id(ID id);
+
 void
 rb_attr(VALUE klass, ID id, int read, int write, int ex)
 {
@@ -834,10 +836,7 @@ rb_attr(VALUE klass, ID id, int read, in https://github.com/ruby/ruby/blob/trunk/vm_method.c#L836
 	}
     }
 
-    if (!rb_is_local_id(id) && !rb_is_const_id(id)) {
-	rb_name_error_str(id, "invalid attribute name `%"PRIsVALUE"'", QUOTE_ID(id));
-    }
-    aname = rb_id2str(id);
+    aname = rb_id2str(rb_check_attr_id(id));
     if (NIL_P(aname)) {
 	rb_raise(rb_eArgError, "argument needs to be symbol or string");
     }
Index: test/-ext-/symbol/test_inadvertent_creation.rb
===================================================================
--- test/-ext-/symbol/test_inadvertent_creation.rb	(revision 43218)
+++ test/-ext-/symbol/test_inadvertent_creation.rb	(revision 43219)
@@ -234,5 +234,33 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L234
       assert_raise(NameError) {s[name] = true}
       assert_not_interned(name)
     end
+
+    def test_invalid_attr
+      name = noninterned_name("*")
+      mod = Module.new
+      assert_raise(NameError) {mod.module_eval {attr(name)}}
+      assert_not_interned(name)
+    end
+
+    def test_invalid_attr_reader
+      name = noninterned_name("*")
+      mod = Module.new
+      assert_raise(NameError) {mod.module_eval {attr_reader(name)}}
+      assert_not_interned(name)
+    end
+
+    def test_invalid_attr_writer
+      name = noninterned_name("*")
+      mod = Module.new
+      assert_raise(NameError) {mod.module_eval {attr_writer(name)}}
+      assert_not_interned(name)
+    end
+
+    def test_invalid_attr_accessor
+      name = noninterned_name("*")
+      mod = Module.new
+      assert_raise(NameError) {mod.module_eval {attr_accessor(name)}}
+      assert_not_interned(name)
+    end
   end
 end

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/