[webkit-reviews] review granted: [Bug 231986] Fix crash when calling setUsernameForLocalCredentialWithID : [Attachment 441811] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted j_pascoe at apple.com
<j_pascoe at apple.com>'s request for review:
Bug 231986: Fix crash when calling setUsernameForLocalCredentialWithID
https://bugs.webkit.org/show_bug.cgi?id=231986

Attachment 441811: Patch

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




--- Comment #3 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 441811
  --> https://bugs.webkit.org/attachment.cgi?id=441811
Patch

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

r=me

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:341
>      auto updatedTag =
cbor::CBORWriter::write(cbor::CBORValue(WTFMove(updatedUserMap)));
> -    auto secAttrApplicationTag = [NSData
dataWithBytesNoCopy:updatedTag->data() length:updatedTag->size()];
> +
> +    auto secAttrApplicationTag = adoptNS([[NSData alloc]
initWithBytes:updatedTag->data() length:updatedTag->size()]);

It's a good idea not to copy memory if you don't have to, but +[NSData
dataWithBytesNoCopy:length:] requires that the object that holds the bytes stay
alive until the NSData object is deallocated.

This looks like the correct fix.

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:344
> -	   (__bridge id)kSecAttrApplicationTag: secAttrApplicationTag,
> +	   (__bridge id)kSecAttrApplicationTag: secAttrApplicationTag.get(),

In place of using (__bridge id) cast, you can use the new bridge_id_cast()
helper function in <wtf/cocoa/TypeCastsCocoa.h>.

This change is not required to land the patch, though.