Bug 164740 – Update SubtleCrypto::sign to match the latest spec

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jiewen Tan]

Update SubtleCrypto::sign to match the latest spec.

[Radar WebKit Bug Importer]

<rdar://problem/29257864>

[Jiewen Tan]

Created attachment 295347 [details]
Patch

[WebKit Commit Bot]

Attachment 295347 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Comment on attachment 295347 [details]
Patch

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

> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:557
> +    auto exceptionCallback = [capturedPromise = promise.copyRef()](ExceptionCode ec) mutable {

You can probably move the promise here.

> Source/WebCore/crypto/SubtleCrypto.idl:36
> +    [Custom] Promise<any> sign(AlgorithmIdentifier algorithm, CryptoKey key, BufferSource data);

Can we try making it non custom?
This would probably require adding converters for AlgorithmIdentifier and CryptoKey but the code is probably already in JSSubtleCryptoCustom.

[Brent Fulgham]

EFL/GTK seems to be failing due to "WebCore::CryptoAlgorithmHMAC::platformSign". Are they not getting built? Or do we need stubs for these platforms?

[Jiewen Tan]

(In reply to comment #5)
> EFL/GTK seems to be failing due to
> "WebCore::CryptoAlgorithmHMAC::platformSign". Are they not getting built? Or
> do we need stubs for these platforms?

I do add WebCore::CryptoAlgorithmHMAC::platformSign into the gnutls implementations. Not sure why GTK/EFL fail to build.

[Jiewen Tan]

Comment on attachment 295347 [details]
Patch

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

Thanks, Youenn.

>> Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp:557
>> +    auto exceptionCallback = [capturedPromise = promise.copyRef()](ExceptionCode ec) mutable {
> 
> You can probably move the promise here.

Hmmm, you are right.

>> Source/WebCore/crypto/SubtleCrypto.idl:36
>> +    [Custom] Promise<any> sign(AlgorithmIdentifier algorithm, CryptoKey key, BufferSource data);
> 
> Can we try making it non custom?
> This would probably require adding converters for AlgorithmIdentifier and CryptoKey but the code is probably already in JSSubtleCryptoCustom.

CryptoKey is an interface, it should be fine. However, AlgorithmIdentifier equals any here. I don't think it is good to make it an interface. Or if I miss some ways that we could add a customized type?

[Jiewen Tan]

Created attachment 295462 [details]
Patch

[Brent Fulgham]

Comment on attachment 295462 [details]
Patch

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

This patch doesn't seem to apply on EWS. Can you upload a rebaselined version?

> LayoutTests/imported/w3c/WebCryptoAPI/idlharness-expected.txt:51
> +PASS SubtleCrypto interface: calling sign(AlgorithmIdentifier,CryptoKey,BufferSource) on crypto.subtle with too few arguments must throw TypeError 

Yay!

[Jiewen Tan]

Created attachment 295511 [details]
Patch

[WebKit Commit Bot]

Attachment 295511 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jiewen Tan]

Created attachment 295515 [details]
Patch

[WebKit Commit Bot]

Attachment 295515 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jiewen Tan]

Created attachment 295519 [details]
Patch

[WebKit Commit Bot]

Attachment 295519 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:105:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jiewen Tan]

Created attachment 295575 [details]
Patch

[WebKit Commit Bot]

Attachment 295575 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:107:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 295575 [details]
Patch

Attachment 295575 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2586814

New failing tests:
intersection-observer/intersection-observer-entry-interface.html

[Build Bot]

Created attachment 295583 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Brent Fulgham]

Comment on attachment 295575 [details]
Patch

I think this looks good. Can you investigate the ios-sim failure and see whether that's a real bug? Otherwise, r=me.

[Jiewen Tan]

(In reply to comment #20)
> Comment on attachment 295575 [details]
> Patch
> 
> I think this looks good. Can you investigate the ios-sim failure and see
> whether that's a real bug? Otherwise, r=me.
According to the test history:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=intersection-observer%2Fintersection-observer-entry-interface.html

I think the test just somehow fails all the time regardless of my patch.

[Jiewen Tan]

(In reply to comment #20)
> Comment on attachment 295575 [details]
> Patch
> 
> I think this looks good. Can you investigate the ios-sim failure and see
> whether that's a real bug? Otherwise, r=me.

Thanks Brent for r+ my patch.

[Jiewen Tan]

Created attachment 295629 [details]
Patch for landing

[Jiewen Tan]

Created attachment 295645 [details]
Patch for landing

[WebKit Commit Bot]

Attachment 295645 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:71:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:60:  signRSASSA_PKCS1_v1_5 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:85:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:107:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jiewen Tan]

Committed r209092: <http://trac.webkit.org/changeset/209092>