[webkit-reviews] review granted: [Bug 204024] [WebAuthn] Add quirk needed to support legacy Google NFC Titan security keys : [Attachment 383167] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Nov 8 15:07:43 PST 2019
Brent Fulgham <bfulgham at webkit.org> has granted Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 204024: [WebAuthn] Add quirk needed to support legacy Google NFC Titan
security keys
https://bugs.webkit.org/show_bug.cgi?id=204024
Attachment 383167: Patch
https://bugs.webkit.org/attachment.cgi?id=383167&action=review
--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 383167
--> https://bugs.webkit.org/attachment.cgi?id=383167
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=383167&action=review
r=me, but please run the autoreleasepool stuff by David before committing.
> Source/WebKit/ChangeLog:12
> + of those legacy key.
actually speak U2F, indicating we are interacting with one of these legacy
keys.
> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:52
> +{
Should the code in here be wrapped in @autoreleasepool?
> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:53
> + auto versionData = [session transceive:adoptNS([[NSData alloc]
initWithBytes:kCtapNfcAppletSelectionCommand
length:sizeof(kCtapNfcAppletSelectionCommand)]).get()];
auto *versionData maybe?
> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:61
> + // of those legacy key.
Same rephrasing I suggest above.
> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:-71
> - @autoreleasepool {
Why don't you need the @autoreleasepool block anymore? Did you check with David
Kilzer to see if this is good to do?
> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:94
> + auto responseData = [m_session transceive:adoptNS([[NSData alloc]
initWithBytes:data.data() length:data.size()]).get()];
auto *responseData =
More information about the webkit-reviews
mailing list