[rfc][icedtea-web] Fix for PR1198, JSObject passed incorrectly to Javascript
Jiri Vanek
jvanek at redhat.com
Fri Jan 4 02:23:26 PST 2013
On 11/30/2012 10:08 PM, Adam Domurad wrote:
> Hi all. Attached is a fix for PR1198.
>
> I still plan to write unit tests for at least the new method PluginAppletViewer.toObjectIDString, but I wanted to get this out before leaving for the weekend.
>
> One tricky thing with this patch was that it had to consolidate _a lot_ of duplicated functionality (actually I found some subtle differences in handling, this should be more consistent). Once that was done the actual patch was fairly straight forward.
>
> The basic issue was that JSObject was being passed as if it were a normal Java object, when the liveconnect spec specifies that it should be returned as the underlying javascript object.
>
> A method was added to JSObject to get the underlying reference. This was kept package-private to not pollute its public interface. As well, a new permission was added for accessing this method. To access this outside of the package, a utility was created in JSUtil.
>
> This patch adds a special case to Java->JS communication when sending objects, to indicate that a Javascript object is being passed.
>
> With patch applied, JSObjectFromEval should pass in all browsers.
>
> 2012-11-30 Adam Domurad <adomurad at redhat.com>
>
> Fix PR1198: JSObject passed incorrectly to Javascript
> * plugin/icedteanp/IcedTeaJavaRequestProcessor.cc: Pass extra data for
> 'jsobject' object result messages.
> * plugin/icedteanp/IcedTeaPluginRequestProcessor.cc: Same.
> * plugin/icedteanp/IcedTeaPluginUtils.cc: Add special casing of
> javascript references passed from java.
> * plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java:
> New permission for unboxing a JSObject's internal reference.
> * plugin/icedteanp/java/netscape/javascript/JSObject.java
> (getInternalReference): New, package-private, retrieves internal
> reference (Must have proper permission).
> * plugin/icedteanp/java/netscape/javascript/JSUtil.java
> (getJSObjectInternalReference) New, utility for accessing
> JSObject#getInternalReference from outside the package.
> * plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java:
> (toObjectIDString): New, creates a string that precisely identifies a
> Java object.
> (handleMessage): Replace a lot of duplicated functionality with
> 'toObjectIDString'.
> * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java: Replace
> duplicated functionality with 'toObjectIDString'.
> * tests/reproducers/simple/JSObjectFromEval/srcs/JSObjectFromEval.java:
> Don't print out type passed (differs from browser to browser).
> * tests/reproducers/simple/JSObjectFromEval/testcases/JSObjectFromEvalTest.java:
> Don't check type passed (differs from browser to browser). Remove
> known-to-fail. Reformat.
>
>
> jsfix2.patch
>
Again adding Jana.
>
> diff --git a/NEWS b/NEWS
> --- a/NEWS
> +++ b/NEWS
> @@ -19,6 +19,7 @@ New in release 1.4 (2012-XX-XX):
> * Plugin
> - PR1106: Buffer overflow in plugin table-
> - PR1166: Embedded JNLP File is not supported in applet tag
> + - PR1198: JSObject is not passed to javascript correctly
> * Common
> - PR1049: Extension jnlp's signed jar with the content of only META-INF/* is considered
> - PR955: regression: SweetHome3D fails to run
> diff --git a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> --- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> +++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> @@ -131,7 +131,7 @@ JavaRequestProcessor::newMessageOnBus(co
> !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
> --- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
> +++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
> @@ -413,7 +413,7 @@ PluginRequestProcessor::setMember(std::v
> 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
> --- a/plugin/icedteanp/IcedTeaPluginUtils.cc
> +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc
> @@ -776,6 +776,14 @@ javaStringResultToNPVariant(const std::s
> }
>
> static bool
> +javaJSObjectResultToNPVariant(const std::string& js_id, NPVariant* variant)
> +{
> + NPVariant* result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(js_id);
> + *variant = *result_variant;
> + return true;
> +}
It returns always true?? Maybe void or *variant?
> +
> +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::javaResultToNPVa
> 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
> --- 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
> --- /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
> --- 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
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
> @@ -53,13 +53,16 @@ import java.security.Permissions;
> import java.security.PrivilegedAction;
> import java.security.ProtectionDomain;
> import java.util.ArrayList;
> +import java.util.Arrays;
> import java.util.Hashtable;
> import java.util.List;
> import java.util.Map;
>
> +import net.sourceforge.jnlp.DefaultLaunchHandler;
> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> -import net.sourceforge.jnlp.DefaultLaunchHandler;
> +import netscape.javascript.JSObject;
> import netscape.javascript.JSObjectCreatePermission;
> +import netscape.javascript.JSUtil;
cosmetic...
>
> class Signature {
> private String signature;
> @@ -318,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) {
> +
> + /* Null, pass special "null" string: */
> + if (obj == null) {
> + return "literalreturn null";
> + }
> +
> + /* Void (can occur from declared return type), pass special "void" string: */
> + if (type == Void.TYPE) {
> + return "literalreturn void";
> + }
> +
> + /* 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 == Double.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();
> @@ -426,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(" ");
> @@ -489,9 +529,7 @@ public class PluginAppletSecurityContext
> final Object fValue = MethodOverloadResolver.getCostAndCastedObject(value, f.getType())[1];
>
> AccessControlContext acc = callContext != null ? callContext : getClosedAccessControlContext();
> - checkPermission(src,
> - message.startsWith("SetStaticField") ? (Class) o : o.getClass(),
> - acc);
> + checkPermission(src, message.startsWith("SetStaticField") ? (Class) o : o.getClass(), acc);
>
> Object ret = AccessController.doPrivileged(new PrivilegedAction<Object>() {
> public Object run() {
> @@ -514,27 +552,12 @@ public class PluginAppletSecurityContext
> Integer arrayID = parseCall(args[1], null, Integer.class);
> Integer index = parseCall(args[2], null, Integer.class);
>
> - Object ret = Array.get(store.getObject(arrayID), index);
> - Class retClass = store.getObject(arrayID).getClass().getComponentType(); // prevent auto-boxing influence
> + Object array = store.getObject(arrayID);
> + 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(" ");
> @@ -584,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));
> @@ -688,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);
Yap, this refactoring looks really nice and seems ok.
> } else if (message.startsWith("GetSuperclass")) {
> String[] args = message.split(" ");
> Integer classID = parseCall(args[1], null, Integer.class);
> @@ -776,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));
Cosmetic change - see lower.
>
> write(reference, "GetStringUTFChars " + buf);
> } else if (message.startsWith("GetStringChars")) {
> @@ -795,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));
Cosmetic change - see lower.
>
> PluginDebug.debug("Java: GetStringChars: ", o);
> PluginDebug.debug(" String BYTES: ", buf);
> @@ -815,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));
Cosmetic change - see lower.
>
> 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
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> @@ -1160,43 +1160,21 @@ public class PluginAppletViewer extends
> }
>
> public static void setMember(long internal, String name, Object value) {
> - System.err.println("Setting to class " + value.getClass() + ":" + value.getClass().isPrimitive());
> - AppletSecurityContextManager.getSecurityContext(0).store(name);
> - int nameID = AppletSecurityContextManager.getSecurityContext(0).getIdentifier(name);
> + PluginDebug.debug("Setting to class " + value.getClass() + ":" + value.getClass().isPrimitive());
nice catch, if you will be in mood, you can add this line to cosmetic changes as told below.
> + PluginAppletSecurityContext securityContext = AppletSecurityContextManager.getSecurityContext(0);
> + securityContext.store(name);
> + int nameID = securityContext.getIdentifier(name);
> 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 + " SetMember " +
> - internal + " " + nameID + " " + valueToSetTo, reference);
> + internal + " " + nameID + " " + objIDStr, reference);
>
> streamhandler.postCallRequest(request);
> streamhandler.write(request.getMessage());
> @@ -1218,41 +1196,17 @@ public class PluginAppletViewer extends
>
> // FIXME: handle long index as well.
> public static void setSlot(long internal, int index, Object value) {
> - AppletSecurityContextManager.getSecurityContext(0).store(value);
> + PluginAppletSecurityContext securityContext = AppletSecurityContextManager.getSecurityContext(0);
> + 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());
> diff --git a/tests/reproducers/simple/JSObjectFromEval/srcs/JSObjectFromEval.java b/tests/reproducers/simple/JSObjectFromEval/srcs/JSObjectFromEval.java
> --- a/tests/reproducers/simple/JSObjectFromEval/srcs/JSObjectFromEval.java
> +++ b/tests/reproducers/simple/JSObjectFromEval/srcs/JSObjectFromEval.java
> @@ -55,8 +55,6 @@ public class JSObjectFromEval extends Ap
> }
>
> public void setJSMember(JSObject js, String memb, Object val) {
> - String typeName = val.getClass().getName();
> - System.out.println("setJSMember: passed '" + typeName + "'");
Not worthy to keep in debug?
Anyway - cosmetic change. See my comment lower.
> js.setMember(memb, val);
> }
> }
> \ No newline at end of file
> diff --git a/tests/reproducers/simple/JSObjectFromEval/testcases/JSObjectFromEvalTest.java b/tests/reproducers/simple/JSObjectFromEval/testcases/JSObjectFromEvalTest.java
> --- a/tests/reproducers/simple/JSObjectFromEval/testcases/JSObjectFromEvalTest.java
> +++ b/tests/reproducers/simple/JSObjectFromEval/testcases/JSObjectFromEvalTest.java
> @@ -40,7 +40,6 @@ import static org.junit.Assert.assertTru
> import net.sourceforge.jnlp.ProcessResult;
> import net.sourceforge.jnlp.ServerAccess.AutoClose;
> import net.sourceforge.jnlp.annotations.Bug;
> -import net.sourceforge.jnlp.annotations.KnownToFail;
> import net.sourceforge.jnlp.annotations.NeedsDisplay;
> import net.sourceforge.jnlp.annotations.TestInBrowsers;
> import net.sourceforge.jnlp.browsertesting.BrowserTest;
> @@ -51,40 +50,37 @@ import org.junit.Test;
>
> public class JSObjectFromEvalTest extends BrowserTest {
>
> - private static final String END_STRING = AutoOkClosingListener.MAGICAL_OK_CLOSING_STRING;
> + private static final String END_STRING = AutoOkClosingListener.MAGICAL_OK_CLOSING_STRING;
>
> - private static final String JAVA_CREATE = "Java create\n";
> - private static final String JS_CREATE = "JS create\n";
> - private static final String JAVA_SET = "Java set\n";
> - private static final String PASSED_INTEGER = "setJSMember: passed 'java.lang.Integer'\n";
> - private static final String CORRECT_VALUE = "obj.test = 0";
> + private static final String JAVA_CREATE = "Java create\n";
> + private static final String JS_CREATE = "JS create\n";
> + private static final String JAVA_SET = "Java set\n";
> + private static final String CORRECT_VALUE = "obj.test = 0";
>
> - @Test
> - @TestInBrowsers(testIn = { Browsers.all })
> - @NeedsDisplay
> - @Bug(id = { "PR1198" })
> - @KnownToFail
> - public void testJSObjectSetMemberIsSet() throws Exception {
> - ProcessResult pr = server.executeBrowser("/JSObjectFromEval.html",
> - AutoClose.CLOSE_ON_BOTH);
> + @Test
> + @TestInBrowsers(testIn = { Browsers.all })
> + @NeedsDisplay
> + @Bug(id = { "PR1198" })
> + public void testJSObjectSetMemberIsSet() throws Exception {
> + ProcessResult pr = server.executeBrowser("/JSObjectFromEval.html",
> + AutoClose.CLOSE_ON_BOTH);
>
> - String expectedJSCreateOutput = JS_CREATE + JAVA_SET + PASSED_INTEGER
> - + CORRECT_VALUE;
> - String expectedJavaCreateOutput = JAVA_CREATE + JAVA_SET
> - + PASSED_INTEGER + CORRECT_VALUE;
> + String expectedJSCreateOutput = JS_CREATE + JAVA_SET + CORRECT_VALUE;
> + String expectedJavaCreateOutput = JAVA_CREATE + JAVA_SET
> + + CORRECT_VALUE;
>
> - // No reason JS create should fail, this is mostly a sanity check:
> - assertTrue("stdout should contain 'JS create [...] " + CORRECT_VALUE
> - + "' but did not.", pr.stdout.contains(expectedJSCreateOutput));
> + // No reason JS create should fail, this is mostly a sanity check:
> + assertTrue("stdout should contain 'JS create [...] " + CORRECT_VALUE
> + + "' but did not.", pr.stdout.contains(expectedJSCreateOutput));
>
> - // Demonstrates PR1198:
> - assertTrue("stdout should contain 'Java create [...] " + CORRECT_VALUE
> - + "' but did not.",
> - pr.stdout.contains(expectedJavaCreateOutput));
> + // Demonstrates PR1198:
> + assertTrue("stdout should contain 'Java create [...] " + CORRECT_VALUE
> + + "' but did not.",
> + pr.stdout.contains(expectedJavaCreateOutput));
>
> - // Make sure we got to the end of the script
> - assertTrue("stdout should contain '" + END_STRING + "' but did not.",
> - pr.stdout.contains(END_STRING));
> - }
> + // Make sure we got to the end of the script
> + assertTrue("stdout should contain '" + END_STRING + "' but did not.",
> + pr.stdout.contains(END_STRING));
> + }
>
> }
Cosmetic changes - normlay I'm for smughling small cosmetich changes which were found during coding, especially somnething like
> - buf
> - .append(" "
> - + Integer
> - .toString(((int) b[i])& 0x0ff, 16));
> + buf.append(" " + Integer.toString(((int) b[i])& 0x0ff, 16));
:))
into patch. But there is one whole refacotred file and little bit more. If you will be willing can you separate the changes? You can push the comsetic changes directly.
Well and then you will start to hate me with removing of KnowToFail...
Thanx for fix.
J.
ps: Try to check js<->java automated tests and manual liveConnect testuite. I think there wil be some hddien fixes by this aptch. Adnd With some luck also no regressions;) Maybe jana Can meassure those results for you. She is th eguru of liveConnect testing afterall..
pps: Dont take me wrong, this is really nice cleanup and fix.
More information about the distro-pkg-dev
mailing list