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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Aug 21 16:07:50 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 65028: Implement NPN_GetValueForURL and NPN_SetValueForURL and
NPN_GetAuthenticationInfo [Update VI]...
https://bugs.webkit.org/attachment.cgi?id=65028&action=review

------- Additional Comments from Andreas Kling <andreas.kling at nokia.com>
>WebCore/plugins/PluginView.cpp:1386
> +	 NPError result;
Should probably be initialized to NPERR_GENERIC_ERROR like in setValueForURL()
below.

>WebCore/plugins/PluginView.cpp:1393
> +	     if (u.isValid()) {
Missing an "else" that sets result to NPERR_INVALID_URL.

>WebCore/plugins/PluginView.cpp:1397
> +		     strncpy(*value, cookieStr.utf8().data(),
cookieStr.length() + 1);
cookieStr.utf8() may be of a different length than cookieStr.length() + 1.
You should use the length() of cookieStr.utf8() instead of cookieStr itself.

>WebCore/plugins/PluginView.cpp:1432
> +	     if (u.isValid()) {
Missing an "else" that sets result to NPERR_INVALID_URL.

>WebCore/plugins/PluginView.cpp:1452
> +  NPError PluginView::getAuthenticationInfo(const char* protocol, const
char* host, int32_t port, const char* scheme, const char *realm, char**
username, uint32_t* ulen, char** password, uint32_t* plen)
Coding style, "const char *realm" should be "const char* realm". This error is
repeated a number of times in the patch.

>WebCore/plugins/qt/PluginViewQt.cpp:754
> +	     *value = static_cast<char*>(NPN_MemAlloc(proxyStr.length() + 1));
If NPN_MemAlloc() fails, we should return NPN_OUT_OF_MEMORY_ERROR.

>WebCore/plugins/qt/PluginViewQt.cpp:755
> +	     strncpy(*value, proxyStr.toUtf8().constData(), proxyStr.length() +
1);
> +	     if (len)
> +		 *len = strlen(*value);
Again mixing the UTF8 data with the Unicode length.

This should be something like:

QByteArray utf8 = proxyStr.toUtf8();
memcpy(*value, utf8.constData(), utf8.length());
if (len)
    *len = utf8.length();

Furthermore, are we really supposed to return an error code when setting an
empty cookie string?
Same question for when getting the cookie string returns "".


More information about the webkit-reviews mailing list