Fixes #3688. Multibyte identifiers not marshaled correctly.

This has two fixes. Missed error messages not using str() method to properly build up encoded error string. Also we should ask for rubyName vs Name so we get valid bytes. There is a mystery I added a FIXME for where I don't understand why we build up a RubyString by walking type containment then we walk that string again looking for problems. So there may be more work here? but if so we will open up a new issue for it.
jruby · Jun 14, 2018 · f176877 · f176877
1 parent c93ba6f
commit f176877
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 6 deletions.
diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -40,6 +40,8 @@
 import static org.jruby.util.CodegenUtils.ci;
 import static org.jruby.util.CodegenUtils.p;
 import static org.jruby.util.CodegenUtils.sig;
+import static org.jruby.util.RubyStringBuilder.str;
+import static org.jruby.util.RubyStringBuilder.types;
 import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
 import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
 import static org.objectweb.asm.Opcodes.ACC_STATIC;
@@ -2114,7 +2116,7 @@ public IRubyObject smartLoadNewUser(IRubyObject target, IRubyObject data) {
                     target.callMethod(context, "marshal_load", data);
                     return target;
                 } else {
-                    throw runtime.newTypeError("class " + getName() + " needs to have method `marshal_load'");
+                    throw runtime.newTypeError(str(runtime, "class ", types(runtime, this), " needs to have method `marshal_load'"));
                 }
 
             } else if (!(cache = searchWithCache("marshal_load")).method.isUndefined()) {
@@ -2164,7 +2166,7 @@ public IRubyObject smartLoadOldUser(IRubyObject data) {
                 if (method.call(context, this, getSingletonClass(), "respond_to?", runtime.newSymbol("_load")).isTrue()) {
                     return callMethod(context, "_load", data);
                 } else {
-                    throw runtime.newTypeError("class " + getName() + " needs to have method `_load'");
+                    throw runtime.newTypeError(str(runtime, "class ", types(runtime, this), " needs to have method `_load'"));
                 }
 
             } else if (!(cache = getSingletonClass().searchWithCache("_load")).method.isUndefined()) {
@@ -2176,7 +2178,7 @@ public IRubyObject smartLoadOldUser(IRubyObject data) {
             } else {
 
                 // provide an error, since it doesn't exist
-                throw runtime.newTypeError("class " + getName() + " needs to have method `_load'");
+                throw runtime.newTypeError(str(runtime, "class ", types(runtime, this), " needs to have method `_load'"));
 
             }
         }

diff --git a/core/src/main/java/org/jruby/runtime/marshal/MarshalStream.java b/core/src/main/java/org/jruby/runtime/marshal/MarshalStream.java
@@ -210,7 +210,7 @@ public void writeDirectly(IRubyObject value) throws IOException {
     }
 
     public static String getPathFromClass(RubyModule clazz) {
-        String path = clazz.getName();
+        RubyString path = clazz.rubyName();
 
         if (path.charAt(0) == '#') {
             Ruby runtime = clazz.getRuntime();
@@ -221,10 +221,14 @@ public static String getPathFromClass(RubyModule clazz) {
         RubyModule real = clazz.isModule() ? clazz : ((RubyClass)clazz).getRealClass();
         Ruby runtime = clazz.getRuntime();
 
-        if (runtime.getClassFromPath(path) != real) {
+        // FIXME: This is weird why we do this.  rubyName should produce something which can be referred so what example
+        // will this fail on?  If there is a failing case then passing asJavaString may be broken since it will not be
+        // a properly encoded string.  If this is an issue we should make a clazz.IdPath where all segments are returned
+        // by their id names.
+        if (runtime.getClassFromPath(path.asJavaString()) != real) {
             throw runtime.newTypeError(str(runtime, types(runtime, clazz), " can't be referred"));
         }
-        return path;
+        return path.asJavaString();
     }
 
     private void writeObjectData(IRubyObject value) throws IOException {