[rfc][icedtea-web] Fix for PR1198, JSObject passed incorrectly to Javascript
Jiri Vanek
jvanek at redhat.com
Mon Jan 7 07:07:53 PST 2013
On 01/04/2013 08:36 PM, Adam Domurad wrote:
> On 01/04/2013 05:23 AM, Jiri Vanek wrote:
>> [.. snipped..]
>
> As you suggested, I have separated the cosmetic changes and pushed them. It did prove quite tricky,
> though.
ty :)
>
>>>
>>> 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?
>
> I left this accidentally after it could no longer fail, made it void.
yap
>
>>
>>> +
>>> +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;
>>>
> [... long snip...]
>>> 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.
>
> No, this was part of the expected reproducer output. It was removed from the checks because the type
> is dependent on the browser. It was causing failures on some browsers. I have pushed it with my
> cosmetic changes anyway.
oook.
>
>>> 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.
>
> Will do.
>
> Attached is an updated patch, with some cleanup removed (since it was pushed) and the fixes from my
> unit test patch added.
> See my reply to unit test review though -- I need some additional advice on what to do about the
> security manager problem.
>
> Happy hacking,
> -Adam
>
> jsfix3.patch
>
>
> diff --git a/NEWS b/NEWS
> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,7 @@ New in release 1.4 (2012-XX-XX):
> - PR1166: Embedded JNLP File is not supported in applet tag
> - PR1217: Add command line arguments for plugins
> - PR1189: Icedtea-plugin requires code attribute when using jnlp_href
> + - 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;
> +}
> +
> +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
> @@ -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
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> @@ -1022,36 +1022,13 @@ public class PluginAppletViewer extends
>
> // 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
> 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/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
> @@ -37,8 +37,6 @@ exception statement from your version.
>
> import static org.junit.Assert.assertTrue;
>
> -import net.sourceforge.jnlp.annotations.KnownToFail;
> -
> import net.sourceforge.jnlp.ProcessResult;
> import net.sourceforge.jnlp.ServerAccess.AutoClose;
> import net.sourceforge.jnlp.annotations.Bug;
> @@ -62,7 +60,6 @@ public class JSObjectFromEvalTest extend
> @Test
> @TestInBrowsers(testIn = { Browsers.all })
> @NeedsDisplay
> - @KnownToFail
> @Bug(id = { "PR1198" })
> public void testJSObjectSetMemberIsSet() throws Exception {
> ProcessResult pr = server.executeBrowser("/JSObjectFromEval.html",
>
I'm now ok with the patch except missing unit tests. Also I'm missing the prommised :
" ( unboxPrimitives&& (type == Float.class || type == Double.class) " fixes.
So lets push after those two are solved. Thank you!
J.
More information about the distro-pkg-dev
mailing list