[rfc][icedtea-web] Fix for PR518, NPString.utf8characters not guaranteed to be null-terminated
Adam Domurad
adomurad at redhat.com
Thu Jun 7 13:43:58 PDT 2012
Here is a patch for consideration for backporting that is suitable for
1.2 & 1.1.
On Mon, 2012-06-04 at 11:53 -0400, Deepak Bhole wrote:
> * Adam Domurad <adomurad at redhat.com> [2012-06-04 11:42]:
> > Hey all. This is a fix for PR518 that adds a utility function for
> > wrapping NPString's in a null terminated std::string. It is used
> > wherever utf8characters was accessed previously. This also has the
> > benefit of consolidating some #ifdef blocks to one location.
> >
> > I took the route of using std::string over gchar* as suggested in the
> > bug report to be conveniently sure memory is freed.
> >
> > (Note, passing along std::string's by value is quite efficient because
> > GCC uses reference counting techniques for sharing of character arrays
> > used in std::string)
> >
>
> Looks fine to me. OK for HEAD.
>
> Cheers,
> Deepak
>
> > ChangeLog:
> > 2012-06-04 Adam Domurad <adomurad at redhat.com>
> >
> > This patch fixes PR518, ensures null termination of strings based off
> > of NPVariant results.
> > * plugin/icedteanp/IcedTeaPluginUtils.h: Added declaration of
> > NPVariantAsString
> > * plugin/icedteanp/IcedTeaPluginUtils.cc
> > (NPVariantAsString): New. Converts an NPVariant to a
> > std::string, assumes it is a string.
> > (isObjectJSArray): Now uses NPVariantAsString, minor cleanup.
> > * plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> > (plugin_get_documentbase): Now uses NPVariantAsString.
> > * plugin/icedteanp/IcedTeaNPPlugin.cc
> > (NPVariantToString): Now uses NPVariantAsString, minor cleanup.
> >
>
> > diff --git a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> > --- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> > +++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
> > @@ -904,11 +904,7 @@ createJavaObjectFromVariant(NPP instance
> > } else if (NPVARIANT_IS_STRING(variant))
> > {
> > className = "java.lang.String";
> > -#if MOZILLA_VERSION_COLLAPSED < 1090200
> > - stringArg.append(NPVARIANT_TO_STRING(variant).utf8characters, NPVARIANT_TO_STRING(variant).utf8length);
> > -#else
> > - stringArg.append(NPVARIANT_TO_STRING(variant).UTF8Characters, NPVARIANT_TO_STRING(variant).UTF8Length);
> > -#endif
> > + stringArg = IcedTeaPluginUtilities::NPVariantAsString(variant);
> > } else if (NPVARIANT_IS_OBJECT(variant))
> > {
> >
> > diff --git a/plugin/icedteanp/IcedTeaNPPlugin.cc b/plugin/icedteanp/IcedTeaNPPlugin.cc
> > --- a/plugin/icedteanp/IcedTeaNPPlugin.cc
> > +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc
> > @@ -1093,17 +1093,10 @@ plugin_get_documentbase (NPP instance)
> > browser_functions.getproperty(instance, NPVARIANT_TO_OBJECT(location),
> > href_id, &href);
> >
> > + std::string href_str = IcedTeaPluginUtilities::NPVariantAsString(href);
> > +
> > // Strip everything after the last "/"
> > - char *href_str;
> > -#if MOZILLA_VERSION_COLLAPSED < 1090200
> > - href_str = (char*) malloc(sizeof(char)*NPVARIANT_TO_STRING(href).utf8length + 1);
> > - snprintf(href_str, NPVARIANT_TO_STRING(href).utf8length+1, "%s", NPVARIANT_TO_STRING(href).utf8characters);
> > -#else
> > - href_str = (char*) malloc(sizeof(char)*NPVARIANT_TO_STRING(href).UTF8Length + 1);
> > - snprintf(href_str, NPVARIANT_TO_STRING(href).UTF8Length+1, "%s", NPVARIANT_TO_STRING(href).UTF8Characters);
> > -#endif
> > -
> > - gchar** parts = g_strsplit (href_str, "/", -1);
> > + gchar** parts = g_strsplit (href_str.c_str(), "/", -1);
> > guint parts_sz = g_strv_length (parts);
> >
> > std::string location_str;
> > @@ -1119,8 +1112,6 @@ plugin_get_documentbase (NPP instance)
> > browser_functions.releasevariantvalue(&href);
> > browser_functions.releasevariantvalue(&location);
> > g_strfreev(parts);
> > - free(href_str);
> > - href_str = NULL;
> > cleanup_done:
> > PLUGIN_DEBUG ("plugin_get_documentbase return\n");
> > PLUGIN_DEBUG("plugin_get_documentbase returning: %s\n", documentbase_copy);
> > diff --git a/plugin/icedteanp/IcedTeaPluginUtils.cc b/plugin/icedteanp/IcedTeaPluginUtils.cc
> > --- a/plugin/icedteanp/IcedTeaPluginUtils.cc
> > +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc
> > @@ -661,11 +661,8 @@ IcedTeaPluginUtilities::printNPVariant(N
> > }
> > else if (NPVARIANT_IS_STRING(variant))
> > {
> > -#if MOZILLA_VERSION_COLLAPSED < 1090200
> > - PLUGIN_DEBUG("STRING: %s (length=%d)\n", NPVARIANT_TO_STRING(variant).utf8characters, NPVARIANT_TO_STRING(variant).utf8length);
> > -#else
> > - PLUGIN_DEBUG("STRING: %s (length=%d)\n", NPVARIANT_TO_STRING(variant).UTF8Characters, NPVARIANT_TO_STRING(variant).UTF8Length);
> > -#endif
> > + std::string str = IcedTeaPluginUtilities::NPVariantAsString(variant);
> > + PLUGIN_DEBUG("STRING: %s (length=%d)\n", str.c_str(), str.size());
> > }
> > else
> > {
> > @@ -676,52 +673,44 @@ IcedTeaPluginUtilities::printNPVariant(N
> > void
> > IcedTeaPluginUtilities::NPVariantToString(NPVariant variant, std::string* result)
> > {
> > - char str[NUM_STR_BUFFER_SIZE]; // enough for everything except string
> > - char* largestr = NULL;
> > - if (NPVARIANT_IS_VOID(variant))
> > + char conv_str[NUM_STR_BUFFER_SIZE]; // conversion buffer
> > + bool was_string_already = false;
> > +
> > + if (NPVARIANT_IS_STRING(variant))
> > {
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "%p", variant);
> > + result->append(IcedTeaPluginUtilities::NPVariantAsString(variant));
> > + was_string_already = true;
> > + }
> > + else if (NPVARIANT_IS_VOID(variant))
> > + {
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "%p", variant);
> > }
> > else if (NPVARIANT_IS_NULL(variant))
> > {
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "NULL");
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "NULL");
> > }
> > else if (NPVARIANT_IS_BOOLEAN(variant))
> > {
> > if (NPVARIANT_TO_BOOLEAN(variant))
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "true");
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "true");
> > else
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "false");
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "false");
> > }
> > else if (NPVARIANT_IS_INT32(variant))
> > {
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "%d", NPVARIANT_TO_INT32(variant));
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "%d", NPVARIANT_TO_INT32(variant));
> > }
> > else if (NPVARIANT_IS_DOUBLE(variant))
> > {
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "%f", NPVARIANT_TO_DOUBLE(variant));
> > - }
> > - else if (NPVARIANT_IS_STRING(variant))
> > - {
> > -#if MOZILLA_VERSION_COLLAPSED < 1090200
> > - size_t buffersize = sizeof(char)*NPVARIANT_TO_STRING(variant).utf8length+1;
> > - largestr = (char*) malloc(buffersize);
> > - snprintf(str, buffersize, "%s", NPVARIANT_TO_STRING(variant).utf8characters);
> > -#else
> > - size_t buffersize = sizeof(char)*NPVARIANT_TO_STRING(variant).UTF8Length+1;
> > - largestr = (char*) malloc(buffersize);
> > - snprintf(str, buffersize, "%s", NPVARIANT_TO_STRING(variant).UTF8Characters);
> > -#endif
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "%f", NPVARIANT_TO_DOUBLE(variant));
> > }
> > else
> > {
> > - snprintf(str, NUM_STR_BUFFER_SIZE, "[Object %p]", variant);
> > + snprintf(conv_str, NUM_STR_BUFFER_SIZE, "[Object %p]", variant);
> > }
> > - if (largestr != NULL){
> > - result->append(largestr);
> > - free(largestr);
> > - } else {
> > - result->append(str);
> > +
> > + if (!was_string_already){
> > + result->append(conv_str);
> > }
> > }
> >
> > @@ -861,13 +850,7 @@ IcedTeaPluginUtilities::isObjectJSArray(
> > browser_functions.invoke(instance, constructor, toString, NULL, 0, &constructor_str);
> > IcedTeaPluginUtilities::printNPVariant(constructor_str);
> >
> > - std::string constructor_name = std::string();
> > -
> > -#if MOZILLA_VERSION_COLLAPSED < 1090200
> > - constructor_name.append(NPVARIANT_TO_STRING(constructor_str).utf8characters, NPVARIANT_TO_STRING(constructor_str).utf8length);
> > -#else
> > - constructor_name.append(NPVARIANT_TO_STRING(constructor_str).UTF8Characters, NPVARIANT_TO_STRING(constructor_str).UTF8Length);
> > -#endif
> > + std::string constructor_name = IcedTeaPluginUtilities::NPVariantAsString(constructor_str);
> >
> > PLUGIN_DEBUG("Constructor for NPObject is %s\n", constructor_name.c_str());
> >
> > @@ -910,6 +893,20 @@ IcedTeaPluginUtilities::decodeURL(const
> > PLUGIN_DEBUG("SENDING URL: %s\n", *decoded_url);
> > }
> >
> > +/* Copies a variant data type into a C++ string */
> > +std::string
> > +IcedTeaPluginUtilities::NPVariantAsString(NPVariant variant)
> > +{
> > +#if MOZILLA_VERSION_COLLAPSED < 1090200
> > + return std::string((
> > + NPVARIANT_TO_STRING(variant).utf8characters,
> > + NPVARIANT_TO_STRING(variant).utf8ength);
> > +#else
> > + return std::string(
> > + NPVARIANT_TO_STRING(variant).UTF8Characters,
> > + NPVARIANT_TO_STRING(variant).UTF8Length);
> > +#endif
> > +}
> >
> > /**
> > * Posts a function for execution on the plug-in thread and wait for result.
> > diff --git a/plugin/icedteanp/IcedTeaPluginUtils.h b/plugin/icedteanp/IcedTeaPluginUtils.h
> > --- a/plugin/icedteanp/IcedTeaPluginUtils.h
> > +++ b/plugin/icedteanp/IcedTeaPluginUtils.h
> > @@ -209,6 +209,9 @@ class IcedTeaPluginUtilities
> > /* Converts the given integer to a string */
> > static void itoa(int i, std::string* result);
> >
> > + /* Copies a variant data type into a C++ string */
> > + static std::string NPVariantAsString(NPVariant variant);
> > +
> > /* Frees the given vector and the strings that its contents point to */
> > static void freeStringPtrVector(std::vector<std::string*>* v);
> >
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixPR518_1.2.patch
Type: text/x-patch
Size: 6297 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120607/a49aac70/fixPR518_1.2.patch
More information about the distro-pkg-dev
mailing list