Bug 175657 – [WebCrypto][Mac] Replace CCECCryptorGetKeyComponents with CCECCryptorExportKey for exporting ECC JWKs

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Jiewen Tan]

For unknown reasons, CCECCryptorGetKeyComponents sometimes output wrong values. Hence, we need to replace it with CCECCryptorGetKeyComponents for reliable outputs.

[Jiewen Tan]

<rdar://problem/33797150>

[Jiewen Tan]

Created attachment 318401 [details]
Patch

[Brent Fulgham]

Comment on attachment 318401 [details]
Patch

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

I think this looks good, but I would prefer you revise the return code as I outlined above. r=me assuming you make the changes I suggest.

> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:149
> +    return Exception { OperationError };

I think this would be clearer if you did the early return for the failure case, rather than success.

I also think this would be clearer (use 'isEmpty()' rather than a zero-check for size())

if (result.isEmpty())
    return Exception { OperationError };

return WTFMove(result);

> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:168
> +    return Exception { OperationError };

Ditto the early-return case.

> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:179
> +    return Exception { OperationError };

Ditto.

> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:190
> +    return Exception { OperationError };

Ditto.

[Jiewen Tan]

Comment on attachment 318401 [details]
Patch

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

Thanks Brent for r+ my patch.

>> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:149
>> +    return Exception { OperationError };
> 
> I think this would be clearer if you did the early return for the failure case, rather than success.
> 
> I also think this would be clearer (use 'isEmpty()' rather than a zero-check for size())
> 
> if (result.isEmpty())
>     return Exception { OperationError };
> 
> return WTFMove(result);

Fixed.

>> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:168
>> +    return Exception { OperationError };
> 
> Ditto the early-return case.

Fixed.

>> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:179
>> +    return Exception { OperationError };
> 
> Ditto.

Fixed.

>> Source/WebCore/crypto/keys/CryptoKeyEC.cpp:190
>> +    return Exception { OperationError };
> 
> Ditto.

Fixed.

[Jiewen Tan]

Created attachment 318528 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 318528 [details]
Patch for landing

Clearing flags on attachment: 318528

Committed r220933: <http://trac.webkit.org/changeset/220933>