WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
164738
Update SubtleCrypto::encrypt to match the latest spec
https://bugs.webkit.org/show_bug.cgi?id=164738
Summary
Update SubtleCrypto::encrypt to match the latest spec
Jiewen Tan
Reported
2016-11-14 15:49:00 PST
Update SubtleCrypto::encrypt to match the latest spec.
Attachments
Patch
(200.26 KB, patch)
2016-11-17 19:00 PST
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Patch
(201.49 KB, patch)
2016-11-17 19:45 PST
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Patch
(204.37 KB, patch)
2016-11-17 20:08 PST
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Patch
(221.89 KB, patch)
2016-11-18 12:16 PST
,
Jiewen Tan
bfulgham
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2016-11-14 16:37:07 PST
<
rdar://problem/29257812
>
Jiewen Tan
Comment 2
2016-11-17 19:00:11 PST
Created
attachment 295119
[details]
Patch
WebKit Commit Bot
Comment 3
2016-11-17 19:03:05 PST
Attachment 295119
[details]
did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:71: CryptoAlgorithmRSA_OAEP::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:41: encryptRSA_OAEP is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:56: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:78: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:41: transformAES_CBC is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:77: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:44: IV_SIZE is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:69: CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:62: CryptoAlgorithmRSAES_PKCS1_v1_5::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:39: encryptRSAES_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:50: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:71: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 12 in 60 files If any of these errors are false positives, please file a bug against check-webkit-style.
Jiewen Tan
Comment 4
2016-11-17 19:45:39 PST
Created
attachment 295128
[details]
Patch
WebKit Commit Bot
Comment 5
2016-11-17 19:48:30 PST
Attachment 295128
[details]
did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:71: CryptoAlgorithmRSA_OAEP::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:41: encryptRSA_OAEP is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:56: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:78: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:39: encryptRSAES_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:50: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:71: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:69: CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:62: CryptoAlgorithmRSAES_PKCS1_v1_5::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:41: transformAES_CBC is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:75: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 11 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.
Jiewen Tan
Comment 6
2016-11-17 20:08:33 PST
Created
attachment 295129
[details]
Patch
WebKit Commit Bot
Comment 7
2016-11-17 20:09:55 PST
Attachment 295129
[details]
did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:71: CryptoAlgorithmRSA_OAEP::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:36: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:41: encryptRSA_OAEP is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:56: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:78: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:39: encryptRSAES_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:50: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:71: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:69: CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:62: CryptoAlgorithmRSAES_PKCS1_v1_5::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:41: transformAES_CBC is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:75: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:36: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 14 in 64 files If any of these errors are false positives, please file a bug against check-webkit-style.
Andy Estes
Comment 8
2016-11-18 10:46:19 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:55 > + Encrypt,
Should this go below Digest to keep the values sorted?
> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:78 > + workQueue.dispatch([capturedParameters = WTFMove(parameters), capturedKey = WTFMove(key), capturedPlainText = WTFMove(plainText), capturedCallback = WTFMove(callback), capturedExceptionCallback = WTFMove(exceptionCallback), context]() mutable {
It's ok to reuse parameter names in your capture list. So this can be [parameters = WTFMove(parameters), key = WTFMove(key), ...].
> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:53 > + workQueue.dispatch([capturedKey = WTFMove(key), capturedPlainText = WTFMove(plainText), capturedCallback = WTFMove(callback), capturedExceptionCallback = WTFMove(exceptionCallback), context]() mutable {
Ditto.
> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:59 > + workQueue.dispatch([capturedParameters = WTFMove(parameters), capturedKey = WTFMove(key), capturedPlainText = WTFMove(plainText), capturedCallback = WTFMove(callback), capturedExceptionCallback = WTFMove(exceptionCallback), context]() mutable {
Ditto.
Brent Fulgham
Comment 9
2016-11-18 10:51:29 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
Looks good! I had a few nitpicks before we land this.
> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:432 > + auto exceptionCallback = [capturedPromise = promise.copyRef()](ExceptionCode ec) mutable {
Please remove the extra space between "capturedPromise =" and "promise.copyRef()"
> Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:71 > + auto& aesParameters = downcast<CryptoAlgorithmAesCbcParams>(*parameters);
Is parameters guaranteed to be non-null? If so, why isn't it passed as a reference? If it can be nullptr, please check (or at least ASSERT).
> Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:73 > + exceptionCallback(OperationError);
Is 'OperationError' from the specification? I wonder if this should be "DataError" or "TypeError" instead?
> Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:64 > + if (key->type() != CryptoKeyType::Public) {
If key cannot be nullptr, it seems like a better API would be to pass Ref<CryptoKey>. Otherwise, we need a nullptr check.
> Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:73 > + if (key->type() != CryptoKeyType::Public) {
Ditto nullptr check (or use Ref<>).
> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:40 > +// FIXME: We should change iv and data to Vector<uint8_t> type once WebKitSubtleCrypto is deprecated.
I suggest you supply a Bugzilla bug here.
> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:77 > + context->ref();
Seems like context should be passed as reference.
> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:100 > + assert(sizeof(parameters.iv) == kCCBlockSizeAES128);
Why is this 'assert' instead of our "ASSERT"?
> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:44 > + if (status)
This could be combined with the line above: if (CCCryptorStatus status = ....) // Do error handling
> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:52 > + context->ref();
nullptr check or pass by reference?
> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:82 > +// FIXME: We should get rid of the magic number 1024. It only makes sense when key length < 8192 bits
Bugzilla please, or else we will never remember to do this!
> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:40 > +// FIXME: We should change data to Vector<uint8_t> type once WebKitSubtleCrypto is deprecated.
Bugzilla please.
> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:50 > + if (status)
Suggest you combine with line 49 into one statement.
> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:58 > + context->ref();
Ditto nullptr check or reference.
> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:60 > + ASSERT(context);
Yikes! We just dereferenced it above!
> Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:40 > +// FIXME: We should get rid of magic number 16384. It assumes that the length of provided key will not exceed 16KB.
Ditto Bugzilla
> Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:234 > +// FIXME: We should use WorkQueue here instead of dispatch_async once WebKitSubtleCrypto is deprecated.
Ditto Bugzilla.
Jiewen Tan
Comment 10
2016-11-18 11:02:59 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
Thanks, Andy.
>> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:55 >> + Encrypt, > > Should this go below Digest to keep the values sorted?
The order I try to follow is that from
https://w3c.github.io/webcrypto/Overview.html#subtlecrypto-interface
. It makes more sense than alphabetic that it groups all related operations together.
>> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:78 >> + workQueue.dispatch([capturedParameters = WTFMove(parameters), capturedKey = WTFMove(key), capturedPlainText = WTFMove(plainText), capturedCallback = WTFMove(callback), capturedExceptionCallback = WTFMove(exceptionCallback), context]() mutable { > > It's ok to reuse parameter names in your capture list. So this can be [parameters = WTFMove(parameters), key = WTFMove(key), ...].
Fixed.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:53 >> + workQueue.dispatch([capturedKey = WTFMove(key), capturedPlainText = WTFMove(plainText), capturedCallback = WTFMove(callback), capturedExceptionCallback = WTFMove(exceptionCallback), context]() mutable { > > Ditto.
Fixed.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:59 >> + workQueue.dispatch([capturedParameters = WTFMove(parameters), capturedKey = WTFMove(key), capturedPlainText = WTFMove(plainText), capturedCallback = WTFMove(callback), capturedExceptionCallback = WTFMove(exceptionCallback), context]() mutable { > > Ditto.
Fixed.
Jiewen Tan
Comment 11
2016-11-18 11:59:51 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
Thanks, Brent.
>> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:432 >> + auto exceptionCallback = [capturedPromise = promise.copyRef()](ExceptionCode ec) mutable { > > Please remove the extra space between "capturedPromise =" and "promise.copyRef()"
Fixed.
>> Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:71 >> + auto& aesParameters = downcast<CryptoAlgorithmAesCbcParams>(*parameters); > > Is parameters guaranteed to be non-null? If so, why isn't it passed as a reference? If it can be nullptr, please check (or at least ASSERT).
Yes, it is guaranteed. I use pointer here because I want polymorphism. Added ASSERT.
>> Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:73 >> + exceptionCallback(OperationError); > > Is 'OperationError' from the specification? I wonder if this should be "DataError" or "TypeError" instead?
It is from specification.
https://w3c.github.io/webcrypto/Overview.html#aes-cbc-operations
Encrypt Step 1.
>> Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:64 >> + if (key->type() != CryptoKeyType::Public) { > > If key cannot be nullptr, it seems like a better API would be to pass Ref<CryptoKey>. Otherwise, we need a nullptr check.
Fixed. Used Ref<CryptoKey>.
>> Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:73 >> + if (key->type() != CryptoKeyType::Public) { > > Ditto nullptr check (or use Ref<>).
Ditto.
>> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:40 >> +// FIXME: We should change iv and data to Vector<uint8_t> type once WebKitSubtleCrypto is deprecated. > > I suggest you supply a Bugzilla bug here.
Added.
https://bugs.webkit.org/show_bug.cgi?id=164939
.
>> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:77 >> + context->ref(); > > Seems like context should be passed as reference.
Fixed.
>> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:100 >> + assert(sizeof(parameters.iv) == kCCBlockSizeAES128); > > Why is this 'assert' instead of our "ASSERT"?
Just a typo. Fixed.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:44 >> + if (status) > > This could be combined with the line above: > > if (CCCryptorStatus status = ....) > // Do error handling
Probably can't. We don't use status afterward. Use if (CCRSACryptorEncrypt(key, ccPKCS1Padding, data, dataLength, cipherText.data(), &cipherTextLength, 0, 0, kCCDigestNone)) instead.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:52 >> + context->ref(); > > nullptr check or pass by reference?
Fixed. By reference.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:82 >> +// FIXME: We should get rid of the magic number 1024. It only makes sense when key length < 8192 bits > > Bugzilla please, or else we will never remember to do this!
Don't worry. I will fix this in my next decrypt patch.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:40 >> +// FIXME: We should change data to Vector<uint8_t> type once WebKitSubtleCrypto is deprecated. > > Bugzilla please.
https://bugs.webkit.org/show_bug.cgi?id=164939
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:50 >> + if (status) > > Suggest you combine with line 49 into one statement.
Fixed.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:58 >> + context->ref(); > > Ditto nullptr check or reference.
Fixed.
>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:60 >> + ASSERT(context); > > Yikes! We just dereferenced it above!
Where?
>> Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:40 >> +// FIXME: We should get rid of magic number 16384. It assumes that the length of provided key will not exceed 16KB. > > Ditto Bugzilla
https://bugs.webkit.org/show_bug.cgi?id=164942
>> Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp:234 >> +// FIXME: We should use WorkQueue here instead of dispatch_async once WebKitSubtleCrypto is deprecated. > > Ditto Bugzilla.
https://bugs.webkit.org/show_bug.cgi?id=164943
Jiewen Tan
Comment 12
2016-11-18 12:16:50 PST
Created
attachment 295176
[details]
Patch
WebKit Commit Bot
Comment 13
2016-11-18 12:19:10 PST
Attachment 295176
[details]
did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:71: CryptoAlgorithmRSA_OAEP::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:173: CryptoAlgorithmRSA_OAEP::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:36: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:42: encryptRSA_OAEP is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:56: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:77: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:40: encryptRSAES_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:50: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:70: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:69: CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:139: CryptoAlgorithmAES_CBC::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_KW.cpp:122: CryptoAlgorithmAES_KW::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:151: CryptoAlgorithmRSASSA_PKCS1_v1_5::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:62: CryptoAlgorithmRSAES_PKCS1_v1_5::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:127: CryptoAlgorithmRSAES_PKCS1_v1_5::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:42: transformAES_CBC is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:76: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:36: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 19 in 70 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brent Fulgham
Comment 14
2016-11-18 13:07:50 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
>>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:60 >>> + ASSERT(context); >> >> Yikes! We just dereferenced it above! > > Where?
We dereference it on line 58, then pass it to the lambda. ASSERTING here is okay, but if it might be nullptr we should check before entering the lambda.
Brent Fulgham
Comment 15
2016-11-18 13:09:19 PST
Comment on
attachment 295176
[details]
Patch Looks great! r=me.
Jiewen Tan
Comment 16
2016-11-18 13:13:26 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
Thanks Brent for r+ my patch.
>>>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:60 >>>> + ASSERT(context); >>> >>> Yikes! We just dereferenced it above! >> >> Where? > > We dereference it on line 58, then pass it to the lambda. > > ASSERTING here is okay, but if it might be nullptr we should check before entering the lambda.
I think that's ref not deref.
Brent Fulgham
Comment 17
2016-11-18 13:24:58 PST
Comment on
attachment 295129
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=295129&action=review
>>>>> Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:60 >>>>> + ASSERT(context); >>>> >>>> Yikes! We just dereferenced it above! >>> >>> Where? >> >> We dereference it on line 58, then pass it to the lambda. >> >> ASSERTING here is okay, but if it might be nullptr we should check before entering the lambda. > > I think that's ref not deref.
lol. When it was a pointer, you were calling the "ref()" function through it without null checking. Now that it's a Ref<>, not a RefPtr<> it's clear that this is safe.
Jiewen Tan
Comment 18
2016-11-18 13:31:56 PST
Committed
r208891
: <
http://trac.webkit.org/changeset/208891
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug