/hg/icedtea-web: Fix many memory leaks due to utf8fromidentifier...
adomurad at icedtea.classpath.org
adomurad at icedtea.classpath.org
Tue Jul 30 10:49:18 PDT 2013
changeset f1eaa1ee7891 in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=f1eaa1ee7891
author: Adam Domurad <adomurad at redhat.com>
date: Tue Jul 30 13:49:13 2013 -0400
Fix many memory leaks due to utf8fromidentifier misuse
diffstat:
ChangeLog | 14 ++
plugin/icedteanp/IcedTeaJavaRequestProcessor.cc | 42 +----
plugin/icedteanp/IcedTeaPluginRequestProcessor.cc | 7 +-
plugin/icedteanp/IcedTeaPluginUtils.cc | 12 +
plugin/icedteanp/IcedTeaPluginUtils.h | 3 +
plugin/icedteanp/IcedTeaScriptablePluginObject.cc | 152 +++++++++++-----------
plugin/icedteanp/IcedTeaScriptablePluginObject.h | 36 ++--
tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc | 14 +-
tests/cpp-unit-tests/browser_mock.cc | 12 +-
9 files changed, 163 insertions(+), 129 deletions(-)
diffs (truncated from 779 to 500 lines):
diff -r 754507c1709a -r f1eaa1ee7891 ChangeLog
--- a/ChangeLog Tue Jul 30 14:28:12 2013 +0200
+++ b/ChangeLog Tue Jul 30 13:49:13 2013 -0400
@@ -1,3 +1,17 @@
+2013-07-30 Adam Domurad <adomurad at redhat.com>
+
+ * plugin/icedteanp/IcedTeaPluginUtils.cc
+ (NPIdentifierAsString): Leak-free utf8fromidentifier wrapper.
+ * plugin/icedteanp/IcedTeaPluginUtils.h: Same.
+ * plugin/icedteanp/IcedTeaJavaRequestProcessor.cc: Update calls
+ * plugin/icedteanp/IcedTeaPluginRequestProcessor.cc: Same.
+ * plugin/icedteanp/IcedTeaScriptablePluginObject.cc: Same.
+ * plugin/icedteanp/IcedTeaScriptablePluginObject.h: Same.
+ * tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc
+ (NPIdentifierAsString): New, tests utility function
+ * tests/cpp-unit-tests/browser_mock.cc
+ (mock_utf8fromidentifier): New, mocks NPAPI function
+
2013-07-30 Jiri Vanek <jvanek at redhat.com>
* tests/reproducers/simple/simpletest1/resources/favicon.ico: new file
diff -r 754507c1709a -r f1eaa1ee7891 plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
--- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc Tue Jul 30 14:28:12 2013 +0200
+++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc Tue Jul 30 13:49:13 2013 -0400
@@ -751,36 +751,27 @@
JavaRequestProcessor::getMethodID(std::string classID, NPIdentifier methodName,
std::vector<std::string> args)
{
- JavaRequestProcessor* java_request;
- std::string message = std::string();
- std::string* signature;
-
- signature = new std::string();
- *signature += "(";
+ std::string message, signature = "(";
// FIXME: Need to determine how to extract array types and complex java objects
for (int i=0; i < args.size(); i++)
{
- *signature += args[i];
+ signature += args[i];
}
- *signature += ")";
+ signature += ")";
this->instance = 0; // context is always 0 (needed for java-side backwards compat.)
this->reference = IcedTeaPluginUtilities::getReference();
IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &message);
- message += " GetMethodID ";
- message += classID;
- message += " ";
- message += browser_functions.utf8fromidentifier(methodName);
- message += " ";
- message += *signature;
+ message += " GetMethodID " + classID + " ";
+ message += IcedTeaPluginUtilities::NPIdentifierAsString(methodName) + " ";
+ message += signature;
postAndWaitForResponse(message);
IcedTeaPluginUtilities::releaseReference();
- delete signature;
return result;
}
@@ -789,36 +780,27 @@
JavaRequestProcessor::getStaticMethodID(std::string classID, NPIdentifier methodName,
std::vector<std::string> args)
{
- JavaRequestProcessor* java_request;
- std::string message = std::string();
- std::string* signature;
-
- signature = new std::string();
- *signature += "(";
+ std::string message, signature = "(";
// FIXME: Need to determine how to extract array types and complex java objects
for (int i=0; i < args.size(); i++)
{
- *signature += args[i];
+ signature += args[i];
}
- *signature += ")";
+ signature += ")";
this->instance = 0; // context is always 0 (needed for java-side backwards compat.)
this->reference = IcedTeaPluginUtilities::getReference();
IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &message);
- message += " GetStaticMethodID ";
- message += classID;
- message += " ";
- message += browser_functions.utf8fromidentifier(methodName);
- message += " ";
- message += *signature;
+ message += " GetStaticMethodID " + classID + " ";
+ message += IcedTeaPluginUtilities::NPIdentifierAsString(methodName) + " ";
+ message += signature;
postAndWaitForResponse(message);
IcedTeaPluginUtilities::releaseReference();
- delete signature;
return result;
}
diff -r 754507c1709a -r f1eaa1ee7891 plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc Tue Jul 30 14:28:12 2013 +0200
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc Tue Jul 30 13:49:13 2013 -0400
@@ -763,7 +763,7 @@
else
property_identifier = browser_functions.getstringidentifier(property_id->c_str());
- PLUGIN_DEBUG("Setting %s on instance %p, object %p to value %s\n", browser_functions.utf8fromidentifier(property_identifier), instance, member, value->c_str());
+ PLUGIN_DEBUG("Setting %s on instance %p, object %p to value %s\n", IcedTeaPluginUtilities::NPIdentifierAsString(property_identifier).c_str(), instance, member, value->c_str());
IcedTeaPluginUtilities::javaResultToNPVariant(instance, value, &value_variant);
@@ -783,6 +783,7 @@
std::vector<void*> parameters = ((AsyncCallThreadData*) data)->parameters;
instance = (NPP) parameters.at(0);
+
parent_ptr = (NPObject*) parameters.at(1);
std::string* member_id = (std::string*) parameters.at(2);
NPIdentifier member_identifier;
@@ -795,11 +796,11 @@
member_identifier = browser_functions.getstringidentifier(member_id->c_str());
// Get the NPVariant corresponding to this member
- PLUGIN_DEBUG("Looking for %p %p %p (%s)\n", instance, parent_ptr, member_identifier, browser_functions.utf8fromidentifier(member_identifier));
+ PLUGIN_DEBUG("Looking for %p %p %p (%s)\n", instance, parent_ptr, member_identifier, IcedTeaPluginUtilities::NPIdentifierAsString(member_identifier).c_str());
if (!browser_functions.hasproperty(instance, parent_ptr, member_identifier))
{
- printf("%s not found!\n", browser_functions.utf8fromidentifier(member_identifier));
+ printf("%s not found!\n", IcedTeaPluginUtilities::NPIdentifierAsString(member_identifier).c_str());
}
((AsyncCallThreadData*) data)->call_successful = browser_functions.getproperty(instance, parent_ptr, member_identifier, member_ptr);
diff -r 754507c1709a -r f1eaa1ee7891 plugin/icedteanp/IcedTeaPluginUtils.cc
--- a/plugin/icedteanp/IcedTeaPluginUtils.cc Tue Jul 30 14:28:12 2013 +0200
+++ b/plugin/icedteanp/IcedTeaPluginUtils.cc Tue Jul 30 13:49:13 2013 -0400
@@ -1087,6 +1087,18 @@
str = str.substr(start, end - start + 1);
}
+std::string IcedTeaPluginUtilities::NPIdentifierAsString(NPIdentifier id) {
+ NPUTF8* cstr = browser_functions.utf8fromidentifier(id);
+ if (cstr == NULL) {
+ /* Treat not-existing strings as empty. To tell if it was a valid string,
+ * use browser_functions.identifierisstring. */
+ return std::string();
+ }
+ std::string str = cstr;
+ browser_functions.memfree(cstr);
+ return str;
+}
+
bool IcedTeaPluginUtilities::file_exists(std::string filename)
{
std::ifstream infile(filename.c_str());
diff -r 754507c1709a -r f1eaa1ee7891 plugin/icedteanp/IcedTeaPluginUtils.h
--- a/plugin/icedteanp/IcedTeaPluginUtils.h Tue Jul 30 14:28:12 2013 +0200
+++ b/plugin/icedteanp/IcedTeaPluginUtils.h Tue Jul 30 13:49:13 2013 -0400
@@ -211,6 +211,9 @@
/* This must be freed with browserfunctions.releasevariantvalue */
static NPVariant NPVariantStringCopy(const std::string& result);
+ /* Returns an std::string represented by the given identifier. */
+ static std::string NPIdentifierAsString(NPIdentifier id);
+
/* Frees the given vector and the strings that its contents point to */
static void freeStringPtrVector(std::vector<std::string*>* v);
diff -r 754507c1709a -r f1eaa1ee7891 plugin/icedteanp/IcedTeaScriptablePluginObject.cc
--- a/plugin/icedteanp/IcedTeaScriptablePluginObject.cc Tue Jul 30 14:28:12 2013 +0200
+++ b/plugin/icedteanp/IcedTeaScriptablePluginObject.cc Tue Jul 30 13:49:13 2013 -0400
@@ -59,14 +59,14 @@
}
bool
-IcedTeaScriptablePluginObject::hasMethod(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptablePluginObject::hasMethod(NPObject *npobj, NPIdentifier name_id)
{
printf ("** Unimplemented: IcedTeaScriptablePluginObject::hasMethod %p\n", npobj);
return false;
}
bool
-IcedTeaScriptablePluginObject::invoke(NPObject *npobj, NPIdentifier name, const NPVariant *args,
+IcedTeaScriptablePluginObject::invoke(NPObject *npobj, NPIdentifier name_id, const NPVariant *args,
uint32_t argCount,NPVariant *result)
{
printf ("** Unimplemented: IcedTeaScriptablePluginObject::invoke %p\n", npobj);
@@ -82,17 +82,17 @@
}
bool
-IcedTeaScriptablePluginObject::hasProperty(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptablePluginObject::hasProperty(NPObject *npobj, NPIdentifier name_id)
{
printf ("** Unimplemented: IcedTeaScriptablePluginObject::hasProperty %p\n", npobj);
return false;
}
bool
-IcedTeaScriptablePluginObject::getProperty(NPObject *npobj, NPIdentifier name, NPVariant *result)
+IcedTeaScriptablePluginObject::getProperty(NPObject *npobj, NPIdentifier name_id, NPVariant *result)
{
// Package request?
- if (!strcmp(browser_functions.utf8fromidentifier(name), "java"))
+ if (IcedTeaPluginUtilities::NPIdentifierAsString(name_id) == "java")
{
//NPObject* obj = IcedTeaScriptableJavaPackageObject::get_scriptable_java_package_object(getInstanceFromMemberPtr(npobj), name);
//OBJECT_TO_NPVARIANT(obj, *result);
@@ -104,14 +104,14 @@
}
bool
-IcedTeaScriptablePluginObject::setProperty(NPObject *npobj, NPIdentifier name, const NPVariant *value)
+IcedTeaScriptablePluginObject::setProperty(NPObject *npobj, NPIdentifier name_id, const NPVariant *value)
{
printf ("** Unimplemented: IcedTeaScriptablePluginObject::setProperty %p\n", npobj);
return false;
}
bool
-IcedTeaScriptablePluginObject::removeProperty(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptablePluginObject::removeProperty(NPObject *npobj, NPIdentifier name_id)
{
printf ("** Unimplemented: IcedTeaScriptablePluginObject::removeProperty %p\n", npobj);
return false;
@@ -189,13 +189,13 @@
void
IcedTeaScriptableJavaPackageObject::setPackageName(const NPUTF8* name)
{
- this->package_name->append(name);
+ this->package_name->assign(name);
}
std::string
IcedTeaScriptableJavaPackageObject::getPackageName()
{
- return this->package_name->c_str();
+ return *this->package_name;
}
void
@@ -211,14 +211,14 @@
}
bool
-IcedTeaScriptableJavaPackageObject::hasMethod(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptableJavaPackageObject::hasMethod(NPObject *npobj, NPIdentifier name_id)
{
// Silly caller. Methods are for objects!
return false;
}
bool
-IcedTeaScriptableJavaPackageObject::invoke(NPObject *npobj, NPIdentifier name, const NPVariant *args,
+IcedTeaScriptableJavaPackageObject::invoke(NPObject *npobj, NPIdentifier name_id, const NPVariant *args,
uint32_t argCount,NPVariant *result)
{
printf ("** Unimplemented: IcedTeaScriptableJavaPackageObject::invoke %p\n", npobj);
@@ -234,30 +234,31 @@
}
bool
-IcedTeaScriptableJavaPackageObject::hasProperty(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptableJavaPackageObject::hasProperty(NPObject *npobj, NPIdentifier name_id)
{
- PLUGIN_DEBUG("IcedTeaScriptableJavaPackageObject::hasProperty %s\n", browser_functions.utf8fromidentifier(name));
+ std::string name = IcedTeaPluginUtilities::NPIdentifierAsString(name_id);
+
+ PLUGIN_DEBUG("IcedTeaScriptableJavaPackageObject::hasProperty %s\n", name.c_str());
bool hasProperty = false;
JavaResultData* java_result;
JavaRequestProcessor* java_request = new JavaRequestProcessor();
NPP instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(npobj);
int plugin_instance_id = get_id_from_instance(instance);
+ IcedTeaScriptableJavaPackageObject* scriptable_obj = (IcedTeaScriptableJavaPackageObject*)npobj;
- PLUGIN_DEBUG("Object package name: \"%s\"\n", ((IcedTeaScriptableJavaPackageObject*) npobj)->getPackageName().c_str());
+ PLUGIN_DEBUG("Object package name: \"%s\"\n", scriptable_obj->getPackageName().c_str());
// "^java" is always a package
- if (((IcedTeaScriptableJavaPackageObject*) npobj)->getPackageName().length() == 0 &&
- ( !strcmp(browser_functions.utf8fromidentifier(name), "java") ||
- !strcmp(browser_functions.utf8fromidentifier(name), "javax")))
+ if (scriptable_obj->getPackageName().empty() && (name == "java" || name == "javax"))
{
return true;
}
- std::string property_name = ((IcedTeaScriptableJavaPackageObject*) npobj)->getPackageName();
- if (property_name.length() > 0)
+ std::string property_name = scriptable_obj->getPackageName();
+ if (!property_name.empty())
property_name += ".";
- property_name += browser_functions.utf8fromidentifier(name);
+ property_name += name;
PLUGIN_DEBUG("Looking for name \"%s\"\n", property_name.c_str());
@@ -279,12 +280,13 @@
}
bool
-IcedTeaScriptableJavaPackageObject::getProperty(NPObject *npobj, NPIdentifier name, NPVariant *result)
+IcedTeaScriptableJavaPackageObject::getProperty(NPObject *npobj, NPIdentifier name_id, NPVariant *result)
{
+ std::string name = IcedTeaPluginUtilities::NPIdentifierAsString(name_id);
- PLUGIN_DEBUG("IcedTeaScriptableJavaPackageObject::getProperty %s\n", browser_functions.utf8fromidentifier(name));
+ PLUGIN_DEBUG("IcedTeaScriptableJavaPackageObject::getProperty %s\n", name.c_str());
- if (!browser_functions.utf8fromidentifier(name))
+ if (!browser_functions.identifierisstring(name_id))
return false;
bool isPropertyClass = false;
@@ -292,11 +294,12 @@
JavaRequestProcessor java_request = JavaRequestProcessor();
NPP instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(npobj);
int plugin_instance_id = get_id_from_instance(instance);
+ IcedTeaScriptableJavaPackageObject* scriptable_obj = (IcedTeaScriptableJavaPackageObject*)npobj;
- std::string property_name = ((IcedTeaScriptableJavaPackageObject*) npobj)->getPackageName();
- if (property_name.length() > 0)
- property_name += ".";
- property_name += browser_functions.utf8fromidentifier(name);
+ std::string property_name = scriptable_obj->getPackageName();
+ if (!property_name.empty())
+ property_name += ".";
+ property_name += name;
java_result = java_request.findClass(plugin_instance_id, property_name);
isPropertyClass = (java_result->return_identifier == 0);
@@ -326,14 +329,14 @@
}
bool
-IcedTeaScriptableJavaPackageObject::setProperty(NPObject *npobj, NPIdentifier name, const NPVariant *value)
+IcedTeaScriptableJavaPackageObject::setProperty(NPObject *npobj, NPIdentifier name_id, const NPVariant *value)
{
// Can't be going around setting properties on namespaces.. that's madness!
return false;
}
bool
-IcedTeaScriptableJavaPackageObject::removeProperty(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptableJavaPackageObject::removeProperty(NPObject *npobj, NPIdentifier name_id)
{
printf ("** Unimplemented: IcedTeaScriptableJavaPackageObject::removeProperty %p\n", npobj);
return false;
@@ -462,28 +465,26 @@
}
bool
-IcedTeaScriptableJavaObject::hasMethod(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptableJavaObject::hasMethod(NPObject *npobj, NPIdentifier name_id)
{
-
+ std::string name = IcedTeaPluginUtilities::NPIdentifierAsString(name_id);
IcedTeaScriptableJavaObject* scriptable_object = (IcedTeaScriptableJavaObject*) npobj;
- PLUGIN_DEBUG("IcedTeaScriptableJavaObject::hasMethod %s (ival=%d)\n", browser_functions.utf8fromidentifier(name), browser_functions.intfromidentifier(name));
+ PLUGIN_DEBUG("IcedTeaScriptableJavaObject::hasMethod %s (ival=%d)\n", name.c_str(), browser_functions.intfromidentifier(name_id));
bool hasMethod = false;
// If object is an array and requested "method" may be a number, check for it first
if ( scriptable_object->is_object_array ||
- (browser_functions.intfromidentifier(name) < 0))
+ (browser_functions.intfromidentifier(name_id) < 0))
{
- if (!browser_functions.utf8fromidentifier(name))
+ if (!browser_functions.identifierisstring(name_id))
return false;
JavaResultData* java_result;
JavaRequestProcessor java_request = JavaRequestProcessor();
- std::string methodName = browser_functions.utf8fromidentifier(name);
-
- java_result = java_request.hasMethod(scriptable_object->class_id, methodName);
+ java_result = java_request.hasMethod(scriptable_object->class_id, name);
hasMethod = java_result->return_identifier != 0;
}
@@ -492,13 +493,13 @@
}
bool
-IcedTeaScriptableJavaObject::invoke(NPObject *npobj, NPIdentifier name, const NPVariant *args,
+IcedTeaScriptableJavaObject::invoke(NPObject *npobj, NPIdentifier name_id, const NPVariant *args,
uint32_t argCount, NPVariant *result)
{
- NPUTF8* method_name = browser_functions.utf8fromidentifier(name);
+ std::string name = IcedTeaPluginUtilities::NPIdentifierAsString(name_id);
// Extract arg type array
- PLUGIN_DEBUG("IcedTeaScriptableJavaObject::invoke %s. Args follow.\n", method_name);
+ PLUGIN_DEBUG("IcedTeaScriptableJavaObject::invoke %s. Args follow.\n", name.c_str());
for (int i=0; i < argCount; i++)
{
IcedTeaPluginUtilities::printNPVariant(args[i]);
@@ -535,13 +536,13 @@
PLUGIN_DEBUG("Calling static method\n");
java_result = java_request.callStaticMethod(
IcedTeaPluginUtilities::getSourceFromInstance(instance),
- scriptable_object->class_id, browser_functions.utf8fromidentifier(name), arg_ids);
+ scriptable_object->class_id, name, arg_ids);
} else
{
PLUGIN_DEBUG("Calling method normally\n");
java_result = java_request.callMethod(
IcedTeaPluginUtilities::getSourceFromInstance(instance),
- scriptable_object->instance_id, browser_functions.utf8fromidentifier(name), arg_ids);
+ scriptable_object->instance_id, name, arg_ids);
}
if (java_result->error_occurred)
@@ -558,26 +559,26 @@
}
bool
-IcedTeaScriptableJavaObject::hasProperty(NPObject *npobj, NPIdentifier name)
+IcedTeaScriptableJavaObject::hasProperty(NPObject *npobj, NPIdentifier name_id)
{
- PLUGIN_DEBUG("IcedTeaScriptableJavaObject::hasProperty %s (ival=%d)\n", browser_functions.utf8fromidentifier(name), browser_functions.intfromidentifier(name));
+ std::string name = IcedTeaPluginUtilities::NPIdentifierAsString(name_id);
+
+ PLUGIN_DEBUG("IcedTeaScriptableJavaObject::hasProperty %s (ival=%d)\n", name.c_str(), browser_functions.intfromidentifier(name_id));
bool hasProperty = false;
IcedTeaScriptableJavaObject* scriptable_object = (IcedTeaScriptableJavaObject*)npobj;
// If it is an array, only length and indexes are valid
if (scriptable_object->is_object_array)
{
- if (browser_functions.intfromidentifier(name) >= 0 ||
- !strcmp(browser_functions.utf8fromidentifier(name), "length"))
+ if (browser_functions.intfromidentifier(name_id) >= 0 || name == "length")
hasProperty = true;
} else
{
-
- if (!browser_functions.utf8fromidentifier(name))
+ if (!browser_functions.identifierisstring(name_id))
return false;
- if (!strcmp(browser_functions.utf8fromidentifier(name), "Packages"))
+ if (name == "Packages")
{
hasProperty = true;
} else {
@@ -585,9 +586,7 @@
JavaResultData* java_result;
JavaRequestProcessor java_request = JavaRequestProcessor();
- std::string fieldName = browser_functions.utf8fromidentifier(name);
-
- java_result = java_request.hasField(scriptable_object->class_id, fieldName);
+ java_result = java_request.hasField(scriptable_object->class_id, name);
hasProperty = java_result->return_identifier != 0;
}
@@ -598,9 +597,11 @@
}
bool
-IcedTeaScriptableJavaObject::getProperty(NPObject *npobj, NPIdentifier name, NPVariant *result)
+IcedTeaScriptableJavaObject::getProperty(NPObject *npobj, NPIdentifier name_id, NPVariant *result)
{
- PLUGIN_DEBUG("IcedTeaScriptableJavaObject::getProperty %s (ival=%d)\n", browser_functions.utf8fromidentifier(name), browser_functions.intfromidentifier(name));
+ std::string name = IcedTeaPluginUtilities::NPIdentifierAsString(name_id);
+ bool is_string_id = browser_functions.identifierisstring(name_id);
+ PLUGIN_DEBUG("IcedTeaScriptableJavaObject::getProperty %s (ival=%d)\n", name.c_str(), browser_functions.intfromidentifier(name_id));
JavaResultData* java_result;
JavaRequestProcessor java_request = JavaRequestProcessor();
@@ -614,13 +615,11 @@
if (instance_id.length() > 0) // Could be an array or a simple object
{
// If array and requesting length
- if ( scriptable_object->is_object_array &&
- browser_functions.utf8fromidentifier(name) &&
- !strcmp(browser_functions.utf8fromidentifier(name), "length"))
+ if ( scriptable_object->is_object_array && name == "length")
{
java_result = java_request.getArrayLength(instance_id);
} else if ( scriptable_object->is_object_array &&
More information about the distro-pkg-dev
mailing list