Fix for PR1198: JSObject passed incorrectly to Javascript

8 files changed, 183 insertions, 171 deletions

diff --git a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
index c20a317..7529f5a 100644
--- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
+++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
@@ -131,7 +131,7 @@ JavaRequestProcessor::newMessageOnBus(const char* message)
 			           !message_parts->at(4)->find("GetObjectArrayElement"))
 			{
 
-			    if (!message_parts->at(5)->find("literalreturn"))
+			    if (!message_parts->at(5)->find("literalreturn") || !message_parts->at(5)->find("jsobject"))
                 {
 			        // literal returns don't have a corresponding jni id
 			        result->return_identifier = 0;
diff --git a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
index 9eba0b5..dc611c6 100644
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
@@ -413,7 +413,7 @@ PluginRequestProcessor::setMember(std::vector<std::string*>* message_parts)
     member = (NPVariant*) (IcedTeaPluginUtilities::stringToJSID(*(message_parts->at(5))));
     propertyNameID = *(message_parts->at(6));
 
-    if (*(message_parts->at(7)) == "literalreturn")
+    if (*(message_parts->at(7)) == "literalreturn" || *(message_parts->at(7)) == "jsobject" )
     {
         value.append(*(message_parts->at(7)));
         value.append(" ");
diff --git a/plugin/icedteanp/IcedTeaPluginUtils.cc b/plugin/icedteanp/IcedTeaPluginUtils.cc
index 150694c..6e41c19 100644
--- a/plugin/icedteanp/IcedTeaPluginUtils.cc
+++ b/plugin/icedteanp/IcedTeaPluginUtils.cc
@@ -776,6 +776,14 @@ javaStringResultToNPVariant(const std::string& jobject_id, NPVariant* variant)
 }
 
 static bool
+javaJSObjectResultToNPVariant(const std::string& js_id, NPVariant* variant)
+{
+    NPVariant* result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(js_id);
+    *variant = *result_variant;
+    return true;
+}
+
+static bool
 javaObjectResultToNPVariant(NPP instance, const std::string& jobject_id, NPVariant* variant)
 {
     // Reference the class object so we can construct an NPObject with it and the instance
@@ -811,9 +819,14 @@ IcedTeaPluginUtilities::javaResultToNPVariant(NPP instance,
         std::string* java_value, NPVariant* variant)
 {
     int literal_n = sizeof("literalreturn"); // Accounts for one space char
+    int jsobject_n = sizeof("jsobject"); // Accounts for one space char
+
     if (strncmp("literalreturn ", java_value->c_str(), literal_n) == 0)
     {
         javaPrimitiveResultToNPVariant(java_value->substr(literal_n), variant);
+    } else if (strncmp("jsobject ", java_value->c_str(), jsobject_n) == 0)
+    {
+        javaJSObjectResultToNPVariant(java_value->substr(jsobject_n), variant);
     } else
     {
         std::string jobject_id = *java_value;
diff --git a/plugin/icedteanp/java/netscape/javascript/JSObject.java b/plugin/icedteanp/java/netscape/javascript/JSObject.java
index 0de500f..489efa6 100644
--- a/plugin/icedteanp/java/netscape/javascript/JSObject.java
+++ b/plugin/icedteanp/java/netscape/javascript/JSObject.java
@@ -100,6 +100,16 @@ public final class JSObject {
     }
 
     /**
+     * Package-private method used through JSUtil#getJSObjectInternalReference.
+     * We make this package-private to avoid polluting the public interface.
+     * @return the internal identifier
+     */
+    long getInternalReference() {
+        AccessController.getContext().checkPermission(new JSObjectUnboxPermission());
+        return internal;
+    }
+
+    /**
      * it is illegal to construct a JSObject manually
      */
     public JSObject(int jsobj_addr) {
diff --git a/plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java b/plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java
new file mode 100644
index 0000000..724ddb3
--- /dev/null
+++ b/plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java
@@ -0,0 +1,49 @@
+/* JSObjectUnboxPermission.java
+   Copyright (C) 2012  Red Hat
+
+This file is part of IcedTea.
+
+IcedTea is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2, or (at your option)
+any later version.
+
+IcedTea is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with IcedTea; see the file COPYING.  If not, write to the
+Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version. */
+
+package netscape.javascript;
+
+import java.security.BasicPermission;
+
+/**
+ * Permission to access internal reference of JSObject
+ */
+public class JSObjectUnboxPermission extends BasicPermission {
+    public JSObjectUnboxPermission() {
+        super("JSObjectUnbox");
+    }
+}
diff --git a/plugin/icedteanp/java/netscape/javascript/JSUtil.java b/plugin/icedteanp/java/netscape/javascript/JSUtil.java
index 14dfda0..8e841fc 100644
--- a/plugin/icedteanp/java/netscape/javascript/JSUtil.java
+++ b/plugin/icedteanp/java/netscape/javascript/JSUtil.java
@@ -57,4 +57,16 @@ public class JSUtil {
 
         return captureStream.toString();
     }
-}
+
+    /**
+     * Uses package-private method JSObject.getInternalReference.
+     * This is package-private to avoid polluting the public interface.
+     * @param js JSObject to unbox
+     * @return the internal reference stored by the JSObject
+     */
+    public static long getJSObjectInternalReference(JSObject js) {
+        // NB: permission is checked in JSObject
+        return js.getInternalReference();
+    }
+
+}
\ No newline at end of file
diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java b/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
index dc209ff..a015884 100644
--- a/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
@@ -321,6 +321,83 @@ public class PluginAppletSecurityContext {
         return map;
     }
 
+    private static long privilegedJSObjectUnbox(final JSObject js) {
+        return AccessController.doPrivileged(new PrivilegedAction<Long>() {
+            public Long run() {  
+                return JSUtil.getJSObjectInternalReference(js); 
+            }
+        });
+    }
+
+    /**
+     * Create a string that identifies a Java object precisely, for passing to 
+     * Javascript.
+     * 
+     * For builtin value types, a 'literalreturn' prefix is used and the object 
+     * is passed with a string representation.
+     * 
+     * For JSObject's, a 'jsobject' prefix is used and the object is passed 
+     * with the JSObject's internal identifier.
+     * 
+     * For other Java objects, an object store reference is used.
+     * 
+     * @param obj the object for which to create an identifier
+     * @param type the type to use for representation decisions
+     * @param unboxPrimitives whether to treat boxed primitives as value types
+     * @return an identifier string
+     */
+    public String toObjectIDString(Object obj, Class<?> type, boolean unboxPrimitives) {
+
+        /* Void (can occur from declared return type), pass special "void" string: */
+        if (type == Void.TYPE) {
+            return "literalreturn void";
+        }
+
+        /* Null, pass special "null" string: */
+        if (obj == null) {
+            return "literalreturn null";
+        }
+
+        /* Primitive, accurately represented by its toString() form: */
+        boolean returnAsString = ( type == Boolean.TYPE
+                                || type == Byte.TYPE 
+                                || type == Short.TYPE 
+                                || type == Integer.TYPE 
+                                || type == Long.TYPE );
+        if (unboxPrimitives) {
+            returnAsString = ( returnAsString 
+                            || type == Boolean.class
+                            || type == Byte.class
+                            || type == Short.class
+                            || type == Integer.class 
+                            || type == Long.class);
+        }
+        if (returnAsString) {
+            return "literalreturn " + obj.toString();
+        } 
+
+        /* Floating point number, we ensure we give enough precision: */
+        if ( type == Float.TYPE || type == Double.TYPE || 
+                ( unboxPrimitives && (type == Float.class || type == Double.class) )) {
+            return "literalreturn " + String.format("%308.308e", obj);
+        }
+
+        /* Character that should be returned as number: */
+        if (type == Character.TYPE || (unboxPrimitives && type == Character.class)) {
+            return "literalreturn " + (int) (Character) obj;
+        }
+
+        /* JSObject, unwrap underlying Javascript reference: */
+        if (type == netscape.javascript.JSObject.class) {
+            long reference = privilegedJSObjectUnbox((JSObject)obj);
+            return "jsobject " + Long.toString(reference);
+        }
+
+        /* Other kind of object, track this object and return our reference: */
+        store.reference(obj);
+        return store.getIdentifier(obj).toString();
+    }
+
     public void handleMessage(int reference, String src, AccessControlContext callContext, String message) {
 
         startTime = new java.util.Date().getTime();
@@ -429,56 +506,16 @@ public class PluginAppletSecurityContext {
                 if (ret instanceof Throwable)
                     throw (Throwable) ret;
 
-                if (ret == null) {
-                    write(reference, "GetStaticField literalreturn null");
-                } else if (f.getType() == Boolean.TYPE
-                        || f.getType() == Byte.TYPE
-                        || f.getType() == Short.TYPE
-                        || f.getType() == Integer.TYPE
-                        || f.getType() == Long.TYPE) {
-                    write(reference, "GetStaticField literalreturn " + ret);
-                } else if (f.getType() == Float.TYPE
-                                || f.getType() == Double.TYPE) {
-                    write(reference, "GetStaticField literalreturn " + String.format("%308.308e", ret));
-                } else if (f.getType() == Character.TYPE) {
-                    write(reference, "GetStaticField literalreturn " + (int) (Character) ret);
-                } else {
-                    // Track returned object.
-                    store.reference(ret);
-                    write(reference, "GetStaticField " + store.getIdentifier(ret));
-                }
+                String objIDStr = toObjectIDString(ret, f.getType(), false /*do not unbox primitives*/);
+                write(reference, "GetStaticField " + objIDStr);
             } else if (message.startsWith("GetValue")) {
                 String[] args = message.split(" ");
                 Integer index = parseCall(args[1], null, Integer.class);
 
                 Object ret = store.getObject(index);
 
-                if (ret == null) {
-                    write(reference, "GetValue literalreturn null");
-                } else if (ret.getClass() == Boolean.TYPE
-                        || ret.getClass() == Boolean.class
-                        || ret.getClass() == Byte.TYPE
-                        || ret.getClass() == Byte.class
-                        || ret.getClass() == Short.TYPE
-                        || ret.getClass() == Short.class
-                        || ret.getClass() == Integer.TYPE
-                        || ret.getClass() == Integer.class
-                        || ret.getClass() == Long.TYPE
-                        || ret.getClass() == Long.class) {
-                    write(reference, "GetValue literalreturn " + ret);
-                } else if (ret.getClass() == Float.TYPE
-                        || ret.getClass() == Float.class
-                        || ret.getClass() == Double.TYPE
-                        || ret.getClass() == Double.class) {
-                    write(reference, "GetValue literalreturn " + String.format("%308.308e", ret));
-                } else if (ret.getClass() == Character.TYPE
-                        || ret.getClass() == Character.class) {
-                    write(reference, "GetValue literalreturn " + (int) (Character) ret);
-                } else {
-                    // Track returned object.
-                    store.reference(ret);
-                    write(reference, "GetValue " + store.getIdentifier(ret));
-                }
+                String objIDStr = toObjectIDString(ret, ret.getClass(), true /*unbox primitives*/);
+                write(reference, "GetValue " + objIDStr);
             } else if (message.startsWith("SetStaticField") ||
                                    message.startsWith("SetField")) {
                 String[] args = message.split(" ");
@@ -519,24 +556,8 @@ public class PluginAppletSecurityContext {
                 Object ret = Array.get(array, index);
                 Class<?> retClass = array.getClass().getComponentType(); // prevent auto-boxing influence
 
-                if (ret == null) {
-                    write(reference, "GetObjectArrayElement literalreturn null");
-                } else if (retClass == Boolean.TYPE
-                        || retClass == Byte.TYPE
-                        || retClass == Short.TYPE
-                        || retClass == Integer.TYPE
-                        || retClass == Long.TYPE) {
-                    write(reference, "GetObjectArrayElement literalreturn " + ret);
-                } else if (retClass == Float.TYPE
-                                || retClass == Double.TYPE) {
-                    write(reference, "GetObjectArrayElement literalreturn " + String.format("%308.308e", ret));
-                } else if (retClass == Character.TYPE) {
-                    write(reference, "GetObjectArrayElement literalreturn " + (int) (Character) ret);
-                } else {
-                    // Track returned object.
-                    store.reference(ret);
-                    write(reference, "GetObjectArrayElement " + store.getIdentifier(ret));
-                }
+                String objIDStr = toObjectIDString(ret, retClass, false /*do not unbox primitives*/);
+                write(reference, "GetObjectArrayElement " + objIDStr);
 
             } else if (message.startsWith("SetObjectArrayElement")) {
                 String[] args = message.split(" ");
@@ -586,25 +607,8 @@ public class PluginAppletSecurityContext {
                 if (ret instanceof Throwable)
                     throw (Throwable) ret;
 
-                if (ret == null) {
-                    write(reference, "GetField literalreturn null");
-                } else if (f.getType() == Boolean.TYPE
-                        || f.getType() == Byte.TYPE
-                        || f.getType() == Short.TYPE
-                        || f.getType() == Integer.TYPE
-                        || f.getType() == Long.TYPE) {
-                    write(reference, "GetField literalreturn " + ret);
-                } else if (f.getType() == Float.TYPE
-                                || f.getType() == Double.TYPE) {
-                    write(reference, "GetField literalreturn " + String.format("%308.308e", ret));
-                } else if (f.getType() == Character.TYPE) {
-                    write(reference, "GetField literalreturn " + (int) (Character) ret);
-                } else {
-                    // Track returned object.
-                    store.reference(ret);
-                    write(reference, "GetField " + store.getIdentifier(ret));
-                }
-
+                String objIDStr = toObjectIDString(ret, f.getType(), false /*do not unbox primitives*/);
+                write(reference, "GetField " + objIDStr);
             } else if (message.startsWith("GetObjectClass")) {
                 int oid = Integer.parseInt(message.substring("GetObjectClass"
                                                 .length() + 1));
@@ -690,27 +694,8 @@ public class PluginAppletSecurityContext {
                                                 , collapsedArgs, " and that returned: ", ret
                                                 , " of type ", retO);
 
-                if (m.getReturnType().equals(java.lang.Void.class) ||
-                                    m.getReturnType().equals(java.lang.Void.TYPE)) {
-                    write(reference, "CallMethod literalreturn void");
-                } else if (ret == null) {
-                    write(reference, "CallMethod literalreturn null");
-                } else if (m.getReturnType() == Boolean.TYPE
-                                                || m.getReturnType() == Byte.TYPE
-                                                || m.getReturnType() == Short.TYPE
-                                                || m.getReturnType() == Integer.TYPE
-                                                || m.getReturnType() == Long.TYPE) {
-                    write(reference, "CallMethod literalreturn " + ret);
-                } else if (m.getReturnType() == Float.TYPE
-                                || m.getReturnType() == Double.TYPE) {
-                    write(reference, "CallMethod literalreturn " + String.format("%308.308e", ret));
-                } else if (m.getReturnType() == Character.TYPE) {
-                    write(reference, "CallMethod literalreturn " + (int) (Character) ret);
-                } else {
-                    // Track returned object.
-                    store.reference(ret);
-                    write(reference, "CallMethod " + store.getIdentifier(ret));
-                }
+                String objIDStr = toObjectIDString(ret, m.getReturnType(), false /*do not unbox primitives*/);
+                write(reference, "CallMethod " + objIDStr);
             } else if (message.startsWith("GetSuperclass")) {
                 String[] args = message.split(" ");
                 Integer classID = parseCall(args[1], null, Integer.class);
@@ -778,10 +763,7 @@ public class PluginAppletSecurityContext {
                 buf = new StringBuffer(b.length * 2);
                 buf.append(b.length);
                 for (int i = 0; i < b.length; i++)
-                    buf
-                                                        .append(" "
-                                                                        + Integer
-                                                                                        .toString(((int) b[i]) & 0x0ff, 16));
+                    buf.append(" " + Integer.toString(((int) b[i]) & 0x0ff, 16));
 
                 write(reference, "GetStringUTFChars " + buf);
             } else if (message.startsWith("GetStringChars")) {
@@ -797,10 +779,7 @@ public class PluginAppletSecurityContext {
                 buf = new StringBuffer(b.length * 2);
                 buf.append(b.length);
                 for (int i = 0; i < b.length; i++)
-                    buf
-                                                        .append(" "
-                                                                        + Integer
-                                                                                        .toString(((int) b[i]) & 0x0ff, 16));
+                    buf.append(" " + Integer.toString(((int) b[i]) & 0x0ff, 16));
 
                 PluginDebug.debug("Java: GetStringChars: ", o);
                 PluginDebug.debug("  String BYTES: ", buf);
@@ -817,10 +796,7 @@ public class PluginAppletSecurityContext {
                 buf = new StringBuffer(b.length * 2);
                 buf.append(b.length);
                 for (int i = 0; i < b.length; i++)
-                    buf
-                            .append(" "
-                                    + Integer
-                                            .toString(((int) b[i]) & 0x0ff, 16));
+                    buf.append(" " + Integer.toString(((int) b[i]) & 0x0ff, 16));
 
                 write(reference, "GetToStringValue " + buf);
             } else if (message.startsWith("NewArray")) {
diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
index 3a56133..ac818d5 100644
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
@@ -1022,36 +1022,13 @@ public class PluginAppletViewer extends XEmbeddedFrame
 
         // work on a copy of value, as we don't want to be manipulating
         // complex objects
-        String valueToSetTo;
-        if (value instanceof java.lang.Byte ||
-                value instanceof java.lang.Character ||
-                value instanceof java.lang.Short ||
-                value instanceof java.lang.Integer ||
-                value instanceof java.lang.Long ||
-                value instanceof java.lang.Float ||
-                value instanceof java.lang.Double ||
-                value instanceof java.lang.Boolean) {
-
-            valueToSetTo = "literalreturn " + value.toString();
-
-            // Character -> Str results in str value.. we need int value as
-            // per specs.
-            if (value instanceof java.lang.Character) {
-                valueToSetTo = "literalreturn " + (int) ((java.lang.Character) value).charValue();
-            } else if (value instanceof Float ||
-                        value instanceof Double) {
-                valueToSetTo = "literalreturn " + String.format("%308.308e", value);
-            }
-
-        } else {
-            AppletSecurityContextManager.getSecurityContext(0).store(value);
-            valueToSetTo = Integer.toString(AppletSecurityContextManager.getSecurityContext(0).getIdentifier(value));
-        }
+        String objIDStr = securityContext.toObjectIDString(value,
+                value.getClass(), true /* unbox primitives */);
 
         // Prefix with dummy instance for convenience.
         PluginCallRequest request = requestFactory.getPluginCallRequest("void",
                 "instance " + 0 + " reference " + reference + " SetMember " +
-                        internal + " " + nameID + " " + valueToSetTo, reference);
+                        internal + " " + nameID + " " + objIDStr, reference);
 
         streamhandler.postCallRequest(request);
         streamhandler.write(request.getMessage());
@@ -1077,38 +1054,13 @@ public class PluginAppletViewer extends XEmbeddedFrame
         securityContext.store(value);
         Long reference = getRequestIdentifier();
 
-        // work on a copy of value, as we don't want to be manipulating
-        // complex objects
-        String valueToSetTo;
-        if (value instanceof java.lang.Byte ||
-                value instanceof java.lang.Character ||
-                value instanceof java.lang.Short ||
-                value instanceof java.lang.Integer ||
-                value instanceof java.lang.Long ||
-                value instanceof java.lang.Float ||
-                value instanceof java.lang.Double ||
-                value instanceof java.lang.Boolean) {
-
-            valueToSetTo = "literalreturn " + value.toString();
-
-            // Character -> Str results in str value.. we need int value as
-            // per specs.
-            if (value instanceof java.lang.Character) {
-                valueToSetTo = "literalreturn " + (int) ((java.lang.Character) value).charValue();
-            } else if (value instanceof Float ||
-                        value instanceof Double) {
-                valueToSetTo = "literalreturn " + String.format("%308.308e", value);
-            }
-
-        } else {
-            AppletSecurityContextManager.getSecurityContext(0).store(value);
-            valueToSetTo = Integer.toString(AppletSecurityContextManager.getSecurityContext(0).getIdentifier(value));
-        }
+        String objIDStr = securityContext.toObjectIDString(value,
+                value.getClass(), true /* unbox primitives */);
 
         // Prefix with dummy instance for convenience.
         PluginCallRequest request = requestFactory.getPluginCallRequest("void",
                 "instance " + 0 + " reference " + reference + " SetSlot " +
-                        internal + " " + index + " " + valueToSetTo, reference);
+                        internal + " " + index + " " + objIDStr, reference);
 
         streamhandler.postCallRequest(request);
         streamhandler.write(request.getMessage());