Merge pull request #4328 from shuujii/fix-constant-name-validation

Fix constant name validation

include/mruby/variable.h

@@ -123,6 +123,7 @@ mrb_value mrb_obj_instance_variables(mrb_state*, mrb_value);
 mrb_value mrb_mod_class_variables(mrb_state*, mrb_value);
 mrb_value mrb_mod_cv_get(mrb_state *mrb, struct RClass * c, mrb_sym sym);
 mrb_bool mrb_mod_cv_defined(mrb_state *mrb, struct RClass * c, mrb_sym sym);
+mrb_bool mrb_ident_p(const char *s, mrb_int len);
 
 /* GC functions */
 void mrb_gc_mark_gv(mrb_state*);

src/class.c

@@ -1898,7 +1898,7 @@ mrb_mod_undef(mrb_state *mrb, mrb_value mod)
 static mrb_bool
 const_name_p(mrb_state *mrb, const char *name, mrb_int len)
 {
-  return len > 0 && ISUPPER(name[0]);
+  return len > 0 && ISUPPER(name[0]) && mrb_ident_p(name+1, len-1);
 }
 
 static void

src/variable.c

@@ -428,22 +428,17 @@ mrb_iv_defined(mrb_state *mrb, mrb_value obj, mrb_sym sym)
   return mrb_obj_iv_defined(mrb, mrb_obj_ptr(obj), sym);
 }
 
-#define identchar(c) (ISALNUM(c) || (c) == '_' || !ISASCII(c))
-
 MRB_API mrb_bool
 mrb_iv_name_sym_p(mrb_state *mrb, mrb_sym iv_name)
 {
   const char *s;
-  mrb_int i, len;
+  mrb_int len;
 
   s = mrb_sym2name_len(mrb, iv_name, &len);
   if (len < 2) return FALSE;
   if (s[0] != '@') return FALSE;
   if (s[1] == '@') return FALSE;
-  for (i=1; i<len; i++) {
-    if (!identchar(s[i])) return FALSE;
-  }
-  return TRUE;
+  return mrb_ident_p(s+1, len-1);
 }
 
 MRB_API void
@@ -1114,3 +1109,14 @@ mrb_class_find_path(mrb_state *mrb, struct RClass *c)
   }
   return path;
 }
+
+#define identchar(c) (ISALNUM(c) || (c) == '_' || !ISASCII(c))
+
+mrb_bool
+mrb_ident_p(const char *s, mrb_int len)
+{
+  for (mrb_int i = 0; i < len; i++) {
+    if (!identchar(s[i])) return FALSE;
+  }
+  return TRUE;
+}

test/t/module.rb

@@ -210,6 +210,7 @@ assert('Module#const_defined?', '15.2.2.4.20') do
 
   assert_true Test4ConstDefined.const_defined?(:Const4Test4ConstDefined)
   assert_false Test4ConstDefined.const_defined?(:NotExisting)
+  assert_raise(NameError){ Test4ConstDefined.const_defined?(:wrong_name) }
 end
 
 assert('Module#const_get', '15.2.2.4.21') do
@@ -224,6 +225,7 @@ assert('Module#const_get', '15.2.2.4.21') do
   assert_raise(TypeError){ Test4ConstGet.const_get(123) }
   assert_raise(NameError){ Test4ConstGet.const_get(:I_DO_NOT_EXIST) }
   assert_raise(NameError){ Test4ConstGet.const_get("I_DO_NOT_EXIST::ME_NEITHER") }
+  assert_raise(NameError){ Test4ConstGet.const_get(:wrong_name) }
 end
 
 assert('Module#const_set', '15.2.2.4.23') do
@@ -233,6 +235,9 @@ assert('Module#const_set', '15.2.2.4.23') do
 
   assert_equal 23, Test4ConstSet.const_set(:Const4Test4ConstSet, 23)
   assert_equal 23, Test4ConstSet.const_get(:Const4Test4ConstSet)
+  ["", "wrongNAME", "Wrong-Name"].each do |n|
+    assert_raise(NameError) { Test4ConstSet.const_set(n, 1) }
+  end
 end
 
 assert('Module#remove_const', '15.2.2.4.40') do
@@ -240,21 +245,12 @@ assert('Module#remove_const', '15.2.2.4.40') do
     ExistingConst = 23
   end
 
-  result = Test4RemoveConst.module_eval { remove_const :ExistingConst }
-
-  name_error = false
-  begin
-    Test4RemoveConst.module_eval { remove_const :NonExistingConst }
-  rescue NameError
-    name_error = true
+  assert_equal 23, Test4RemoveConst.remove_const(:ExistingConst)
+  assert_false Test4RemoveConst.const_defined?(:ExistingConst)
+  assert_raise(NameError) { Test4RemoveConst.remove_const(:NonExistingConst) }
+  %i[x X!].each do |n|
+    assert_raise(NameError) { Test4RemoveConst.remove_const(n) }
   end
-
-  # Constant removed from Module
-  assert_false Test4RemoveConst.const_defined? :ExistingConst
-  # Return value of binding
-  assert_equal 23, result
-  # Name Error raised when Constant doesn't exist
-  assert_true name_error
 end
 
 assert('Module#const_missing', '15.2.2.4.22') do