[webkit-reviews] review denied: [Bug 34539] Implement NPN_GetValueForURL and NPN_SetValueForURL and provide a stub for NPN_GetAuthenticationInfo : [Attachment 66374] Implement NPN_GetValueForURL and NPN_SetValueForURL and NPN_GetAuthenticationInfo [Update X]...

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 10 04:34:04 PDT 2010


Andreas Kling <andreas.kling at nokia.com> has denied Dawit A. <adawit at kde.org>'s
request for review:
Bug 34539: Implement NPN_GetValueForURL and NPN_SetValueForURL and provide a
stub for NPN_GetAuthenticationInfo
https://bugs.webkit.org/show_bug.cgi?id=34539

Attachment 66374: Implement NPN_GetValueForURL and NPN_SetValueForURL and
NPN_GetAuthenticationInfo [Update X]...
https://bugs.webkit.org/attachment.cgi?id=66374&action=review

------- Additional Comments from Andreas Kling <andreas.kling at nokia.com>
This page is rather interesting:
https://developer.mozilla.org/en/NPN_GetValueForURL

About the "value" argument:
"Out parameter. The browser passes back the requested information. If the
function succeeds, the result buffer will be allocated with NPN_MemAlloc; the
plugin is responsible for freeing the buffer. Note: the value may have internal
NULL bytes and may not be NULL-terminated."

I think we should be nice and null-terminate the returned value, it won't hurt
and will shield against broken plugins.

> +		   *value =
static_cast<char*>(NPN_MemAlloc(cookieStr.length()));

Allocate cookieStr.length() + 1 bytes.

> +		       strncpy(*value, cookieStr.data(), cookieStr.length());

Use memcpy() instead of strncpy(). (And cookieStr.length() + 1.)

The above two comments apply to all cases where you're NPN_MemAlloc()ing a char
buffer and copying something to it.

> +	   LOG(Plugins, "PluginView::getValueForURL(%s): Need to implement
PluginView::platformGetValueForURL!",
prettyNameForNPNURLVariable(variable).data());
> ...
> +	   LOG(Plugins, "PluginView::getValueForURL: %s",
prettyNameForNPNURLVariable(variable).data());

Nit: inconsistent formatting, second case should be something like:
LOG(Plugins, "PluginView::getValueForURL(%s): Unhandled variable",
prettyNameForNPNURLVariable(variable).data());

r- because of lack of test, that's the important missing piece.


More information about the webkit-reviews mailing list