[rfc][icedtea-web] Fix for PR518, NPString.utf8characters not guaranteed to be null-terminated
Deepak Bhole
dbhole at redhat.com
Mon Jun 4 08:53:00 PDT 2012
* 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);
>
More information about the distro-pkg-dev
mailing list