[webkit-reviews] review granted: [Bug 196213] Do not terminate the NetworkProcess if a third party application sends a NSCredential with a SecIdentityRef : [Attachment 365898] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 196213: Do not terminate the NetworkProcess if a third party application
sends a NSCredential with a SecIdentityRef
https://bugs.webkit.org/show_bug.cgi?id=196213

Attachment 365898: Patch

https://bugs.webkit.org/attachment.cgi?id=365898&action=review




--- Comment #4 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 365898
  --> https://bugs.webkit.org/attachment.cgi?id=365898
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365898&action=review

r=me

> Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:719
> +    if (!hasProcessPrivilege(ProcessPrivilege::CanAccessCredentials)) {
> +	   result = nullptr;
> +	   return true;
> +    }

It seems a little sloppy that the !hasKey case above and the false if (key)
case below return true without assigning nullptr to result, while here we do
assign nullptr to result. I'd like us to be consistent.

Looking at our caller, nullptr is the default value. So, both choices can work.
Can you pick one and apply it consistently in this function?